diff --git a/src/main/java/net/starlark/java/eval/MutableStarlarkList.java b/src/main/java/net/starlark/java/eval/MutableStarlarkList.java index 51199f585bbb1d..325d9e9ab8b929 100644 --- a/src/main/java/net/starlark/java/eval/MutableStarlarkList.java +++ b/src/main/java/net/starlark/java/eval/MutableStarlarkList.java @@ -123,10 +123,7 @@ private void grow(int mincap) { // Grow capacity enough to insert given number of elements private void growAdditional(int additional) throws EvalException { - int mincap = size + additional; - if (mincap < 0 || mincap > MAX_ALLOC) { - throw Starlark.errorf("excessive capacity requested (%d + %d elements)", size, additional); - } + int mincap = addSizesAndFailIfExcessive(size, additional); grow(mincap); } diff --git a/src/main/java/net/starlark/java/eval/StarlarkList.java b/src/main/java/net/starlark/java/eval/StarlarkList.java index 81837301a93d66..00157461713562 100644 --- a/src/main/java/net/starlark/java/eval/StarlarkList.java +++ b/src/main/java/net/starlark/java/eval/StarlarkList.java @@ -204,17 +204,28 @@ public static StarlarkList immutableOf(T... elems) { /** * Returns a new {@code StarlarkList} that is the concatenation of two {@code StarlarkList}s. The * new list will have the given {@link Mutability}. + * + * @throws EvalException if the resulting list would be too large */ public static StarlarkList concat( - StarlarkList x, StarlarkList y, Mutability mutability) { + StarlarkList x, StarlarkList y, Mutability mutability) + throws EvalException { int xsize = x.size(); int ysize = y.size(); - Object[] res = new Object[xsize + ysize]; + Object[] res = new Object[addSizesAndFailIfExcessive(xsize, ysize)]; System.arraycopy(x.elems(), 0, res, 0, xsize); System.arraycopy(y.elems(), 0, res, xsize, ysize); return wrap(mutability, res); } + protected static int addSizesAndFailIfExcessive(int xsize, int ysize) throws EvalException { + int sum = xsize + ysize; + if (sum < 0 || sum > MAX_ALLOC) { + throw Starlark.errorf("excessive capacity requested (%d + %d elements)", xsize, ysize); + } + return sum; + } + @Nonnull @Override public Iterator iterator() { diff --git a/src/test/java/net/starlark/java/eval/BUILD b/src/test/java/net/starlark/java/eval/BUILD index 3249dc9e1dea0b..39473a4dc04e00 100644 --- a/src/test/java/net/starlark/java/eval/BUILD +++ b/src/test/java/net/starlark/java/eval/BUILD @@ -32,7 +32,11 @@ java_test( "StarlarkThreadDebuggingTest.java", "StarlarkThreadTest.java", ], - jvm_flags = ["-Dfile.encoding=UTF8"], + jvm_flags = [ + "-Dfile.encoding=UTF8", + # StarlarkListTest.concat_failsCleanlyOnOverflow() needs at least 4GB of max heap + "-Xmx4096m", + ], deps = [ "//src/main/java/net/starlark/java/annot", "//src/main/java/net/starlark/java/eval", diff --git a/src/test/java/net/starlark/java/eval/StarlarkListTest.java b/src/test/java/net/starlark/java/eval/StarlarkListTest.java index 2dc9255332fb19..75e4fd4ee7a35e 100644 --- a/src/test/java/net/starlark/java/eval/StarlarkListTest.java +++ b/src/test/java/net/starlark/java/eval/StarlarkListTest.java @@ -187,6 +187,20 @@ public void testTupleToArray() { } } + @Test + public void concat_failsCleanlyOnOverflow() throws Exception { + System.gc(); + Mutability mu = Mutability.create("test"); + // veryBigArray is large enough that veryBigArray.size * 2 > StarlarkList.MAX_ALLOC + Object[] veryBigArray = new Object[(1 << 29) + 1]; // This will OOM if jvm max heap is too low. + StarlarkList veryBigList = StarlarkList.wrap(mu, veryBigArray); + EvalException e = + assertThrows(EvalException.class, () -> StarlarkList.concat(veryBigList, veryBigList, mu)); + assertThat(e) + .hasMessageThat() + .isEqualTo("excessive capacity requested (536870913 + 536870913 elements)"); + } + @FormatMethod private static void fail(@FormatString String format, Object... args) { throw new AssertionError(String.format(format, args));