Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/main/java/org/apache/commons/lang3/StringUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,15 @@ public static String center(String str, final int size, String padStr) {
return rightPad(str, size, padStr);
}

private static void checkFromToIndex(final int startIndex, final int endIndex, final int length) {
if (startIndex < 0) {
throw new ArrayIndexOutOfBoundsException(startIndex);
}
if (endIndex > length) {
throw new ArrayIndexOutOfBoundsException(endIndex);
}
}

/**
* Removes one newline from end of a String if it's there, otherwise leave it alone. A newline is &quot;{@code \n}&quot;, &quot;{@code \r}&quot;, or
* &quot;{@code \r\n}&quot;.
Expand Down Expand Up @@ -3866,6 +3875,7 @@ public static String join(final boolean[] array, final char delimiter, final int
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -3943,6 +3953,7 @@ public static String join(final byte[] array, final char delimiter, final int st
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4020,6 +4031,7 @@ public static String join(final char[] array, final char delimiter, final int st
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4097,6 +4109,7 @@ public static String join(final double[] array, final char delimiter, final int
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4174,6 +4187,7 @@ public static String join(final float[] array, final char delimiter, final int s
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4251,6 +4265,7 @@ public static String join(final int[] array, final char delimiter, final int sta
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4491,6 +4506,7 @@ public static String join(final long[] array, final char delimiter, final int st
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down Expand Up @@ -4687,6 +4703,7 @@ public static String join(final short[] array, final char delimiter, final int s
if (array == null) {
return null;
}
checkFromToIndex(startIndex, endIndex, array.length);
final int count = endIndex - startIndex;
if (count <= 0) {
return EMPTY;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.commons.lang3;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTimeout;

import java.time.Duration;

import org.junit.jupiter.api.Test;

/**
* Primitive joins can OOM before index validation.
* <p>
* capacity() is called before bounds validation in join(byte[], char, int, int). An endIndex of Integer.MAX_VALUE causes an attempt to allocate ~8 GB before
* the array bounds are checked. Negative-startIndex bypass case.
* </p>
* <p>
* For {@code startIndex = -2_000_000_000, endIndex = 1, array.length = 2}: {@code count = endIndex - startIndex = 2_000_000_001}, which is positive (no
* overflow into negative territory), so the {@code count <= 0} early-return does not fire. The pre-existing {@code endIndex > array.length} check also passes
* (1 &lt; 2). The capacity allocation then attempts to size a StringBuilder for ~2 billion items, triggering OOM before the loop body would have caught the bad
* index.
* </p>
*
* Pre-patch: throws OutOfMemoryError or hangs for > 2 seconds. Post-patch: throws ArrayIndexOutOfBoundsException quickly (within 2 seconds).
*/
public class StringUtilsJoinExceptionTest {

private static final Class<ArrayIndexOutOfBoundsException> EXPECTED_EX = ArrayIndexOutOfBoundsException.class;
private static final Duration TIMEOUT = Duration.ofSeconds(2);

@Test
public void testBooleanJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new boolean[] { true }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testBooleanJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new boolean[] { true, false }, ',', -2_000_000_000, 1)));
}

@Test
public void testByteJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new byte[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testByteJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new byte[] { 1, 2 }, ',', -2_000_000_000, 1)));
}

@Test
public void testCharJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new char[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testCharJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new char[] { 1, 2 }, ',', -2_000_000_000, 1)));
}

@Test
public void testDoubleJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new double[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testDoubleJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new double[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1)));
}

@Test
public void testFloatJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new float[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testFloatJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new float[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1)));
}

@Test
public void testIntJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new int[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testIntJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new int[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1)));
}

@Test
public void testLongJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new long[] { 1L }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testLongJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new long[] { 1L, 2L }, ',', -2_000_000_000, 1)));
}

@Test
public void testShortJoinValidatesEndIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new short[] { 1 }, ',', 0, Integer.MAX_VALUE)));
}

@Test
public void testShortJoinValidatesNegativeStartIndexBeforeCapacity() {
assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new short[] { 1, 2 }, ',', -2_000_000_000, 1)));
}
}
Loading