Changeset 252836 in webkit for trunk/Source/JavaScriptCore/builtins

Timestamp:

Nov 23, 2019, 3:23:31 PM (5 years ago)

Author:

Ross Kirsling

Message:

[JSC] GetSubstitution is performed incorrectly via RegExp.prototype[@@replace]
​https://bugs.webkit.org/show_bug.cgi?id=204490

Reviewed by Mark Lam.

JSTests:

• stress/replace-indexed-backreferences.js: Added.
• test262/expectations.yaml: Mark four test cases as passing.

Source/JavaScriptCore:

String.prototype.replace and RegExp.prototype[Symbol.replace] are meant to perform the same substitution
of $-backreferences (called GetSubstitution in the spec: ​https://tc39.es/ecma262/#sec-getsubstitution).

The implementation of this in StringPrototype.cpp is correct but the one in RegExpPrototype.js is not.
In particular, the latter *removes* backreferences with out-of-range indices, instead of leaving them as-is.

One thing that is *not* broken in either implementation and thus maintained here is the fact $10 is interpreted
as $1 followed by a 0 when we have 1 <= n < 10 captures (and analogously for other invalid $nn backreferences).
This behavior is consistent across all engines but currently described incorrectly in the spec; this patch thus
aligns with the spec PR currently open to correct this (​https://github.com/tc39/ecma262/pull/1732).

• builtins/RegExpPrototype.js:

(getSubstitution): Ensure that invalid backreferences remain untouched in the output string.
(replace): Fix off-by-one error when populating captures list. We shouldn't be reserving a slot for the full match.

File:

• trunk/Source/JavaScriptCore/builtins/RegExpPrototype.js (modified) (4 diffs)

trunk/Source/JavaScriptCore/builtins/RegExpPrototype.js

@@ -223 +223 @@
                     let chCode = ch.charCodeAt(0);
                     if (chCode >= 0x30 && chCode <= 0x39) {
+                        let originalStart = start - 1;
                         start++;
+
                         let n = chCode - 0x30;
-                        if (n > m)
+                        if (n > m) {
+                            result = result + replacement.substring(originalStart, start);
                             break;
+                        }
+
                         if (start < replacementLength) {
                             let nextChCode = replacement.charCodeAt(start);
@@ -238 +243 @@
                         }
 
-                        if (n == 0)
+                        if (n == 0) {
+                            result = result + replacement.substring(originalStart, start);
                             break;
-
-                        if (captures[n] != @undefined)
-                            result = result + captures[n];
+                        }
+
+                        let capture = captures[n - 1];
+                        if (capture !== @undefined)
+                            result = result + capture;
                     } else
                         result = result + "$";
@@ -314 +322 @@
             if (capN !== @undefined)
                 capN = @toString(capN);
-            captures[n] = capN;
+            captures.@push(capN);
         }
 
@@ -320 +328 @@
 
         if (functionalReplace) {
-            let replacerArgs = [ matched ].concat(captures.slice(1));
+            let replacerArgs = [ matched ].concat(captures);
             replacerArgs.@push(position);
             replacerArgs.@push(str);