[Merge chakra-core/ChakraCore@8a61170da3] [1.6>1.7] [MERGE #3452 @akr…

…oshg] Splice helper function should check for side-effect in the prototype. Merge pull request #3452 from akroshg:os12444598 Splice has fast path which did not check the prototype (say you have proxy) properly which resulted to incorrect result. Fixed that by using the side-effect macro to take the slower and observable code path.
nodejs · Aug 10, 2017 · 62fc612 · 62fc612
1 parent 5a30daf
commit 62fc612
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 16 deletions.
diff --git a/deps/chakrashim/core/lib/Runtime/Library/JavascriptArray.cpp b/deps/chakrashim/core/lib/Runtime/Library/JavascriptArray.cpp
@@ -6901,7 +6901,8 @@ namespace Js
         Var* insertArgs = args.Info.Count > 3 ? &args.Values[3] : nullptr;
         uint32 insertLen = args.Info.Count > 3 ? args.Info.Count - 3 : 0;
 
-        if (pArr != nullptr)
+        // Force check the prototype as we may insert values more than current elements
+        if (pArr != nullptr && !HasAnyES5ArrayInPrototypeChain(pArr, true /*forceCheckProtoChain*/))
         {
             // Since we get the length from an array and that cannot be more than uint32.
             _Analysis_assume_(length <= UINT_MAX);

diff --git a/deps/chakrashim/core/test/Array/Array_TypeConfusion_bugs.js b/deps/chakrashim/core/test/Array/Array_TypeConfusion_bugs.js
@@ -799,5 +799,19 @@ var tests = [
             assert.areEqual(16, ret[0]);
         }
     },
+    {
+        name: "splice : the method splice should get property from prototype which is a proxy",
+        body: function ()
+        {
+            var v1 = [];
+            v1.length = 20;
+            var hasCalled = 0;
+            v1.__proto__ = new Proxy([222], {has : function() { hasCalled++;} });
+            v1.push(1);
+            assert.areEqual(222, v1[0]);
+            var ret  = v1.splice(0, 10);
+            assert.areEqual(20, hasCalled);
+        }
+    },
 ];
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
diff --git a/deps/chakrashim/core/test/Array/protoLookupWithGetters.baseline b/deps/chakrashim/core/test/Array/protoLookupWithGetters.baseline
@@ -1,35 +1,35 @@
 Test case 1
-d1,d2,d3,d4,p5,p6,p7,p8,p9
+p0,p1,p2,p3,p4,p5,p6,p7,p8
 9
 p0,p1,p2,p3,p4
 5
 
 Test case 2
-d1,d2,d3,d4,d5,d6,d7,p5,p6,p7,p8,p9
+p0,p1,p2,p3,p4,p5,p6,p7,p8,p9,p8,p9
 12
 p0,p1,p2,p3,p4
 5
 
 Test case 3
-d1,d2,d3,d4,d5,d6,d7,p5,,p7,,p9
+d1,p1,d3,p3,d5,p5,d7,p7,,p9,,p9
 12
 ,p1,,p3,
 5
 
 Test case 4
-P0,P1,P2,d1,d2,d3,P8,P9
+P0,P1,P2,P3,P4,P5,P6,P7
 8
 P3,P4,P5,P6,P7
 5
 
 Test case 5
-P0,P1,P2,d1,d2,d3,P8,P9
+P0,P1,P2,P3,P4,P5,P6,P7
 8
 P3,P4,P5,P6,P7
 5
 
 Test case 6
-P0,P1,P2,d1,d2,d3,P4,P5,P6,P7,P8,P9
+P0,P1,P2,P3,P4,P5,P6,P7,P8,P9,P8,P9
 12
 P3
 1
@@ -60,13 +60,13 @@ Test case 10
 20
 a4,O6
 2
-10,a1,a1,O3,a4,a,b,a,b,a13,a13,O15,a16,a16,O18,a19
+10,a1,a1,O3,a4,a,b,a7,b,a13,a10,O15,a16,a13,O18,a19
 16
 a7,a7,O9,a10,a10,O12
 6
-10,a1,a1,O3,a4,a,b,a,b,a13,a,b,c,e,f,a16,a16,O18,a19
+10,a1,a1,O3,a4,a,b,a7,b,a13,a10,b,c,a13,f,a16,a16,O18,O18
 19
-a13,O15
+a10,O15
 2
 
 Test case 11

diff --git a/deps/chakrashim/core/test/Array/protoLookupWithGetters.js b/deps/chakrashim/core/test/Array/protoLookupWithGetters.js
@@ -12,7 +12,7 @@ write("Test case 1");
 
 for(var i =0;i<10;i++)
 {
-  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
 }
 
 var arr=new Array(10);
@@ -33,7 +33,7 @@ write("Test case 2");
 
 for(var i =0;i<10;i++)
 {
-  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
 }
 
 var arr=new Array(10);
@@ -54,7 +54,7 @@ write("Test case 3");
 for(var i =0;i<10;i++)
 {
   i++;
-  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, i, { get: function (i) { return function () { return "p"+i; } }(i), set : function(a) {}, configurable: true, enumerable: true });
 }
 
 var arr=new Array(10);
@@ -73,7 +73,7 @@ write("");
 write("Test case 4");
 for(var k=0;k<10;k++)
 {
-  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
 }
 var arr=new Array(10);
 var newarr=arr.splice(3,5,"d1","d2","d3")
@@ -90,7 +90,7 @@ write("");
 write("Test case 5");
 for(var k=0;k<10;k++)
 {
-  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
 }
 var arr=new Array(10);
 var newarr=arr.splice(3,5,"d1","d2","d3")
@@ -108,7 +108,7 @@ write("");
 write("Test case 6");
 for(var k=0;k<10;k++)
 {
-  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), configurable: true, enumerable: true });
+  Object.defineProperty(Array.prototype, k, { get: function (k) { return function () { return "P"+k; } }(k), set : function(a) {}, configurable: true, enumerable: true });
 }
 var arr=new Array(10);
 var newarr=arr.splice(3,1,"d1","d2","d3")