From 8720b6bbc20a36ffdc6c6ce397f1308bd7b630d6 Mon Sep 17 00:00:00 2001 From: Istvan Neuwirth Date: Mon, 26 Jun 2023 08:52:50 -0700 Subject: [PATCH] Changed `Longs.concat()` to throw `IllegalArgumentException` if the input arrays contain too many elements. See https://github.com/google/guava/issues/3303 Fixes #3304 RELNOTES=n/a PiperOrigin-RevId: 543451555 --- .../google/common/primitives/LongsTest.java | 31 +++++++++++++++++++ .../com/google/common/primitives/Longs.java | 14 +++++++-- .../google/common/primitives/LongsTest.java | 31 +++++++++++++++++++ .../com/google/common/primitives/Longs.java | 14 +++++++-- 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/android/guava-tests/test/com/google/common/primitives/LongsTest.java b/android/guava-tests/test/com/google/common/primitives/LongsTest.java index a911a0370aff..83363c41144c 100644 --- a/android/guava-tests/test/com/google/common/primitives/LongsTest.java +++ b/android/guava-tests/test/com/google/common/primitives/LongsTest.java @@ -198,6 +198,37 @@ public void testConcat() { .isEqualTo(new long[] {(long) 1, (long) 2, (long) 3, (long) 4}); } + @GwtIncompatible // different overflow behavior; could probably be made to work by using ~~ + public void testConcat_overflow_negative() { + int dim1 = 1 << 16; + int dim2 = 1 << 15; + assertThat(dim1 * dim2).isLessThan(0); + testConcat_overflow(dim1, dim2); + } + + @GwtIncompatible // different overflow behavior; could probably be made to work by using ~~ + public void testConcat_overflow_nonNegative() { + int dim1 = 1 << 16; + int dim2 = 1 << 16; + assertThat(dim1 * dim2).isAtLeast(0); + testConcat_overflow(dim1, dim2); + } + + private static void testConcat_overflow(int arraysDim1, int arraysDim2) { + assertThat((long) arraysDim1 * arraysDim2).isNotEqualTo((long) (arraysDim1 * arraysDim2)); + + long[][] arrays = new long[arraysDim1][]; + // it's shared to avoid using too much memory in tests + long[] sharedArray = new long[arraysDim2]; + Arrays.fill(arrays, sharedArray); + + try { + Longs.concat(arrays); + fail(); + } catch (IllegalArgumentException expected) { + } + } + private static void assertByteArrayEquals(byte[] expected, byte[] actual) { assertWithMessage( "Expected: " + Arrays.toString(expected) + ", but got: " + Arrays.toString(actual)) diff --git a/android/guava/src/com/google/common/primitives/Longs.java b/android/guava/src/com/google/common/primitives/Longs.java index 536fdd5fbfa2..ce6a6fa39485 100644 --- a/android/guava/src/com/google/common/primitives/Longs.java +++ b/android/guava/src/com/google/common/primitives/Longs.java @@ -244,13 +244,15 @@ public static long constrainToRange(long value, long min, long max) { * * @param arrays zero or more {@code long} arrays * @return a single array containing all the values from the source arrays, in order + * @throws IllegalArgumentException if the total number of elements in {@code arrays} does not fit + * in an {@code int} */ public static long[] concat(long[]... arrays) { - int length = 0; + long length = 0; for (long[] array : arrays) { length += array.length; } - long[] result = new long[length]; + long[] result = new long[checkNoOverflow(length)]; int pos = 0; for (long[] array : arrays) { System.arraycopy(array, 0, result, pos, array.length); @@ -259,6 +261,14 @@ public static long[] concat(long[]... arrays) { return result; } + private static int checkNoOverflow(long result) { + checkArgument( + result == (int) result, + "the total number of elements (%s) in the arrays must fit in an int", + result); + return (int) result; + } + /** * Returns a big-endian representation of {@code value} in an 8-element byte array; equivalent to * {@code ByteBuffer.allocate(8).putLong(value).array()}. For example, the input value {@code diff --git a/guava-tests/test/com/google/common/primitives/LongsTest.java b/guava-tests/test/com/google/common/primitives/LongsTest.java index a911a0370aff..83363c41144c 100644 --- a/guava-tests/test/com/google/common/primitives/LongsTest.java +++ b/guava-tests/test/com/google/common/primitives/LongsTest.java @@ -198,6 +198,37 @@ public void testConcat() { .isEqualTo(new long[] {(long) 1, (long) 2, (long) 3, (long) 4}); } + @GwtIncompatible // different overflow behavior; could probably be made to work by using ~~ + public void testConcat_overflow_negative() { + int dim1 = 1 << 16; + int dim2 = 1 << 15; + assertThat(dim1 * dim2).isLessThan(0); + testConcat_overflow(dim1, dim2); + } + + @GwtIncompatible // different overflow behavior; could probably be made to work by using ~~ + public void testConcat_overflow_nonNegative() { + int dim1 = 1 << 16; + int dim2 = 1 << 16; + assertThat(dim1 * dim2).isAtLeast(0); + testConcat_overflow(dim1, dim2); + } + + private static void testConcat_overflow(int arraysDim1, int arraysDim2) { + assertThat((long) arraysDim1 * arraysDim2).isNotEqualTo((long) (arraysDim1 * arraysDim2)); + + long[][] arrays = new long[arraysDim1][]; + // it's shared to avoid using too much memory in tests + long[] sharedArray = new long[arraysDim2]; + Arrays.fill(arrays, sharedArray); + + try { + Longs.concat(arrays); + fail(); + } catch (IllegalArgumentException expected) { + } + } + private static void assertByteArrayEquals(byte[] expected, byte[] actual) { assertWithMessage( "Expected: " + Arrays.toString(expected) + ", but got: " + Arrays.toString(actual)) diff --git a/guava/src/com/google/common/primitives/Longs.java b/guava/src/com/google/common/primitives/Longs.java index 90ceb4f7c028..8369973cc789 100644 --- a/guava/src/com/google/common/primitives/Longs.java +++ b/guava/src/com/google/common/primitives/Longs.java @@ -246,13 +246,15 @@ public static long constrainToRange(long value, long min, long max) { * * @param arrays zero or more {@code long} arrays * @return a single array containing all the values from the source arrays, in order + * @throws IllegalArgumentException if the total number of elements in {@code arrays} does not fit + * in an {@code int} */ public static long[] concat(long[]... arrays) { - int length = 0; + long length = 0; for (long[] array : arrays) { length += array.length; } - long[] result = new long[length]; + long[] result = new long[checkNoOverflow(length)]; int pos = 0; for (long[] array : arrays) { System.arraycopy(array, 0, result, pos, array.length); @@ -261,6 +263,14 @@ public static long[] concat(long[]... arrays) { return result; } + private static int checkNoOverflow(long result) { + checkArgument( + result == (int) result, + "the total number of elements (%s) in the arrays must fit in an int", + result); + return (int) result; + } + /** * Returns a big-endian representation of {@code value} in an 8-element byte array; equivalent to * {@code ByteBuffer.allocate(8).putLong(value).array()}. For example, the input value {@code