Skip to content

Commit

Permalink
Ensures that using a cursor after close has no effect.
Browse files Browse the repository at this point in the history
  • Loading branch information
tgregg committed Apr 23, 2024
1 parent 94caf27 commit 5ee9f7e
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
23 changes: 17 additions & 6 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private static class RefillableState {
* READY state). This enables the reader to complete the previous IonCursor API invocation if it returned a
* NEEDS_DATA event.
*/
State state = State.READY;
State state;

/**
* The number of bytes that still need to be consumed from the input during a fill or seek operation.
Expand Down Expand Up @@ -124,14 +124,20 @@ private static class RefillableState {
*/
int individualBytesSkippedWithoutBuffering = 0;

RefillableState(InputStream inputStream, int capacity, int maximumBufferSize) {
RefillableState(InputStream inputStream, int capacity, int maximumBufferSize, State initialState) {
this.inputStream = inputStream;
this.capacity = capacity;
this.maximumBufferSize = maximumBufferSize;
this.state = initialState;
}

}

/**
* Dummy state that indicates the cursor has been terminated and that additional API calls will have no effect.
*/
private static final RefillableState TERMINATED_STATE = new RefillableState(null, -1, -1, State.TERMINATED);

/**
* Stack to hold container info. Stepping into a container results in a push; stepping out results in a pop.
*/
Expand Down Expand Up @@ -249,7 +255,7 @@ private static class RefillableState {
/**
* Holds information necessary for reading from refillable input. Null if the cursor is byte-backed.
*/
private final RefillableState refillableState;
private RefillableState refillableState;

/**
* Describes the byte at the `checkpoint` index.
Expand Down Expand Up @@ -455,7 +461,8 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor(
refillableState = new RefillableState(
inputStream,
configuration.getInitialBufferSize(),
configuration.getMaximumBufferSize()
configuration.getMaximumBufferSize(),
State.READY
);
registerOversizedValueHandler(configuration.getOversizedValueHandler());
}
Expand Down Expand Up @@ -1898,12 +1905,15 @@ private boolean slowIsAwaitingMoreData() {
* exceeds the maximum buffer size.
*/
void terminate() {
refillableState.state = State.TERMINATED;
refillableState = TERMINATED_STATE;
// Use a unified code path for all cursors after termination. This path forces a termination check before
// accessing the input stream or buffer.
isSlowMode = true;
}

@Override
public void close() {
if (refillableState != null) {
if (refillableState != null && refillableState.inputStream != null) {
try {
refillableState.inputStream.close();
} catch (IOException e) {
Expand All @@ -1913,6 +1923,7 @@ public void close() {
buffer = null;
containerStack = null;
byteBuffer = null;
terminate();
}

/* ---- End: version-agnostic parsing, utility, and public API methods ---- */
Expand Down
20 changes: 19 additions & 1 deletion src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package com.amazon.ion.impl;

import com.amazon.ion.IonBufferConfiguration;
Expand Down Expand Up @@ -31,6 +30,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class IonCursorBinaryTest {
Expand Down Expand Up @@ -427,4 +427,22 @@ public void expectValueLargerThanMaxArraySizeToFailCleanly() {

cursor.close();
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void expectUseAfterCloseToHaveNoEffect(boolean constructFromBytes) {
// Using the cursor after close should not fail with an obscure unchecked
// exception like NullPointerException, ArrayIndexOutOfBoundsException, etc.
IonCursorBinary cursor = initializeCursor(
constructFromBytes,
0xE0, 0x01, 0x00, 0xEA,
0x20
);
cursor.close();
assertEquals(IonCursor.Event.NEEDS_DATA, cursor.nextValue());
assertNull(cursor.getValueMarker().typeId);
assertEquals(IonCursor.Event.NEEDS_DATA, cursor.nextValue());
assertNull(cursor.getValueMarker().typeId);
cursor.close();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4009,4 +4009,22 @@ public void corruptEachByteThrowsIonException(boolean constructFromBytes) {
corruptEachByteThrowsIonException(constructFromBytes, false, true);
corruptEachByteThrowsIonException(constructFromBytes, false, false);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void expectUseAfterCloseToHaveNoEffect(boolean constructFromBytes) throws Exception {
// Using the cursor after close should not fail with an obscure unchecked
// exception like NullPointerException, ArrayIndexOutOfBoundsException, etc.
reader = readerFor(
constructFromBytes,
0xE0, 0x01, 0x00, 0xEA,
0x20
);
reader.close();
assertNull(reader.next());
assertNull(reader.getType());
assertNull(reader.next());
assertNull(reader.getType());
reader.close();
}
}

0 comments on commit 5ee9f7e

Please sign in to comment.