8016542: String.prototype.replace called with function argument should not replace $ patterns

Reviewed-by: lagergren, jlaskey

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

@@ -641,26 +641,19 @@ public final class NativeRegExp extends ScriptObject {
             return string;
         }
 
-        /*
-         * $$ -> $
-         * $& -> the matched substring
-         * $` -> the portion of string that preceeds matched substring
-         * $' -> the portion of string that follows the matched substring
-         * $n -> the nth capture, where n is [1-9] and $n is NOT followed by a decimal digit
-         * $nn -> the nnth capture, where nn is a two digit decimal number [01-99].
-         */
-        String replace = replacement;
-
         if (!regexp.isGlobal()) {
             if (!matcher.search(0)) {
                 return string;
             }
 
             final StringBuilder sb = new StringBuilder();
+            sb.append(string, 0, matcher.start());
+
             if (function != null) {
-                replace = callReplaceValue(function, matcher, string);
+                sb.append(callReplaceValue(function, matcher, string));
+            } else {
+                appendReplacement(matcher, string, replacement, sb);
             }
-            appendReplacement(matcher, string, replace, sb, 0);
             sb.append(string, matcher.end(), string.length());
             return sb.toString();
         }
@@ -676,12 +669,13 @@ public final class NativeRegExp extends ScriptObject {
         final StringBuilder sb = new StringBuilder();
 
         do {
+            sb.append(string, thisIndex, matcher.start());
             if (function != null) {
-                replace = callReplaceValue(function, matcher, string);
+                sb.append(callReplaceValue(function, matcher, string));
+            } else {
+                appendReplacement(matcher, string, replacement, sb);
             }
 
-            appendReplacement(matcher, string, replace, sb, thisIndex);
-
             // ECMA 15.5.4.10 String.prototype.match(regexp)
             thisIndex = matcher.end();
             if (thisIndex == previousLastIndex) {
@@ -697,10 +691,19 @@ public final class NativeRegExp extends ScriptObject {
         return sb.toString();
     }
 
-    private void appendReplacement(final RegExpMatcher matcher, final String text, final String replacement, final StringBuilder sb, final int lastAppendPosition) {
-        // Process substitution string to replace group references with groups
+    private void appendReplacement(final RegExpMatcher matcher, final String text, final String replacement, final StringBuilder sb) {
+        /*
+         * Process substitution patterns:
+         *
+         * $$ -> $
+         * $& -> the matched substring
+         * $` -> the portion of string that preceeds matched substring
+         * $' -> the portion of string that follows the matched substring
+         * $n -> the nth capture, where n is [1-9] and $n is NOT followed by a decimal digit
+         * $nn -> the nnth capture, where nn is a two digit decimal number [01-99].
+         */
+
         int cursor = 0;
-        final StringBuilder result = new StringBuilder();
         Object[] groups = null;
 
         while (cursor < replacement.length()) {
@@ -732,37 +735,33 @@ public final class NativeRegExp extends ScriptObject {
                         }
                         // Append group if matched.
                         if (groups[refNum] != UNDEFINED) {
-                            result.append((String) groups[refNum]);
+                            sb.append((String) groups[refNum]);
                         }
                     } else { // $0. ignore.
                         assert refNum == 0;
-                        result.append("$0");
+                        sb.append("$0");
                     }
                 } else if (nextChar == '$') {
-                    result.append('$');
+                    sb.append('$');
                     cursor++;
                 } else if (nextChar == '&') {
-                    result.append(matcher.group());
+                    sb.append(matcher.group());
                     cursor++;
                 } else if (nextChar == '`') {
-                    result.append(text.substring(0, matcher.start()));
+                    sb.append(text, 0, matcher.start());
                     cursor++;
                 } else if (nextChar == '\'') {
-                    result.append(text.substring(matcher.end()));
+                    sb.append(text, matcher.end(), text.length());
                     cursor++;
                 } else {
                     // unknown substitution or $n with n>m. skip.
-                    result.append('$');
+                    sb.append('$');
                 }
             } else {
-                result.append(nextChar);
+                sb.append(nextChar);
                 cursor++;
             }
         }
-        // Append the intervening text
-        sb.append(text, lastAppendPosition, matcher.start());
-        // Append the match substitution
-        sb.append(result);
     }
 
     private String callReplaceValue(final ScriptFunction function, final RegExpMatcher matcher, final String string) {

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

+/*
+ * 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-8016542: String.prototype.replace called with function argument should not replace $ patterns
+ *
+ * @test
+ * @run
+ */
+
+print("abc".replace("a", "$&"));
+print("abc".replace("b", "$&"));
+print("abc".replace("c", "$&"));
+
+print("abc".replace("a", function(){return "$&"}));
+print("abc".replace("b", function(){return "$&"}));
+print("abc".replace("c", function(){return "$&"}));

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

+abc
+abc
+abc
+$&bc
+a$&c
+ab$&