#923 Fixed: IndexOutOfBoundsException in FBCachedBlob.getBytes(long, int) for position or length beyond end of data

Author: mrotteveel

src/main/org/firebirdsql/jdbc/FBCachedBlob.java

@@ -8,6 +8,7 @@
 
 import org.firebirdsql.gds.JaybirdErrorCodes;
 import org.firebirdsql.gds.ng.FbExceptionBuilder;
+import org.firebirdsql.jaybird.util.ByteArrayHelper;
 import org.firebirdsql.util.InternalApi;
 import org.jspecify.annotations.NullMarked;
 
@@ -47,8 +48,7 @@ public FBCachedBlob(byte[] data) {
 
     @Override
     public FirebirdBlob detach() throws SQLException {
-        checkOpen();
-        return new FBCachedBlob(blobData);
+        return new FBCachedBlob(requireBlobData());
     }
 
     /**
@@ -70,32 +70,31 @@ public boolean isSegmented() throws SQLException {
      */
     @Override
     public long length() throws SQLException {
-        checkOpen();
-        return blobData.length;
+        return requireBlobData().length;
     }
 
     @Override
     public byte[] getBytes(long pos, int length) throws SQLException {
         if (pos < 1) {
             throw new SQLException("Expected value of pos > 0, got " + pos,
                     SQLStateConstants.SQL_STATE_INVALID_STRING_LENGTH);
-        }
-        if (length < 0) {
+        } else if (length < 0) {
             throw new SQLException("Expected value of length >= 0, got " + length,
                     SQLStateConstants.SQL_STATE_INVALID_STRING_LENGTH);
         }
-        checkOpen();
+        byte[] blobData = requireBlobData();
+
+        if (pos > blobData.length) return ByteArrayHelper.emptyByteArray();
+        length = (int) Math.min(length, blobData.length - pos + 1L);
 
-        // TODO What if pos or length are beyond blobData
         byte[] result = new byte[length];
         System.arraycopy(blobData, (int) pos - 1, result, 0, length);
         return result;
     }
 
     @Override
     public byte[] getBytes() throws SQLException {
-        checkOpen();
-        return blobData.clone();
+        return requireBlobData().clone();
     }
 
     /**
@@ -122,8 +121,7 @@ public long position(Blob pattern, long start) throws SQLException {
 
     @Override
     public InputStream getBinaryStream() throws SQLException {
-        checkOpen();
-        return new ByteArrayInputStream(blobData);
+        return new ByteArrayInputStream(requireBlobData());
     }
 
     @Override
@@ -180,9 +178,25 @@ public void free() throws SQLException {
         blobData = FREED_MARKER;
     }
 
+    /**
+     * Checks if the blob is open (not freed).
+     * <p>
+     * If a method needs access to the blob data, use {@link #requireBlobData()} instead.
+     * </p>
+     *
+     * @see #requireBlobData()
+     */
     private void checkOpen() throws SQLException {
         if (blobData == FREED_MARKER) {
             throw FbExceptionBuilder.toException(JaybirdErrorCodes.jb_blobClosed);
         }
     }
+
+    private byte[] requireBlobData() throws SQLException {
+        byte[] blobData = this.blobData;
+        if (blobData == FREED_MARKER) {
+            throw FbExceptionBuilder.toException(JaybirdErrorCodes.jb_blobClosed);
+        }
+        return blobData;
+    }
 }

src/test/org/firebirdsql/jdbc/FBCachedBlobTest.java

@@ -5,6 +5,8 @@
 import org.firebirdsql.gds.JaybirdErrorCodes;
 import org.hamcrest.Matcher;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
@@ -15,6 +17,7 @@
 import static org.firebirdsql.common.matchers.SQLExceptionMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.jupiter.api.Assertions.*;
 
 /**
@@ -129,6 +132,28 @@ void testGetBytes_long_int() throws Exception {
         assertArrayEquals(new byte[] { 5, 6, 7, 8, 9 }, data, "Unexpected data");
     }
 
+    @ParameterizedTest
+    @ValueSource(longs = { 11, Integer.MAX_VALUE, Integer.MAX_VALUE + 1L, Long.MAX_VALUE })
+    void testGetBytes_long_int_posBeyondEnd(long pos) throws Exception {
+        assertThat("Wrong value for pos", pos, greaterThan(10L));
+        FBCachedBlob blob = new FBCachedBlob(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 });
+
+        byte[] data = blob.getBytes(pos, 5);
+
+        assertNotNull(data, "Expected non-null array");
+        assertArrayEquals(new byte[0], data, "Unexpected data");
+    }
+
+    @Test
+    void testGetBytes_long_int_lengthBeyondEnd() throws Exception {
+        FBCachedBlob blob = new FBCachedBlob(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 });
+
+        byte[] data = blob.getBytes(5, 10);
+
+        assertNotNull(data, "Expected non-null array");
+        assertArrayEquals(new byte[] { 5, 6, 7, 8, 9, 10 }, data, "Unexpected data");
+    }
+
     @Test
     void testGetBytes() throws Exception {
         byte[] input = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };