Changeset 23675 in webkit

Timestamp:

Jun 20, 2007 5:00:12 PM (17 years ago)

Author:

bdash

Message:

2007-06-20 Mark Rowe <​mrowe@apple.com>

Reviewed by Mitz.

Fix ​http://bugs.webkit.org/show_bug.cgi?id=14244
Bug 14244: Data corruption when using a replace() callback function with data containing "$"

• kjs/string_object.cpp: (KJS::replace): When 'replacement' is a function, do not replace $n placeholders in its return value. This matches the behaviour described in ECMA 262 3rd Ed section 15.5.4.1, and as implemented in Firefox.

2007-06-20 Mark Rowe <​mrowe@apple.com>

Reviewed by Mitz.

Test for ​http://bugs.webkit.org/show_bug.cgi?id=14244
Bug 14244: Data corruption when using a replace() callback function with data containing "$"

• fast/js/resources/string-replace-2.js: Update to test with 'replaceValue' being a function returning strings with "$n" placeholders.
• fast/js/string-replace-2-expected.txt:

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/kjs/string_object.cpp (modified) (3 diffs)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/resources/string-replace-2.js (modified) (1 diff)
• LayoutTests/fast/js/string-replace-2-expected.txt (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-06-20  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Mitz.
+
+        Fix http://bugs.webkit.org/show_bug.cgi?id=14244
+        Bug 14244: Data corruption when using a replace() callback function with data containing "$"
+
+        * kjs/string_object.cpp:
+        (KJS::replace):  When 'replacement' is a function, do not replace $n placeholders in its return value.
+        This matches the behaviour described in ECMA 262 3rd Ed section 15.5.4.1, and as implemented in Firefox.
+
 2007-06-14  Anders Carlsson  <andersca@apple.com>
 

trunk/JavaScriptCore/kjs/string_object.cpp

@@ -345 +345 @@
       pushSourceRange(sourceRanges, sourceRangeCount, sourceRangeCapacity, UString::Range(lastIndex, matchIndex - lastIndex));
 
+      UString substitutedReplacement;
       if (replacementFunction) {
           int completeMatchStart = ovector[0];
@@ -350 +351 @@
 
           args.append(jsString(matchString));
-          
+
           for (unsigned i = 0; i < reg->subPatterns(); i++) {
               int matchStart = ovector[(i + 1) * 2];
               int matchLen = ovector[(i + 1) * 2 + 1] - matchStart;
-              
+
               args.append(jsString(source.substr(matchStart, matchLen)));
           }
@@ -361 +362 @@
           args.append(jsString(source));
 
-          replacementString = replacementFunction->call(exec, exec->dynamicInterpreter()->globalObject(), 
-                                                        args)->toString(exec);
-      }
-      
-      UString substitutedReplacement = substituteBackreferences(replacementString, source, ovector, reg);
+          substitutedReplacement = replacementFunction->call(exec, exec->dynamicInterpreter()->globalObject(), 
+                                                             args)->toString(exec);
+      } else
+          substitutedReplacement = substituteBackreferences(replacementString, source, ovector, reg);
+
       pushReplacement(replacements, replacementCount, replacementCapacity, substitutedReplacement);
 

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2007-06-20  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Mitz.
+
+        Test for http://bugs.webkit.org/show_bug.cgi?id=14244
+        Bug 14244: Data corruption when using a replace() callback function with data containing "$"
+
+        * fast/js/resources/string-replace-2.js: Update to test with 'replaceValue' being a function returning strings with "$n" placeholders.
+        * fast/js/string-replace-2-expected.txt:
+
 2007-06-20  Mitz Pettel  <mitz@webkit.org>
 

trunk/LayoutTests/fast/js/resources/string-replace-2.js

@@ -16 +16 @@
 shouldBe("testString.replace(/([aeiou])([a-z])/g, function Capitalize(orig,re1,re2) { return re1.toUpperCase()+re2; })",
         "\"It's the End Of the wOrld As we knOw It, And I fEel fIne.\"");
+shouldBe("testString.replace(/(.*)/g, function replaceWithDollars(matchGroup) { return '$1'; })", "\"$1$1\"");
+shouldBe("testString.replace(/(.)(.*)/g, function replaceWithMultipleDollars(matchGroup) { return '$1$2'; })", "\"$1$2\"");
+shouldBe("testString.replace(/(.)(.*)/, function checkReplacementArguments() { return arguments.length; })", "\"5\"");
 
 var successfullyParsed = true;

trunk/LayoutTests/fast/js/string-replace-2-expected.txt

@@ -10 +10 @@
 PASS testString.replace(/([aeiou])([a-z])/g, function Capitalize(){ return RegExp.$1.toUpperCase()+RegExp.$2; }) is "It's the End Of the wOrld As we knOw It, And I fEel fIne."
 PASS testString.replace(/([aeiou])([a-z])/g, function Capitalize(orig,re1,re2) { return re1.toUpperCase()+re2; }) is "It's the End Of the wOrld As we knOw It, And I fEel fIne."
+PASS testString.replace(/(.*)/g, function replaceWithDollars(matchGroup) { return '$1'; }) is "$1$1"
+PASS testString.replace(/(.)(.*)/g, function replaceWithMultipleDollars(matchGroup) { return '$1$2'; }) is "$1$2"
+PASS testString.replace(/(.)(.*)/, function checkReplacementArguments() { return arguments.length; }) is "5"
 PASS successfullyParsed is true