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() {
