Source/JavaScriptCore: Build fix attempt after r89885.
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jun 2011 05:55:45 +0000 (05:55 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jun 2011 05:55:45 +0000 (05:55 +0000)
Patch by Ryosuke Niwa <rniwa@webkit.org> on 2011-06-27
* JavaScriptCore.exp:
* jsc.cpp:

LayoutTests: https://bugs.webkit.org/show_bug.cgi?id=50554
RegExp.prototype.toString does not escape slashes

Reviewed by Darin Adler & Oliver Hunt.

The problem here is that we don't escape forwards slashes when converting
a RegExp to a string. This means that RegExp("/").toString() is "///",
which is not a valid RegExp literal. Also, we return an invalid literal
for RegExp.prototype.toString() ("//", which is an empty single-line comment).

From ES5:
"NOTE: The returned String has the form of a RegularExpressionLiteral that
evaluates to another RegExp object with the same behaviour as this object."

Added test cases.

* fast/regex/script-tests/toString.js: Added.
(testFwdSlash):
* fast/regex/toString-expected.txt: Added.
* fast/regex/toString.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@89895 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/regex/script-tests/toString.js [new file with mode: 0644]
LayoutTests/fast/regex/toString-expected.txt [new file with mode: 0644]
LayoutTests/fast/regex/toString.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/RegExpObject.cpp
Source/JavaScriptCore/runtime/RegExpPrototype.cpp

index 6e8f113..ec95006 100644 (file)
@@ -1,3 +1,26 @@
+2011-06-27  Gavin Barraclough  <barraclough@apple.com>
+
+        Reviewed by Darin Adler & Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=50554
+        RegExp.prototype.toString does not escape slashes
+
+        The problem here is that we don't escape forwards slashes when converting
+        a RegExp to a string. This means that RegExp("/").toString() is "///",
+        which is not a valid RegExp literal. Also, we return an invalid literal
+        for RegExp.prototype.toString() ("//", which is an empty single-line comment).
+
+        From ES5:
+        "NOTE: The returned String has the form of a RegularExpressionLiteral that
+        evaluates to another RegExp object with the same behaviour as this object."
+
+        Added test cases.
+
+        * fast/regex/script-tests/toString.js: Added.
+        (testFwdSlash):
+        * fast/regex/toString-expected.txt: Added.
+        * fast/regex/toString.html: Added.
+
 2011-06-27  Pavel Feldman  <pfeldman@chromium.org>
 
         Not reviewed: chromium baselines update.
diff --git a/LayoutTests/fast/regex/script-tests/toString.js b/LayoutTests/fast/regex/script-tests/toString.js
new file mode 100644 (file)
index 0000000..5808e84
--- /dev/null
@@ -0,0 +1,34 @@
+description("This page tests toString conversion of RegExp objects, particularly wrt to '/' characters and RegExp.prototype.");
+
+function testForwardSlash(pattern, _string)
+{
+    string = _string;
+
+    re1 = new RegExp(pattern);
+    re2 = eval(re1.toString());
+
+    return re1.test(string) && re2.test(string);
+}
+
+shouldBe("RegExp('/').source", '"\\\\/"');
+shouldBe("RegExp('').source", '""');
+shouldBe("RegExp.prototype.source", '""');
+
+shouldBe("RegExp('/').toString()", '"/\\\\//"');
+shouldBe("RegExp('').toString()", '"/(?:)/"');
+shouldBe("RegExp.prototype.toString()", '"/(?:)/"');
+
+// These strings are equivalent, since the '\' is identity escaping the '/' at the string level.
+shouldBeTrue('testForwardSlash("^/$", "/");');
+shouldBeTrue('testForwardSlash("^\/$", "/");');
+// This string passes "^\/$" to the RegExp, so the '/' is escaped in the re!
+shouldBeTrue('testForwardSlash("^\\/$", "/");');
+// These strings pass "^\\/$" and "^\\\/$" respectively to the RegExp, giving one '\' to match.
+shouldBeTrue('testForwardSlash("^\\\\/$", "\\/");');
+shouldBeTrue('testForwardSlash("^\\\\\\/$", "\\/");');
+// These strings match two backslashes (the second with the '/' escaped).
+shouldBeTrue('testForwardSlash("^\\\\\\\\/$", "\\\\/");');
+shouldBeTrue('testForwardSlash("^\\\\\\\\\\/$", "\\\\/");');
+
+var successfullyParsed = true;
+
diff --git a/LayoutTests/fast/regex/toString-expected.txt b/LayoutTests/fast/regex/toString-expected.txt
new file mode 100644 (file)
index 0000000..8839092
--- /dev/null
@@ -0,0 +1,22 @@
+This page tests toString conversion of RegExp objects, particularly wrt to '/' characters and RegExp.prototype.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS RegExp('/').source is "\\/"
+PASS RegExp('').source is ""
+PASS RegExp.prototype.source is ""
+PASS RegExp('/').toString() is "/\\//"
+PASS RegExp('').toString() is "/(?:)/"
+PASS RegExp.prototype.toString() is "/(?:)/"
+PASS testForwardSlash("^/$", "/"); is true
+PASS testForwardSlash("^/$", "/"); is true
+PASS testForwardSlash("^\/$", "/"); is true
+PASS testForwardSlash("^\\/$", "\/"); is true
+PASS testForwardSlash("^\\\/$", "\/"); is true
+PASS testForwardSlash("^\\\\/$", "\\/"); is true
+PASS testForwardSlash("^\\\\\/$", "\\/"); is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/regex/toString.html b/LayoutTests/fast/regex/toString.html
new file mode 100644 (file)
index 0000000..2bcfa3e
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/toString.js"></script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index f17412e..61b4ad3 100644 (file)
 
 2011-06-27  Gavin Barraclough  <barraclough@apple.com>
 
+        Reviewed by Darin Adler & Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=50554
+        RegExp.prototype.toString does not escape slashes
+
+        The problem here is that we don't escape forwards slashes when converting
+        a RegExp to a string. This means that RegExp("/").toString() is "///",
+        which is not a valid RegExp literal. Also, we return an invalid literal
+        for RegExp.prototype.toString() ("//", which is an empty single-line comment).
+
+        From ES5:
+        "NOTE: The returned String has the form of a RegularExpressionLiteral that
+        evaluates to another RegExp object with the same behaviour as this object."
+
+        * runtime/RegExpObject.cpp:
+        (JSC::regExpObjectSource):
+            - Escape forward slashes when getting the source of a RegExp.
+        * runtime/RegExpPrototype.cpp:
+        (JSC::regExpProtoFuncToString):
+            - Remove unnecessary and erroneous hack to return "//" as the string
+            representation of RegExp.prototype. This is not a valid RegExp literal
+            (it is an empty single-line comment).
+
+2011-06-27  Gavin Barraclough  <barraclough@apple.com>
+
         Reviewed by Oliver Hunt.
 
         https://bugs.webkit.org/show_bug.cgi?id=63497
index 83e7c5a..06dd0a4 100644 (file)
@@ -29,6 +29,7 @@
 #include "Lookup.h"
 #include "RegExpConstructor.h"
 #include "RegExpPrototype.h"
+#include "UStringBuilder.h"
 #include "UStringConcatenate.h"
 #include <wtf/PassOwnPtr.h>
 
@@ -111,7 +112,41 @@ JSValue regExpObjectMultiline(ExecState*, JSValue slotBase, const Identifier&)
 
 JSValue regExpObjectSource(ExecState* exec, JSValue slotBase, const Identifier&)
 {
-    return jsString(exec, asRegExpObject(slotBase)->regExp()->pattern());
+    UString pattern = asRegExpObject(slotBase)->regExp()->pattern();
+
+    size_t forwardSlashPosition = pattern.find('/');
+    if (forwardSlashPosition == notFound)
+        return jsString(exec, pattern);
+
+    // 'completed' tracks the length of original pattern already copied
+    // into the result buffer.
+    size_t completed = 0;
+    UStringBuilder result;
+
+    do {
+        // 'slashesPosition' points to the first (of possibly zero) backslash
+        // prior to the forwards slash.
+        size_t slashesPosition = forwardSlashPosition;
+        while (slashesPosition && pattern[slashesPosition - 1] == '\\')
+            --slashesPosition;
+
+        // Check whether the number of backslashes is odd or even -
+        // if odd, the forwards slash is already escaped, so we mustn't
+        // double escape it.
+        if ((forwardSlashPosition - slashesPosition) & 1)
+            result.append(pattern.substringSharingImpl(completed, forwardSlashPosition + 1));
+        else {
+            result.append(pattern.substringSharingImpl(completed, forwardSlashPosition));
+            result.append("\\/");
+        }
+        completed = forwardSlashPosition + 1;
+
+        forwardSlashPosition = pattern.find('/', completed);
+    } while (forwardSlashPosition != notFound);
+
+    // Copy in the remainder of the pattern to the buffer.
+    result.append(pattern.substringSharingImpl(completed));
+    return jsString(exec, result.toUString());
 }
 
 JSValue regExpObjectLastIndex(ExecState*, JSValue slotBase, const Identifier&)
index 26f6233..8bcf4f9 100644 (file)
@@ -136,11 +136,8 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState* exec)
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncToString(ExecState* exec)
 {
     JSValue thisValue = exec->hostThisValue();
-    if (!thisValue.inherits(&RegExpObject::s_info)) {
-        if (thisValue.inherits(&RegExpPrototype::s_info))
-            return JSValue::encode(jsNontrivialString(exec, "//"));
+    if (!thisValue.inherits(&RegExpObject::s_info))
         return throwVMTypeError(exec);
-    }
 
     RegExpObject* thisObject = asRegExpObject(thisValue);