Skip to content

Commit

Permalink
Ensures that the binary reader can handle a ByteArrayInputStream that…
Browse files Browse the repository at this point in the history
… returns a negative number from available(). (#625)
  • Loading branch information
tgregg authored Nov 3, 2023
1 parent 61d76e0 commit 8df9b7d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 34 deletions.
5 changes: 4 additions & 1 deletion src/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,10 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor(
ByteArrayInputStream inputStream,
int alreadyReadLen
) {
int fixedBufferSize = inputStream.available();
// Note: ByteArrayInputStream.available() can return a negative number because its constructor does
// not validate that the offset and length provided are actually within range of the provided byte array.
// Setting the result to 0 in this case avoids an error when looking up the fixed sized configuration.
int fixedBufferSize = Math.max(0, inputStream.available());
if (alreadyReadLen > 0) {
fixedBufferSize += alreadyReadLen;
}
Expand Down
49 changes: 22 additions & 27 deletions src/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ class IonReaderContinuableCoreBinary extends IonCursorBinary implements IonReade
// The number of bytes occupied by a Java long.
private static final int LONG_SIZE_IN_BYTES = 8;

// The smallest negative 8-byte integer that can fit in a long is -0x80_00_00_00_00_00_00_00.
private static final int MOST_SIGNIFICANT_BYTE_OF_MIN_LONG = 0x80;
// The smallest negative 8-byte integer that can fit in a long is 0x80_00_00_00_00_00_00_00 and the smallest
// negative 4-byte integer that can fit in an int is 0x80_00_00_00.
private static final int MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER = 0x80;

// The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF.
private static final int MOST_SIGNIFICANT_BYTE_OF_MAX_LONG = 0x7F;
// The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF and the largest positive
// 4-byte integer that can fit in an int is 0x7F_FF_FF_FF.
private static final int MOST_SIGNIFICANT_BYTE_OF_MAX_INTEGER = 0x7F;

// The second-most significant bit in the most significant byte of a VarInt is the sign.
private static final int VAR_INT_SIGN_BITMASK = 0x40;
Expand Down Expand Up @@ -418,34 +420,30 @@ private boolean readBoolean_1_0() {
}

/**
* Determines whether the 8-byte integer starting at `valueMarker.startIndex` and ending at `valueMarker.endIndex`
* fits in a long, or requires a BigInteger.
* @return either `IntegerSize.LONG` or `IntegerSize.BIG_INTEGER`.
* Determines whether the integer starting at `valueMarker.startIndex` and ending at `valueMarker.endIndex`
* crosses a type boundary. Callers must only invoke this method when the integer's size is known to be either
* 4 or 8 bytes.
* @return true if the value fits in the Java integer type that matches its Ion serialized size; false if it
* requires the next larger size.
*/
private IntegerSize classifyEightByteInt_1_0() {
private boolean classifyInteger_1_0() {
if (valueTid.isNegativeInt) {
// The smallest negative 8-byte integer that can fit in a long is -0x80_00_00_00_00_00_00_00.
int firstByte = buffer[(int)(valueMarker.startIndex)] & SINGLE_BYTE_MASK;
if (firstByte < MOST_SIGNIFICANT_BYTE_OF_MIN_LONG) {
return IntegerSize.LONG;
} else if (firstByte > MOST_SIGNIFICANT_BYTE_OF_MIN_LONG) {
return IntegerSize.BIG_INTEGER;
if (firstByte < MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER) {
return true;
} else if (firstByte > MOST_SIGNIFICANT_BYTE_OF_MIN_INTEGER) {
return false;
}
for (long i = valueMarker.startIndex + 1; i < valueMarker.endIndex; i++) {
if (0x00 != buffer[(int)(i)]) {
return IntegerSize.BIG_INTEGER;
return false;
}
}
} else {
// The largest positive 8-byte integer that can fit in a long is 0x7F_FF_FF_FF_FF_FF_FF_FF.
if ((buffer[(int)(valueMarker.startIndex)] & SINGLE_BYTE_MASK) > MOST_SIGNIFICANT_BYTE_OF_MAX_LONG) {
return IntegerSize.BIG_INTEGER;
}
return true;
}
return IntegerSize.LONG;
return (buffer[(int) (valueMarker.startIndex)] & SINGLE_BYTE_MASK) <= MOST_SIGNIFICANT_BYTE_OF_MAX_INTEGER;
}


int readVarUInt_1_1() {
throw new UnsupportedOperationException();
}
Expand Down Expand Up @@ -536,16 +534,13 @@ public IntegerSize getIntegerSize() {
if (valueTid.length < 0) {
return IntegerSize.BIG_INTEGER;
} else if (valueTid.length < INT_SIZE_IN_BYTES) {
// Note: this is conservative. Most integers of size 4 also fit in an int, but since exactly the
// same parsing code is used for ints and longs, there is no point wasting the time to determine the
// smallest possible type.
return IntegerSize.INT;
} else if (valueTid.length == INT_SIZE_IN_BYTES) {
return (minorVersion != 0 || classifyInteger_1_0()) ? IntegerSize.INT : IntegerSize.LONG;
} else if (valueTid.length < LONG_SIZE_IN_BYTES) {
return IntegerSize.LONG;
} else if (valueTid.length == LONG_SIZE_IN_BYTES) {
// Because creating BigIntegers is so expensive, it is worth it to look ahead and determine exactly
// which 8-byte integers can fit in a long.
return minorVersion == 0 ? classifyEightByteInt_1_0() : IntegerSize.LONG;
return (minorVersion != 0 || classifyInteger_1_0()) ? IntegerSize.LONG : IntegerSize.BIG_INTEGER;
}
return IntegerSize.BIG_INTEGER;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3368,6 +3368,17 @@ public void concatenatedAfterMissingGZIPTrailer() throws Exception {
assertEquals(1, oversizedCounter.get());
}

@Test
public void shouldNotFailWhenProvidedWithAnEmptyByteArrayInputStream() throws Exception {
reader = IonReaderBuilder.standard().build(new ByteArrayInputStream(new byte[]{}));
assertSequence(next(null));
reader.close();
// The following ByteArrayInputStream is weird, but not disallowed. Its available() method will return -1.
reader = IonReaderBuilder.standard().build(new ByteArrayInputStream(new byte[]{}, 1, 1));
assertSequence(next(null));
reader.close();
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void incompleteContainerNonContinuable(boolean constructFromBytes) throws Exception {
Expand Down
10 changes: 4 additions & 6 deletions test/com/amazon/ion/streaming/ReaderIntegerSizeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,10 @@ public void testGetIntegerSizeNegativeIntBoundary()
private void testGetIntegerSizeIntBoundary(int boundaryValue, long pastBoundary)
{
in.next();
// It's fine if IntegerSize recommends a larger-than-necessary type, just not a smaller one.
assertTrue(IntegerSize.INT == in.getIntegerSize() || IntegerSize.LONG == in.getIntegerSize());
assertEquals(IntegerSize.INT, in.getIntegerSize());
assertEquals(boundaryValue, in.intValue());
// assert nothing changes until next()
assertTrue(IntegerSize.INT == in.getIntegerSize() || IntegerSize.LONG == in.getIntegerSize());
assertEquals(IntegerSize.INT, in.getIntegerSize());
in.next();
assertEquals(IntegerSize.LONG, in.getIntegerSize());
assertEquals(pastBoundary, in.longValue());
Expand All @@ -156,10 +155,9 @@ private void testGetIntegerSizeIntBoundary(int boundaryValue, long pastBoundary)
private void testGetIntegerSizeLongBoundary(long boundaryValue, BigInteger pastBoundary)
{
in.next();
// It's fine if IntegerSize recommends a larger-than-necessary type, just not a smaller one.
assertTrue(IntegerSize.LONG == in.getIntegerSize() || IntegerSize.BIG_INTEGER == in.getIntegerSize());
assertEquals(IntegerSize.LONG, in.getIntegerSize());
assertEquals(boundaryValue, in.longValue());
assertTrue(IntegerSize.LONG == in.getIntegerSize() || IntegerSize.BIG_INTEGER == in.getIntegerSize());
assertEquals(IntegerSize.LONG, in.getIntegerSize());
in.next();
assertEquals(IntegerSize.BIG_INTEGER, in.getIntegerSize());
assertEquals(pastBoundary, in.bigIntegerValue());
Expand Down

0 comments on commit 8df9b7d

Please sign in to comment.