[MERGE #5064 @xiaoyinl] JavascriptString::ToDouble should return NaN for "\x00" (fixes #5038)

Merge pull request #5064 from xiaoyinl:nullstring_Number

The bug is in JavascriptString::ToDouble. After skipping whitespace characters, the function determines if the string is empty by checking if `pch` points to \0. This bug makes the function incorrectly return 0 for string "\x00". It should check the length, instead of null terminator.

Fixes #5038

lib/Runtime/Library/JavascriptString.cpp

@@ -3006,7 +3006,7 @@ case_2:
         // TODO: Use GetString here instead of GetSz (need to modify DblFromHex and StrToDbl to take a length)
         for (pch = this->GetSz(); IsWhiteSpaceCharacter(*pch); pch++)
             ;
-        if (0 == *pch)
+        if (pch == this->m_pszValue + len)
         {
             *result = 0;
             return true;

test/Strings/null_embedded_string_toDouble.js

@@ -0,0 +1,38 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+// Number tests for issue https://github.com/Microsoft/ChakraCore/issues/5038
+
+if (this.WScript && this.WScript.LoadScriptFile) { // Check for running in ch
+    this.WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+}
+
+let tests =
+[
+    {
+        name: 'Converting "\\x00" to Number should return NaN',
+        body: function () {
+            let arr_tests = [Number("\x00"), Number(" \x00"), -" \x00", +"\x00", +"  \n\x00"];
+            arr_tests.forEach(function (num, index) {
+                assert.areEqual("number", typeof (num), "Element at index " + index + " has wrong type.");
+                assert.isTrue(Number.isNaN(num), "Element at index " + index + " is " + num + ". It should be NaN.");
+            });
+        }
+    },
+    {
+        name: "Converting empty strings or whitespace-only strings to Number should return 0",
+        body: function () {
+            let arr_tests = [Number(""), Number(" "), +"", -"", +"  ",
+                +"\t\n\r\v\f\xa0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff"];
+
+            arr_tests.forEach(function (num, index) {
+                assert.areEqual("number", typeof (num), "Element at index " + index + " has wrong type.");
+                assert.areEqual(0, num, "Element at index " + index + " is " + num + ". It should be 0.");
+            });
+        }
+    }
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

test/Strings/rlexe.xml

@@ -251,4 +251,10 @@
       <tags>exclude_debug,Slow</tags>
     </default>
   </test>  -->
+  <test>
+    <default>
+      <files>null_embedded_string_toDouble.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+    </default>
+  </test>
 </regress-exe>