[Java] Do not crash when reading partial or corrupted error messages.…

… Attempt to recover last error message when length is out of bounds.
real-logic · Apr 29, 2024 · 6a7a040 · 6a7a040
1 parent ea12615
commit 6a7a040
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 6 deletions.
diff --git 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
@@ -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);
+    }
 }