Bug 26890 - String.prototype.match and replace do not clear global regexp lastIndex per ES5.1 15.5.4.10
Summary: String.prototype.match and replace do not clear global regexp lastIndex per E...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-01 10:54 PDT by John-David Dalton
Modified: 2012-03-08 16:49 PST (History)
4 users (show)

See Also:


Attachments
Fix (11.75 KB, patch)
2012-03-08 16:14 PST, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John-David Dalton 2009-07-01 10:54:19 PDT
According to spec String#replace should perform the following.

var s = '0x2x4x6x8';
var p = /x/g;
s.replace(p, function() { alert(p.lastIndex)  });
// alert 2, then 4, then 6, then 8

alert(p.lastIndex); // alert 0

Opera currently does this correctly. There is a bug reported to Mozilla as well.
https://bugzilla.mozilla.org/show_bug.cgi?id=501739

Discussion:
https://mail.mozilla.org/pipermail/es-discuss/2009-July/009628.html
https://mail.mozilla.org/pipermail/es-discuss/2009-July/009631.html
Comment 1 Gavin Barraclough 2012-03-07 20:42:26 PST
We definitely have a bug here,
Comment 2 Gavin Barraclough 2012-03-07 22:27:51 PST
Gah, premature commit!  Anyway, we definitely have a bug here, but I think the spec may actually call for slightly different expected results.

The spec requires that match first clears lastIndex, then calls exec until it returns null (per 15.5.4.10 8.f.ii).  When exec returns null, it also clears lastIndex (per 15.10.6.2 step 9.a.1).  So after a match, the value of lastIndex must be 0.  Our implementation of match ignores and does not update lastIndex.  Ignoring its initial value is correct, failing to reset it isn't.

var s = '0x2x4x6x8';
var p = /x/g;
p.lastIndex = 3;
s.match(p);
print(p.lastIndex); // prints 3, should be 0 after a match.

Similarly, after a replace without a callback replacer function, the lastIndex should definitely be reset.

var s = '0x2x4x6x8';
var p = /x/g;
p.lastIndex = 3;
s.replace(p,'y');
print(p.lastIndex); // prints 3, should be 0 after a replace.

The spec does not enumerate the stages of performing a replace as clearly as it does for some library functions, but it contains the following two directions:

firstly: "If searchValue.global is true, then search string for all matches of the regular expression searchValue. Do the search in the same manner as in String.prototype.match, including the update of searchValue.lastIndex."
secondly: "If replaceValue is a function, then for each matched substring, call the function"

This clearly implies that these actions should take place in the following order:
1) Find all match results
2) Set the value of lastIndex to 0 (implicitly performed by 15.10.6.2 step 9.a.1 called via the action of the last iteration of 15.5.4.10 8.f.i).
3) Call all replacers.

As such, the replace method should set lastIndex to 0 once, before any replacers are called, and then not change the value.

var s = '0x1x2x3x4';
var p = /x/g;
p.lastIndex = 5;
s.replace(p, function() { return ++p.lastIndex; })
// should be "011223344"
Comment 3 Gavin Barraclough 2012-03-08 16:14:37 PST
Created attachment 130933 [details]
Fix
Comment 4 Gavin Barraclough 2012-03-08 16:49:45 PST
Fixed in r110233