8068872: Nashorn JSON.parse drops numeric keys
authorhannesw
Thu, 05 Feb 2015 14:47:28 +0100
changeset 28787 cab4b0497d27
parent 28786 679fd2d26470
child 28788 51c662748340
8068872: Nashorn JSON.parse drops numeric keys Reviewed-by: attila, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ContinuousArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/IntArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/LongArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/NumberArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ObjectArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java
nashorn/test/script/basic/JDK-8068872.js
nashorn/test/script/basic/JDK-8068872.js.EXPECTED
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java	Thu Feb 05 14:47:28 2015 +0100
@@ -120,9 +120,6 @@
         this(ArrayData.allocate(array.length));
 
         ArrayData arrayData = this.getArray();
-        if (array.length > 0) {
-            arrayData.ensure(array.length - 1);
-        }
 
         for (int index = 0; index < array.length; index++) {
             final Object value = array[index];
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Feb 05 14:47:28 2015 +0100
@@ -722,8 +722,12 @@
     public void defineOwnProperty(final int index, final Object value) {
         assert isValidArrayIndex(index) : "invalid array index";
         final long longIndex = ArrayIndex.toLongIndex(index);
-        doesNotHaveEnsureDelete(longIndex, getArray().length(), false);
-        setArray(getArray().ensure(longIndex).set(index,value, false));
+        final long oldLength = getArray().length();
+        if (longIndex >= oldLength) {
+            setArray(getArray().ensure(longIndex));
+            doesNotHaveEnsureDelete(longIndex, oldLength, false);
+        }
+        setArray(getArray().set(index,value, false));
     }
 
     private void checkIntegerKey(final String key) {
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -118,7 +118,7 @@
                     return new SparseArrayData(this, safeIndex + 1);
                 }
                 //known to fit in int
-                return toRealArrayData((int)safeIndex).ensure(safeIndex);
+                return toRealArrayData((int)safeIndex);
            }
            return this;
         }
@@ -497,7 +497,9 @@
     public abstract ArrayData shiftRight(final int by);
 
     /**
-     * Ensure that the given index exists and won't fail subsequent
+     * Ensure that the given index exists and won't fail in a subsequent access.
+     * If {@code safeIndex} is equal or greater than the current length the length is
+     * updated to {@code safeIndex + 1}.
      *
      * @param safeIndex the index to ensure wont go out of bounds
      * @return new array data (or same)
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ContinuousArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ContinuousArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -57,7 +57,7 @@
     }
 
     /**
-     * Check if we can put one more element at the end of this continous
+     * Check if we can put one more element at the end of this continuous
      * array without reallocating, or if we are overwriting an already
      * allocated element
      *
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/IntArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/IntArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -221,7 +221,9 @@
             final int newLength = ArrayData.nextSize((int)safeIndex);
             array = Arrays.copyOf(array, newLength);
         }
-        setLength(safeIndex + 1);
+        if (safeIndex >= length()) {
+            setLength(safeIndex + 1);
+        }
         return this;
     }
 
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/LongArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/LongArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -157,7 +157,9 @@
             final int newLength = ArrayData.nextSize((int)safeIndex);
             array = Arrays.copyOf(array, newLength);
         }
-        setLength(safeIndex + 1);
+        if (safeIndex >= length()) {
+            setLength(safeIndex + 1);
+        }
         return this;
     }
 
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/NumberArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/NumberArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -139,7 +139,9 @@
             final int newLength = ArrayData.nextSize((int)safeIndex);
             array = Arrays.copyOf(array, newLength); //todo fill with nan or never accessed?
         }
-        setLength(safeIndex + 1);
+        if (safeIndex >= length()) {
+            setLength(safeIndex + 1);
+        }
         return this;
 
     }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ObjectArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ObjectArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -123,7 +123,9 @@
             final int newLength = ArrayData.nextSize((int)safeIndex);
             array = Arrays.copyOf(array, newLength); //fill with undefined or OK? TODO
         }
-        setLength(safeIndex + 1);
+        if (safeIndex >= length()) {
+            setLength(safeIndex + 1);
+        }
         return this;
     }
 
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java	Thu Feb 05 14:42:14 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java	Thu Feb 05 14:47:28 2015 +0100
@@ -135,10 +135,17 @@
 
     @Override
     public ArrayData ensure(final long safeIndex) {
+        // Usually #ensure only needs to be called if safeIndex is greater or equal current length.
+        // SparseArrayData is an exception as an index smaller than our current length may still
+        // exceed the underlying ArrayData's capacity. Because of this, SparseArrayData invokes
+        // its ensure method internally in various places where other ArrayData subclasses don't,
+        // making it safe for outside uses to only call ensure(safeIndex) if safeIndex >= length.
         if (safeIndex < maxDenseLength && underlying.length() <= safeIndex) {
             underlying = underlying.ensure(safeIndex);
         }
-        setLength(Math.max(safeIndex + 1, length()));
+        if (safeIndex >= length()) {
+            setLength(safeIndex + 1);
+        }
         return this;
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8068872.js	Thu Feb 05 14:47:28 2015 +0100
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8068872:  Nashorn JSON.parse drops numeric keys
+ *
+ * @test
+ * @run
+ */
+
+print(JSON.stringify(JSON.parse('{"3": 1, "5": "a"}')));
+print(JSON.stringify(JSON.parse('{"5": 1, "3": "a"}')));
+print(JSON.stringify(JSON.parse('{"0": 1, "4294967294": "a"}')));
+print(JSON.stringify(JSON.parse('{"4294967294": 1, "0": "a"}')));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8068872.js.EXPECTED	Thu Feb 05 14:47:28 2015 +0100
@@ -0,0 +1,4 @@
+{"3":1,"5":"a"}
+{"3":"a","5":1}
+{"0":1,"4294967294":"a"}
+{"0":"a","4294967294":1}