Check bounds arguments in String#substring ... and various API depending on it PiperOrigin-RevId: 309475943 Change-Id: I54785fe304787ced52c64dc3827977bf992b5324
diff --git a/user/super/com/google/gwt/emul/java/io/Writer.java b/user/super/com/google/gwt/emul/java/io/Writer.java index a2e34e2..d837120 100644 --- a/user/super/com/google/gwt/emul/java/io/Writer.java +++ b/user/super/com/google/gwt/emul/java/io/Writer.java
@@ -15,7 +15,6 @@ */ package java.io; -import static javaemul.internal.InternalPreconditions.checkCriticalStringBounds; import static javaemul.internal.InternalPreconditions.checkNotNull; import java.util.Objects; @@ -76,15 +75,6 @@ if (csq == null) { csq = "null"; } - /* - * Ensure we throw StringIndexOutOfBoundsException as String#subsequence method depends on - * JavaScript's string#substr() which has different behaviour than original Java method. - * - * JavaScript behaviour of substr(start, length): - * "gwt".substr(0, -1) returns "" - * "gwt".substr(-1, 1) returns "t" -> Calculates substring from end. - */ - checkCriticalStringBounds(start, end, csq.length()); write(csq.subSequence(start, end).toString()); return this; }
diff --git a/user/super/com/google/gwt/emul/java/lang/AbstractStringBuilder.java b/user/super/com/google/gwt/emul/java/lang/AbstractStringBuilder.java index dabfd83..159dbde 100644 --- a/user/super/com/google/gwt/emul/java/lang/AbstractStringBuilder.java +++ b/user/super/com/google/gwt/emul/java/lang/AbstractStringBuilder.java
@@ -15,6 +15,8 @@ */ package java.lang; +import static javaemul.internal.InternalPreconditions.checkStringElementIndex; + /** * A base class to share implementation between {@link StringBuffer} and {@link StringBuilder}. * <p> @@ -113,6 +115,14 @@ } void replace0(int start, int end, String toInsert) { + int length = string.length(); + if (end > length) { + end = length; + } else { + // Only checking for start > end; since rest is checked with substring. + checkStringElementIndex(start, end + 1); + } + string = string.substring(0, start) + toInsert + string.substring(end); } @@ -142,4 +152,4 @@ buffer[f] = buffer[s]; buffer[s] = tmp; } -} \ No newline at end of file +}
diff --git a/user/super/com/google/gwt/emul/java/lang/String.java b/user/super/com/google/gwt/emul/java/lang/String.java index 0b58db7..45ddd0e 100644 --- a/user/super/com/google/gwt/emul/java/lang/String.java +++ b/user/super/com/google/gwt/emul/java/lang/String.java
@@ -18,6 +18,7 @@ import static javaemul.internal.InternalPreconditions.checkCriticalStringBounds; import static javaemul.internal.InternalPreconditions.checkNotNull; +import static javaemul.internal.InternalPreconditions.checkStringBounds; import static javaemul.internal.InternalPreconditions.checkStringElementIndex; import java.io.Serializable; @@ -27,14 +28,12 @@ import java.util.Comparator; import java.util.Locale; import java.util.StringJoiner; - import javaemul.internal.ArrayHelper; import javaemul.internal.EmulatedCharset; import javaemul.internal.HashCodes; import javaemul.internal.JsUtils; import javaemul.internal.NativeRegExp; import javaemul.internal.annotations.DoNotInline; - import jsinterop.annotations.JsMethod; import jsinterop.annotations.JsNonNull; import jsinterop.annotations.JsPackage; @@ -560,7 +559,8 @@ // treat "$$" as "$". // Escape regex special characters from literal replacement string. - String regex = from.toString().replaceAll("([/\\\\\\.\\*\\+\\?\\|\\(\\)\\[\\]\\{\\}$^])", "\\\\$1"); + String regex = + from.toString().replaceAll("([/\\\\\\.\\*\\+\\?\\|\\(\\)\\[\\]\\{\\}$^])", "\\\\$1"); // Escape $ since it is for match backrefs and \ since it is used to escape // $. String replacement = to.toString().replaceAll("\\\\", "\\\\\\\\").replaceAll("\\$", "\\\\$"); @@ -682,10 +682,12 @@ } public String substring(int beginIndex) { + checkStringElementIndex(beginIndex, length() + 1); return asNativeString().substr(beginIndex); } public String substring(int beginIndex, int endIndex) { + checkStringBounds(beginIndex, endIndex, length()); return asNativeString().substr(beginIndex, endIndex - beginIndex); } @@ -732,10 +734,6 @@ @Override public String toString() { - /* - * Magic: this method is only used during compiler optimizations; the generated JS will instead alias - * this method to the native String.prototype.toString() function. - */ return checkNotNull(this); }
diff --git a/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java b/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java index a427e59..a66c330 100644 --- a/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java +++ b/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java
@@ -591,6 +591,23 @@ * * @throws StringIndexOutOfBoundsException if the range is not legal */ + public static void checkStringBounds(int start, int end, int length) { + if (IS_BOUNDS_CHECKED) { + checkCriticalStringBounds(start, end, length); + } else if (IS_ASSERTED) { + try { + checkCriticalStringBounds(start, end, length); + } catch (Exception e) { + throw new AssertionError(e); + } + } + } + + /** + * Checks that string bounds are correct. + * + * @throws StringIndexOutOfBoundsException if the range is not legal + */ public static void checkCriticalStringBounds(int start, int end, int length) { if (start < 0 || end > length || end < start) { throw new StringIndexOutOfBoundsException(
diff --git a/user/test/com/google/gwt/emultest/java/lang/StringTest.java b/user/test/com/google/gwt/emultest/java/lang/StringTest.java index e940b92..4791200 100644 --- a/user/test/com/google/gwt/emultest/java/lang/StringTest.java +++ b/user/test/com/google/gwt/emultest/java/lang/StringTest.java
@@ -811,6 +811,32 @@ assertEquals("cd", hideFromCompiler("abcdefghi").substring(2, 4)); assertEquals("bc", hideFromCompiler("abcdef").substring(1, 3)); assertEquals("bcdef", hideFromCompiler("abcdef").substring(1)); + assertEquals("", hideFromCompiler("abcdef").substring(6)); + assertEquals("", hideFromCompiler("abcdef").substring(6, 6)); + + try { + hideFromCompiler("abc").substring(-1); + fail("Should have thrown"); + } catch (IndexOutOfBoundsException expected) { + } + + try { + hideFromCompiler("abc").substring(4); + fail("Should have thrown"); + } catch (IndexOutOfBoundsException expected) { + } + + try { + hideFromCompiler("abc").substring(2, 4); + fail("Should have thrown"); + } catch (IndexOutOfBoundsException expected) { + } + + try { + hideFromCompiler("abc").substring(2, 1); + fail("Should have thrown"); + } catch (IndexOutOfBoundsException expected) { + } } public void testToCharArray() {