8013131: Various compatibility issues in String.prototype.split()

Reviewed-by: lagergren, jlaskey

nashorn/src/jdk/nashorn/internal/objects/NativeJSON.java

@@ -229,7 +229,7 @@ public final class NativeJSON extends ScriptObject {
         final JSType type = JSType.of(value);
         if (type == JSType.OBJECT) {
             if (isArray(value)) {
-                return JA((NativeArray)value, state);
+                return JA((ScriptObject)value, state);
             } else if (value instanceof ScriptObject) {
                 return JO((ScriptObject)value, state);
             }
@@ -315,7 +315,7 @@ public final class NativeJSON extends ScriptObject {
     }
 
     // Spec: The abstract operation JA(value) serializes an array.
-    private static Object JA(final NativeArray value, final StringifyState state) {
+    private static Object JA(final ScriptObject value, final StringifyState state) {
         if (state.stack.containsKey(value)) {
             throw typeError("JSON.stringify.cyclic");
         }

nashorn/src/jdk/nashorn/internal/objects/NativeRegExp.java

@@ -523,23 +523,28 @@ public final class NativeRegExp extends ScriptObject {
     }
 
     private RegExpResult execInner(final String string) {
+        final boolean isGlobal = regexp.isGlobal();
         int start = getLastIndex();
-        if (! regexp.isGlobal()) {
+        if (!isGlobal) {
             start = 0;
         }
 
         if (start < 0 || start > string.length()) {
-            setLastIndex(0);
+            if (isGlobal) {
+                setLastIndex(0);
+            }
             return null;
         }
 
         final RegExpMatcher matcher = regexp.match(string);
         if (matcher == null || !matcher.search(start)) {
-            setLastIndex(0);
+            if (isGlobal) {
+                setLastIndex(0);
+            }
             return null;
         }
 
-        if (regexp.isGlobal()) {
+        if (isGlobal) {
             setLastIndex(matcher.end());
         }
 
@@ -548,6 +553,22 @@ public final class NativeRegExp extends ScriptObject {
         return match;
     }
 
+    // String.prototype.split method ignores the global flag and should not update lastIndex property.
+    private RegExpResult execSplit(final String string, int start) {
+        if (start < 0 || start > string.length()) {
+            return null;
+        }
+
+        final RegExpMatcher matcher = regexp.match(string);
+        if (matcher == null || !matcher.search(start)) {
+            return null;
+        }
+
+        final RegExpResult match = new RegExpResult(string, matcher.start(), groups(matcher));
+        globalObject.setLastRegExpResult(match);
+        return match;
+    }
+
     /**
      * Convert java.util.regex.Matcher groups to JavaScript groups.
      * That is, replace null and groups that didn't match with undefined.
@@ -600,7 +621,7 @@ public final class NativeRegExp extends ScriptObject {
      * @return True if a match is found.
      */
     public Object test(final String string) {
-        return exec(string) != null;
+        return execInner(string) != null;
     }
 
     /**
@@ -765,35 +786,31 @@ public final class NativeRegExp extends ScriptObject {
      * @return Array of substrings.
      */
     Object split(final String string, final long limit) {
-        return split(this, string, limit);
-    }
-
-    private static Object split(final NativeRegExp regexp0, final String input, final long limit) {
-        final List<Object> matches = new ArrayList<>();
-
-        final NativeRegExp regexp = new NativeRegExp(regexp0);
-        regexp.setGlobal(true);
-
         if (limit == 0L) {
             return new NativeArray();
         }
 
+        final List<Object> matches = new ArrayList<>();
+
         RegExpResult match;
-        final int inputLength = input.length();
+        final int inputLength = string.length();
         int lastLength = -1;
+        int lastIndex = 0;
         int lastLastIndex = 0;
 
-        while ((match = regexp.execInner(input)) != null) {
+        while ((match = execSplit(string, lastIndex)) != null) {
-            final int lastIndex = match.getIndex() + match.length();
+            lastIndex = match.getIndex() + match.length();
 
             if (lastIndex > lastLastIndex) {
-                matches.add(input.substring(lastLastIndex, match.getIndex()));
+                matches.add(string.substring(lastLastIndex, match.getIndex()));
-                if (match.getGroups().length > 1 && match.getIndex() < inputLength) {
+                final Object[] groups = match.getGroups();
-                    matches.addAll(Arrays.asList(match.getGroups()).subList(1, match.getGroups().length));
+                if (groups.length > 1 && match.getIndex() < inputLength) {
+                    for (int index = 1; index < groups.length && matches.size() < limit; index++) {
+                        matches.add(groups[index]);
+                    }
                 }
 
                 lastLength = match.length();
-                lastLastIndex = lastIndex;
 
                 if (matches.size() >= limit) {
                     break;
@@ -801,8 +818,10 @@ public final class NativeRegExp extends ScriptObject {
             }
 
             // bump the index to avoid infinite loop
-            if (regexp.getLastIndex() == match.getIndex()) {
+            if (lastIndex == lastLastIndex) {
-                regexp.setLastIndex(match.getIndex() + 1);
+                lastIndex++;
+            } else {
+                lastLastIndex = lastIndex;
             }
         }
 
@@ -810,12 +829,12 @@ public final class NativeRegExp extends ScriptObject {
             // check special case if we need to append an empty string at the
             // end of the match
             // if the lastIndex was the entire string
-            if (lastLastIndex == input.length()) {
+            if (lastLastIndex == string.length()) {
-                if (lastLength > 0 || regexp.test("") == Boolean.FALSE) {
+                if (lastLength > 0 || execSplit("", 0) == null) {
                     matches.add("");
                 }
             } else {
-                matches.add(input.substring(lastLastIndex, inputLength));
+                matches.add(string.substring(lastLastIndex, inputLength));
             }
         }
 

nashorn/src/jdk/nashorn/internal/objects/NativeRegExpExecResult.java

@@ -51,6 +51,7 @@ public final class NativeRegExpExecResult extends ScriptObject {
 
     NativeRegExpExecResult(final RegExpResult result) {
         setProto(Global.instance().getArrayPrototype());
+        setIsArray();
         this.setArray(ArrayData.allocate(result.getGroups().clone()));
         this.index = result.getIndex();
         this.input = result.getInput();

nashorn/src/jdk/nashorn/internal/objects/NativeString.java

@@ -841,7 +841,7 @@ public final class NativeString extends ScriptObject {
         final long lim = (limit == UNDEFINED) ? JSType.MAX_UINT : JSType.toUint32(limit);
 
         if (separator == UNDEFINED) {
-            return new NativeArray(new Object[]{str});
+            return limit == 0 ? new NativeArray() : new NativeArray(new Object[]{str});
         }
 
         if (separator instanceof NativeRegExp) {
@@ -854,8 +854,9 @@ public final class NativeString extends ScriptObject {
 
     private static Object splitString(String str, String separator, long limit) {
         if (separator.isEmpty()) {
-            final Object[] array = new Object[str.length()];
+            final int length = (int) Math.min(str.length(), limit);
-            for (int i = 0; i < array.length; i++) {
+            final Object[] array = new Object[length];
+            for (int i = 0; i < length; i++) {
                 array[i] = String.valueOf(str.charAt(i));
             }
             return new NativeArray(array);

nashorn/test/script/basic/JDK-8013131.js

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2010, 2013, 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-8013131: Various compatibility issues in String.prototype.split()
+ *
+ * @test
+ * @run
+ */
+
+
+// Make sure limit is honored with undefined/empty separator
+print(JSON.stringify("aa".split(undefined, 0)));
+print(JSON.stringify("abc".split("", 1)));
+
+// Make sure limit is honored with capture groups
+print(JSON.stringify("aa".split(/(a)/, 1)));
+print(JSON.stringify("aa".split(/(a)/, 2)));
+print(JSON.stringify("aa".split(/((a))/, 1)));
+print(JSON.stringify("aa".split(/((a))/, 2)));
+
+// Empty capture group at end of string should be ignored
+print(JSON.stringify("aaa".split(/((?:))/)));
+
+// Tests below are to make sure that split does not read or write lastIndex property
+var r = /a/;
+r.lastIndex = {
+    valueOf: function(){throw 2}
+};
+print(JSON.stringify("aa".split(r)));
+
+r = /a/g;
+r.lastIndex = 100;
+print(JSON.stringify("aa".split(r)));
+print(r.lastIndex);
+
+r = /((?:))/g;
+r.lastIndex = 100;
+print(JSON.stringify("aaa".split(r)));
+print(r.lastIndex);
+
+// Make sure lastIndex is not updated on non-global regexp
+r = /a/;
+r.lastIndex = 100;
+print(JSON.stringify(r.exec("aaa")));
+print(r.lastIndex);

nashorn/test/script/basic/JDK-8013131.js.EXPECTED

@@ -0,0 +1,14 @@
+[]
+["a"]
+[""]
+["","a"]
+[""]
+["","a"]
+["a","","a","","a"]
+["","",""]
+["","",""]
+100
+["a","","a","","a"]
+100
+["a"]
+100