From fb8a0daae8aa8a4c30a9861cb338eb4d2650e65e Mon Sep 17 00:00:00 2001 From: Peter Shipton Date: Tue, 23 Jul 2024 08:46:59 -0400 Subject: [PATCH] String.replaceAll() fast path must check regex is compressed Also, return the original String in the fast paths when nothing is replaced. Issue https://github.com/eclipse-openj9/openj9/issues/19903 Signed-off-by: Peter Shipton --- .../share/classes/java/lang/String.java | 50 ++++++++++++++++--- .../openj9/test/java/lang/Test_String.java | 14 ++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/jcl/src/java.base/share/classes/java/lang/String.java b/jcl/src/java.base/share/classes/java/lang/String.java index 356d161d3a1..25ac5ae0b0d 100644 --- a/jcl/src/java.base/share/classes/java/lang/String.java +++ b/jcl/src/java.base/share/classes/java/lang/String.java @@ -3202,6 +3202,9 @@ public String replaceAll(String regex, String substitute) { int length = lengthInternal(); if (substituteLength < 2) { if (COMPACT_STRINGS && isCompressed() && (substituteLength == 0 || substitute.isCompressed())) { + if (!regex.isCompressed()) { + return this; + } byte[] newChars = new byte[length]; byte toReplace = helpers.getByteFromArrayByIndex(regex.value, 0); byte replacement = (byte)0; // assign dummy value that isn't used @@ -3210,14 +3213,21 @@ public String replaceAll(String regex, String substitute) { checkLastChar((char)replacement); } int newCharIndex = 0; + boolean replaced = false; for (int i = 0; i < length; ++i) { byte current = helpers.getByteFromArrayByIndex(value, i); if (current != toReplace) { helpers.putByteInArrayByIndex(newChars, newCharIndex++, current); - } else if (substituteLength == 1) { - helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement); + } else { + replaced = true; + if (substituteLength == 1) { + helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement); + } } } + if (!replaced) { + return this; + } return new String(newChars, 0, newCharIndex, true); } else if (!COMPACT_STRINGS || !isCompressed()) { byte[] newChars = StringUTF16.newBytesFor(length); @@ -3228,14 +3238,21 @@ public String replaceAll(String regex, String substitute) { checkLastChar(replacement); } int newCharIndex = 0; + boolean replaced = false; for (int i = 0; i < length; ++i) { char current = helpers.getCharFromArrayByIndex(value, i); if (current != toReplace) { helpers.putCharInArrayByIndex(newChars, newCharIndex++, current); - } else if (substituteLength == 1) { - helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement); + } else { + replaced = true; + if (substituteLength == 1) { + helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement); + } } } + if (!replaced) { + return this; + } if (replacement > 255) { // If the original String isn't compressed and the replacement character isn't Latin1, the result is uncompressed. return new String(newChars, UTF16); @@ -7601,6 +7618,9 @@ public String replaceAll(String regex, String substitute) { int length = lengthInternal(); if (substituteLength < 2) { if (COMPACT_STRINGS && isCompressed() && (substituteLength == 0 || substitute.isCompressed())) { + if (!regex.isCompressed()) { + return this; + } char[] newChars = new char[(length + 1) >>> 1]; byte toReplace = helpers.getByteFromArrayByIndex(regex.value, 0); byte replacement = (byte)-1; // assign dummy value that will never be used @@ -7609,14 +7629,21 @@ public String replaceAll(String regex, String substitute) { checkLastChar((char)replacement); } int newCharIndex = 0; + boolean replaced = false; for (int i = 0; i < length; ++i) { byte current = helpers.getByteFromArrayByIndex(value, i); if (current != toReplace) { helpers.putByteInArrayByIndex(newChars, newCharIndex++, current); - } else if (substituteLength == 1) { - helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement); + } else { + replaced = true; + if (substituteLength == 1) { + helpers.putByteInArrayByIndex(newChars, newCharIndex++, replacement); + } } } + if (!replaced) { + return this; + } return new String(newChars, 0, newCharIndex, true); } else if (!COMPACT_STRINGS || !isCompressed()) { char[] newChars = new char[length]; @@ -7627,14 +7654,21 @@ public String replaceAll(String regex, String substitute) { checkLastChar(replacement); } int newCharIndex = 0; + boolean replaced = false; for (int i = 0; i < length; ++i) { char current = helpers.getCharFromArrayByIndex(value, i); if (current != toReplace) { helpers.putCharInArrayByIndex(newChars, newCharIndex++, current); - } else if (substituteLength == 1) { - helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement); + } else { + replaced = true; + if (substituteLength == 1) { + helpers.putCharInArrayByIndex(newChars, newCharIndex++, replacement); + } } } + if (!replaced) { + return this; + } return new String(newChars, 0, newCharIndex, false); } } diff --git a/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java b/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java index b275b56eb34..05432a6589a 100644 --- a/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java +++ b/test/functional/Java8andUp/src/org/openj9/test/java/lang/Test_String.java @@ -1860,6 +1860,19 @@ public void test_join2() { } } + /** + * @tests java.lang.String#replaceAll(String, String) + */ + @Test + public void test_replaceAll() { + String replace1 = "1a2a3a\u0011"; + String result1 = replace1.replaceAll("\u1161", "["); + AssertJUnit.assertSame("replaceAll() compact result should be identical", replace1, result1); + String replace2 = "1a2b3c\u1161"; + String result2 = replace2.replaceAll("\u1162", "\u1234"); + AssertJUnit.assertSame("replaceAll() non-compact result should be identical", replace2, result2); + } + /** * @tests java.lang.String#replaceAll(String, String) */ @@ -1872,6 +1885,7 @@ public void test_replaceAll_last_char_dollarsign() { // expected } } + /** * @tests java.lang.String#replaceAll(String, String) */