diff --git a/agrona/src/main/java/org/agrona/concurrent/errors/ErrorLogReader.java b/agrona/src/main/java/org/agrona/concurrent/errors/ErrorLogReader.java index 01c8497dc..fd6e40d2c 100644 --- a/agrona/src/main/java/org/agrona/concurrent/errors/ErrorLogReader.java +++ b/agrona/src/main/java/org/agrona/concurrent/errors/ErrorLogReader.java @@ -36,7 +36,7 @@ public class ErrorLogReader */ public static boolean hasErrors(final AtomicBuffer buffer) { - return buffer.capacity() >= SIZE_OF_INT && 0 != buffer.getIntVolatile(LENGTH_OFFSET); + return buffer.capacity() >= SIZE_OF_INT && buffer.getIntVolatile(LENGTH_OFFSET) > 0; } /** @@ -65,10 +65,10 @@ public static int read(final AtomicBuffer buffer, final ErrorConsumer consumer, int offset = 0; final int capacity = buffer.capacity(); - while (offset < capacity) + while (offset <= capacity - ENCODED_ERROR_OFFSET) { - final int length = buffer.getIntVolatile(offset + LENGTH_OFFSET); - if (0 == length) + final int length = Math.min(buffer.getIntVolatile(offset + LENGTH_OFFSET), capacity - offset); + if (length <= 0) { break; } diff --git a/agrona/src/test/java/org/agrona/concurrent/errors/ErrorLogReaderTest.java b/agrona/src/test/java/org/agrona/concurrent/errors/ErrorLogReaderTest.java index 05e3e1038..88a4e004e 100644 --- a/agrona/src/test/java/org/agrona/concurrent/errors/ErrorLogReaderTest.java +++ b/agrona/src/test/java/org/agrona/concurrent/errors/ErrorLogReaderTest.java @@ -25,10 +25,10 @@ import java.io.StringWriter; import java.nio.ByteBuffer; +import static org.agrona.concurrent.errors.DistinctErrorLog.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; class ErrorLogReaderTest @@ -132,4 +132,47 @@ void shouldReadOneObservationSinceTimestamp() verify(consumer).accept(eq(1), eq(timestampTwo), eq(timestampTwo), any(String.class)); verifyNoMoreInteractions(consumer); } + + @Test + void readShouldNotReadIfRemainingSpaceIsLessThanOneErrorPrefix() + { + final UnsafeBuffer buffer = new UnsafeBuffer(new byte[64]); + final long lastTimestamp = 543495734L; + final long firstTimestamp = lastTimestamp - 1000; + final int count = 123; + final int totalLength = 45; + buffer.putInt(LENGTH_OFFSET, totalLength); + buffer.putLong(LAST_OBSERVATION_TIMESTAMP_OFFSET, lastTimestamp); + buffer.putLong(FIRST_OBSERVATION_TIMESTAMP_OFFSET, firstTimestamp); + buffer.putInt(OBSERVATION_COUNT_OFFSET, count); + buffer.putStringWithoutLengthAscii(ENCODED_ERROR_OFFSET, "abcdefghijklmnopqrstuvwxyz"); + buffer.putInt(totalLength + LENGTH_OFFSET, 12); + final ErrorConsumer errorConsumer = mock(ErrorConsumer.class); + + assertEquals(1, ErrorLogReader.read(buffer, errorConsumer, 0)); + + verify(errorConsumer).accept(count, firstTimestamp, lastTimestamp, "abcdefghijklmnopqrstu"); + } + + @Test + void shouldNotExceedEndOfBufferWhenReadinErrorMessage() + { + final UnsafeBuffer buffer = new UnsafeBuffer(new byte[64]); + buffer.setMemory(0, buffer.capacity(), (byte)'?'); + final long lastTimestamp = 347923749327L; + final long firstTimestamp = -8530458948593L; + final int count = 999; + buffer.putInt(LENGTH_OFFSET, Integer.MAX_VALUE); + buffer.putLong(LAST_OBSERVATION_TIMESTAMP_OFFSET, lastTimestamp); + buffer.putLong(FIRST_OBSERVATION_TIMESTAMP_OFFSET, firstTimestamp); + buffer.putInt(OBSERVATION_COUNT_OFFSET, count); + buffer.putStringWithoutLengthAscii(ENCODED_ERROR_OFFSET, "test"); + final String expectedErrorString = + buffer.getStringWithoutLengthAscii(ENCODED_ERROR_OFFSET, buffer.capacity() - ENCODED_ERROR_OFFSET); + final ErrorConsumer errorConsumer = mock(ErrorConsumer.class); + + assertEquals(1, ErrorLogReader.read(buffer, errorConsumer, 0)); + + verify(errorConsumer).accept(count, firstTimestamp, lastTimestamp, expectedErrorString); + } }