REGRESSION (r183373): ASSERT failed in wtf/SHA1.h
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 17:44:53 +0000 (17:44 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 17:44:53 +0000 (17:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144257

Reviewed by Darin Adler.

Source/JavaScriptCore:

SHA1 is used to calculate CodeBlockHash.
To calculate hash value, we pass the source code UTF-8 CString to SHA1::addBytes.
However, the source code can contain null character.
So when performing `strlen` on the source code's CString, it returns the incorrect length.
In SHA1::addBytes, there's assertion `input.length() == strlen(string)` and it fails.

In the template-literal-syntax.js, we perform `eval` with the script contains "\0".
As the result, `strlen(string)` accidentally shortened by the contained "\0", and assertion fails.

CString will be changed not to contain a null-character[1]. However, inserting the assertion here
is not correct. Because

1. If CString should not contain a null character, this should be asserted in CString side instead of SHA1::addBytes.
2. If CString can contain a null character, this assertion becomes incorrect.

So this patch just drops the assertion.

In the current implementation, we once convert the entire source code to the newly allocated
UTF-8 string and pass it to the SHA1 processing. However, this is memory consuming.
Ideally, we should stream the decoded bytes into the SHA1 processing iteratively.
We'll implement it in the separate patch[2].

[1]: https://bugs.webkit.org/show_bug.cgi?id=144339
[2]: https://bugs.webkit.org/show_bug.cgi?id=144263

* tests/stress/eval-script-contains-null-character.js: Added.
(shouldBe):
(test):
* tests/stress/template-literal-line-terminators.js:
* tests/stress/template-literal-syntax.js:
* tests/stress/template-literal.js:

Source/WTF:

* wtf/SHA1.h:
(WTF::SHA1::addBytes):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/tests/stress/eval-script-contains-null-character.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/template-literal-line-terminators.js
Source/JavaScriptCore/tests/stress/template-literal-syntax.js
Source/JavaScriptCore/tests/stress/template-literal.js
Source/WTF/ChangeLog
Source/WTF/wtf/SHA1.h

index 0deb738..f56e11c 100644 (file)
@@ -1,3 +1,42 @@
+2015-04-29  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        REGRESSION (r183373): ASSERT failed in wtf/SHA1.h
+        https://bugs.webkit.org/show_bug.cgi?id=144257
+
+        Reviewed by Darin Adler.
+
+        SHA1 is used to calculate CodeBlockHash.
+        To calculate hash value, we pass the source code UTF-8 CString to SHA1::addBytes.
+        However, the source code can contain null character.
+        So when performing `strlen` on the source code's CString, it returns the incorrect length.
+        In SHA1::addBytes, there's assertion `input.length() == strlen(string)` and it fails.
+
+        In the template-literal-syntax.js, we perform `eval` with the script contains "\0".
+        As the result, `strlen(string)` accidentally shortened by the contained "\0", and assertion fails.
+
+        CString will be changed not to contain a null-character[1]. However, inserting the assertion here
+        is not correct. Because
+
+        1. If CString should not contain a null character, this should be asserted in CString side instead of SHA1::addBytes.
+        2. If CString can contain a null character, this assertion becomes incorrect.
+
+        So this patch just drops the assertion.
+
+        In the current implementation, we once convert the entire source code to the newly allocated
+        UTF-8 string and pass it to the SHA1 processing. However, this is memory consuming.
+        Ideally, we should stream the decoded bytes into the SHA1 processing iteratively.
+        We'll implement it in the separate patch[2].
+
+        [1]: https://bugs.webkit.org/show_bug.cgi?id=144339
+        [2]: https://bugs.webkit.org/show_bug.cgi?id=144263
+
+        * tests/stress/eval-script-contains-null-character.js: Added.
+        (shouldBe):
+        (test):
+        * tests/stress/template-literal-line-terminators.js:
+        * tests/stress/template-literal-syntax.js:
+        * tests/stress/template-literal.js:
+
 2015-04-29  Filip Pizlo  <fpizlo@apple.com>
 
         Evict IsEnvironmentRecord from inline type flags
diff --git a/Source/JavaScriptCore/tests/stress/eval-script-contains-null-character.js b/Source/JavaScriptCore/tests/stress/eval-script-contains-null-character.js
new file mode 100644 (file)
index 0000000..419215d
--- /dev/null
@@ -0,0 +1,13 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error("bad value: " + actual);
+}
+
+function test() {
+    shouldBe(eval("(`\0`)"), "\0");
+    shouldBe(eval("('\0')"), "\0");
+}
+noInline(test);
+
+for (var i = 0; i < 10000; ++i)
+    test();
index 8aeeadc..eb9561c 100644 (file)
@@ -1,3 +1,13 @@
+2015-04-29  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        REGRESSION (r183373): ASSERT failed in wtf/SHA1.h
+        https://bugs.webkit.org/show_bug.cgi?id=144257
+
+        Reviewed by Darin Adler.
+
+        * wtf/SHA1.h:
+        (WTF::SHA1::addBytes):
+
 2015-04-29  Martin Robinson  <mrobinson@igalia.com>
 
         [CMake] [GTK] Organize and clean up unused CMake variables
index 63365cf..009b484 100644 (file)
@@ -52,11 +52,6 @@ public:
     void addBytes(const CString& input)
     {
         const char* string = input.data();
-        // Make sure that the creator of the CString didn't make the mistake
-        // of forcing length() to be the size of the buffer used to create the
-        // string, prior to inserting the null terminator earlier in the
-        // sequence.
-        ASSERT(input.length() == strlen(string));
         addBytes(reinterpret_cast<const uint8_t*>(string), input.length());
     }
     WTF_EXPORT_PRIVATE void addBytes(const uint8_t* input, size_t length);