140687 - XPath substring('AB',2) returns empty string - should be 'B'.

Status: VERIFIED FIXED

(Core :: XSLT, defect)

Description

[Thomas Andrews]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.0rc1) Gecko/20020417
BuildID:    2002041711

<xsl:value-of select="substring('AB',2)"/>

returns an empty string, when it should return B.

Similarly, 

<xsl:value-of select="substring('ABC',3)"/>

returns an empty string, but should return C.

I'm guessing this is a boundary condition bug - that it returns an empty string
if the index is >= the length, when it should be >.  That's a C++ writers
mistake, after all, since XSLT starts indices at 1.


This bug can seriously mess up a recursion which processes a string a character
at a time, "losing" the last character.

The URL above is a simple stylesheet which tests a few basic cases.

Reproducible: Always
Steps to Reproduce:
1. Load the given URL above and check the output.

Actual Results:  
substring('A',2) = '' ? 
substring('AB',2) = '' ? 
substring('ABC',2) = 'BC' ? 
substring('ABCD',2) = 'BCD' ?
substring('ABC',3) = '' ? 

Expected Results:  
substring('A',2) = '' ? 
substring('AB',2) = 'A' ? 
substring('ABC',2) = 'BC' ? 
substring('ABCD',2) = 'BCD' ?
substring('ABC',3) = 'C' ?

Comment 1

[Axel Hecht]

hrm. mozilla 0.9.9 goes into an infinite loop on the testcase, that's prolly fixed
by the invalid XML crasher (bug 129006), as we don't process style PI's no more.
Peter, could you try RC1 (I didn't take that home yet, bad me), this might be
another pretty argument to get that landed

Comment 2

[Peter Van der Beken [:peterv]]

Sicking:
http://lxr.mozilla.org/seamonkey/source/extensions/transformiix/source/xpath/StringFunctionCall.cpp#172
, shouldn't that be floor(start + 0.5) > src.length() ? Reassigning to you
because a) doing maths is hard for me on a Sunday morning, b) it's your code and
c) you're not with Netscape ;-).

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

hmm.. yeah, something like that.

Anyway, Thomas, thanks for a good catch! Though you use the wrong namespace for 
the html-output, either use none to get html output or use 
http://www.w3.org/1999/xhtml to get xhtml output.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: patch to fix

actually, this way saves a cycle or two. I.e. if the startposition is more then
length+0.5 (and thus rounded to more then length) we return an empty string

Comment 5

[Axel Hecht]

Comment on attachment 81363 [details] [diff] [review]
patch to fix

r=[e-mail]

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 81363 [details] [diff] [review]
patch to fix

sr=jst

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 81363 [details] [diff] [review]
patch to fix

a=[e-mail].  Make sure it's in the branch and trunk.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

landed on trunk, branch isn't open yet

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in on branch too

Thomas: it would be great if you could download a nightly build in a couple of 
days and test that this now works as it should. If it does could you mark the 
bug "verified"

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Please add fixed1.0.0 keyword when you check in in the branch. 

Comment 11

[Thomas Andrews]

Done!