[Merge chakra-core/ChakraCore@4f6b659a41] [MERGE #3857 @jackhorton] F…

…ix issue with calling JSON.stringify on an object with internal properties Merge pull request #3857 from jackhorton:stringify-internal-props
nodejs · Oct 5, 2017 · 4f7c99c · 4f7c99c
1 parent 8fc6067
commit 4f7c99c
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/deps/chakrashim/core/lib/Runtime/Library/JSON.cpp b/deps/chakrashim/core/lib/Runtime/Library/JSON.cpp
@@ -681,9 +681,9 @@ namespace JSON
                 Js::JavascriptStaticEnumerator enumerator;
                 if (object->GetEnumerator(&enumerator, EnumeratorFlags::SnapShotSemantics | EnumeratorFlags::EphemeralReference, scriptContext))
                 {
-                      Js::DynamicObject * dynamicObject = ((Js::DynamicObject*)object);
-                      if (ReplacerFunction != replacerType || !dynamicObject->HasObjectArray())
-                      {
+                    Js::DynamicObject * dynamicObject = ((Js::DynamicObject*)object);
+                    if (ReplacerFunction != replacerType || !dynamicObject->HasObjectArray())
+                    {
                         uint32 propertyCount = dynamicObject->GetPropertyCount();
 
                         result = Js::ConcatStringBuilder::New(this->scriptContext, propertyCount);
@@ -713,7 +713,14 @@ namespace JSON
                         for (uint32 i = 0; i < propertyCount; i++)
                         {
                             id = typeHandler->GetPropertyId(scriptContext, (Js::PropertyId)i);
-                            if (id == Js::Constants::NoProperty)
+
+                            if (id < Js::InternalPropertyIds::Count)
+                            {
+                                // if the property ID is internal, we cannot assume dynamicObject->getSlot(i) is a var later
+                                previousId = id;
+                                continue;
+                            }
+                            else if (id == Js::Constants::NoProperty)
                             {
                                 if ((propertyName = enumerator.MoveAndGetNext(previousId)) == NULL) break;
 

diff --git a/deps/chakrashim/core/test/JSON/stringifyobjects.js b/deps/chakrashim/core/test/JSON/stringifyobjects.js
@@ -0,0 +1,20 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+if (this.WScript && this.WScript.LoadScriptFile) { // Check for running in ch
+    this.WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+}
+
+const tests = [
+    {
+        name: "Stringify error",
+        body: function () {
+            const err = new Error("message");
+            assert.areEqual('{"description":"message","message":"message"}', JSON.stringify(err));
+        }
+    }
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });