8016542: String.prototype.replace called with function argument should not replace $ patterns
Reviewed-by: lagergren, jlaskey
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeRegExp.java Thu Jun 13 15:26:49 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeRegExp.java Thu Jun 13 20:50:24 2013 +0200
@@ -641,26 +641,19 @@
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 @@
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 @@
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 @@
}
// 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) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8016542.js Thu Jun 13 20:50:24 2013 +0200
@@ -0,0 +1,37 @@
+/*
+ * 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 "$&"}));
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8016542.js.EXPECTED Thu Jun 13 20:50:24 2013 +0200
@@ -0,0 +1,6 @@
+abc
+abc
+abc
+$&bc
+a$&c
+ab$&