#767 Change FBBlob.getBytes to return shorter array than requested at…

… end-of-blob instead of exception

src/docs/asciidoc/release_notes.adoc

@@ -878,6 +878,12 @@ Other drivers and tools simply send the requests for blob id `0` to the server,
 For consistency, we decided to let Jaybird handle it the same.
 +
 This change was also backported to Jaybird 5.0.3.
+* Fixed: The implementation of `Blob.getBytes(long, int)` threw a `SQLException` if the remaining bytes of the blob where less than the requested number of bytes (https://github.com/FirebirdSQL/jaybird/issues/767[#767])
++
+The JDBC API specifies _" This `byte` array contains up to `length` consecutive bytes starting at position pos."_, so the implementation was changed to return up to `length` bytes, or the remaining actual blob length, whichever is shorter.
++
+The JDBC API does not specify what should happen if the requested position is beyond the end-of-blob.
+The modified implementation returns an empty array, but given this is unspecified behaviour, we reserve the option to change this in the future to throw an exception instead.
 
 [#removal-of-deprecated-classes-and-packages]
 === Removal of deprecated classes and packages

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

@@ -42,6 +42,7 @@
 import java.sql.SQLException;
 import java.sql.SQLNonTransientException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -295,9 +296,10 @@ public byte[] getBytes(long pos, int length) throws SQLException {
                     in.seek((int) pos - 1);
                 }
 
+                // We optimize for the case where we can read all data
                 byte[] result = new byte[length];
-                in.readFully(result);
-                return result;
+                int read = in.readNBytes(result, 0, length);
+                return read == length ? result : Arrays.copyOf(result, read);
             } catch (IOException e) {
                 if (e.getCause() instanceof SQLException sqle) {
                     throw sqle;

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

@@ -83,7 +83,7 @@ public String getSubString(long pos, int length) throws SQLException {
 			}
 			return sb.toString();
 		} catch (IOException e) {
-			throw new FBSQLException(e);
+			throw new SQLException(e.toString(), SQLStateConstants.SQL_STATE_GENERAL_ERROR, e);
 		}
 	}
 

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

@@ -162,6 +162,16 @@ interface BlobInputStream extends AutoCloseable {
          */
         void readFully(byte[] buffer) throws IOException;
 
+        /**
+         * @see InputStream#readNBytes(byte[], int, int)
+         */
+        int readNBytes(byte[] b, int off, int len) throws IOException;
+
+        /**
+         * @see InputStream#readNBytes(int)
+         */
+        byte[] readNBytes(int len) throws IOException;
+
         /**
          * Move current position in the Blob stream. This is a shortcut method to {@link #seek(int, int)} passing
          * {@link #SEEK_MODE_ABSOLUTE} as seek mode.

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

@@ -18,6 +18,7 @@
  */
 package org.firebirdsql.jdbc;
 
+import org.firebirdsql.common.DataGenerator;
 import org.firebirdsql.common.extension.UsesDatabaseExtension;
 import org.firebirdsql.gds.ISCConstants;
 import org.firebirdsql.gds.JaybirdErrorCodes;
@@ -35,7 +36,6 @@
 import java.util.Properties;
 
 import static org.firebirdsql.common.DdlHelper.executeCreateTable;
-import static org.firebirdsql.common.FBTestProperties.getConnectionViaDriverManager;
 import static org.firebirdsql.common.FBTestProperties.getDefaultPropertiesForConnection;
 import static org.firebirdsql.common.FBTestProperties.getUrl;
 import static org.firebirdsql.common.matchers.SQLExceptionMatchers.*;
@@ -70,7 +70,7 @@ class FBBlobTest {
 
     @BeforeEach
     void setup() throws SQLException {
-        conn = getConnectionViaDriverManager();
+        conn = getConnection(true);
     }
 
     @AfterEach
@@ -86,18 +86,20 @@ void tearDown() throws SQLException {
     @ParameterizedTest
     @ValueSource(booleans = { false, true })
     void testIsSegmented(boolean useStreamBlobs) throws SQLException {
-        try (Connection conn = getConnection(useStreamBlobs)) {
-            populateBlob(conn, new byte[] { 1, 2, 3, 4, 5 });
+        if (!useStreamBlobs) {
+            conn.close();
+            conn = getConnection(false);
+        }
+        populateBlob(conn, new byte[] { 1, 2, 3, 4, 5 });
 
-            try (PreparedStatement select = conn.prepareStatement(SELECT_BLOB)) {
-                select.setInt(1, 1);
-                try (ResultSet rs = select.executeQuery()) {
-                    assertTrue(rs.next(), "Expected a row in result set");
-                    FBBlob blob = (FBBlob) rs.getBlob(1);
-                    assertEquals(!useStreamBlobs, blob.isSegmented(),
-                            "Expected a " + (useStreamBlobs ? "stream" : "segmented") + " blob");
-                    blob.free();
-                }
+        try (PreparedStatement select = conn.prepareStatement(SELECT_BLOB)) {
+            select.setInt(1, 1);
+            try (ResultSet rs = select.executeQuery()) {
+                assertTrue(rs.next(), "Expected a row in result set");
+                FBBlob blob = (FBBlob) rs.getBlob(1);
+                assertEquals(!useStreamBlobs, blob.isSegmented(),
+                        "Expected a " + (useStreamBlobs ? "stream" : "segmented") + " blob");
+                blob.free();
             }
         }
     }
@@ -211,9 +213,9 @@ void testSetBytes_long_byteArr_int_int() throws Exception {
 
     @Test
     void testGetBinaryStream_long_long_throwsSQLFeatureNotSupported() throws Exception {
-            Blob blob = conn.createBlob();
+        Blob blob = conn.createBlob();
 
-            assertThrows(SQLFeatureNotSupportedException.class, () -> blob.getBinaryStream(1, 1));
+        assertThrows(SQLFeatureNotSupportedException.class, () -> blob.getBinaryStream(1, 1));
     }
 
     @SuppressWarnings("resource")
@@ -274,19 +276,17 @@ void testGetBlobId_newBlob_throwsSQLException() throws Exception {
 
     @Test
     void testGetBytes_withOffset_streamBlob() throws Exception {
-        try (Connection conn = getConnection(true)) {
-            populateBlob(conn, new byte[] { 1, 2, 3, 4, 5 });
-            try (PreparedStatement select = conn.prepareStatement(SELECT_BLOB)) {
-                select.setInt(1, 1);
-                try (ResultSet rs = select.executeQuery()) {
-                    assertTrue(rs.next(), "Expected a row in result set");
-                    FirebirdBlob blob = (FirebirdBlob) rs.getBlob(1);
+        populateBlob(conn, new byte[] { 1, 2, 3, 4, 5 });
+        try (PreparedStatement select = conn.prepareStatement(SELECT_BLOB)) {
+            select.setInt(1, 1);
+            try (ResultSet rs = select.executeQuery()) {
+                assertTrue(rs.next(), "Expected a row in result set");
+                FirebirdBlob blob = (FirebirdBlob) rs.getBlob(1);
 
-                    byte[] bytes = blob.getBytes(2, 4);
+                byte[] bytes = blob.getBytes(2, 4);
 
-                    assertArrayEquals(new byte[] { 2, 3, 4, 5 }, bytes,
-                            "Expected array equal to original from index 1");
-                }
+                assertArrayEquals(new byte[] { 2, 3, 4, 5 }, bytes,
+                        "Expected array equal to original from index 1");
             }
         }
     }
@@ -310,18 +310,20 @@ void testGetBytes_withOffset_segmentedBlob_throwsSQLException() throws Exception
 
     @ParameterizedTest
     @ValueSource(booleans = { true, false })
-    void testGetBytes_withOffset_streamBlob(boolean useStreamBlobs) throws Exception {
-        try (Connection conn = getConnection(useStreamBlobs)) {
-            byte[] input = { 1, 2, 3, 4, 5 };
-            populateBlob(conn, input);
-            try (var select = conn.prepareStatement(SELECT_BLOB)) {
-                select.setInt(1, 1);
-                try (var rs = select.executeQuery()) {
-                    assertTrue(rs.next(), "Expected a row in result set");
-                    FirebirdBlob blob = (FirebirdBlob) rs.getBlob(1);
+    void testGetBytes(boolean useStreamBlobs) throws Exception {
+        if (!useStreamBlobs) {
+            conn.close();
+            conn = getConnection(false);
+        }
+        byte[] input = { 1, 2, 3, 4, 5 };
+        populateBlob(conn, input);
+        try (var select = conn.prepareStatement(SELECT_BLOB)) {
+            select.setInt(1, 1);
+            try (var rs = select.executeQuery()) {
+                assertTrue(rs.next(), "Expected a row in result set");
+                FirebirdBlob blob = (FirebirdBlob) rs.getBlob(1);
 
-                    assertArrayEquals(input, blob.getBytes(), "Expected array equal to original");
-                }
+                assertArrayEquals(input, blob.getBytes(), "Expected array equal to original");
             }
         }
     }
@@ -344,6 +346,43 @@ void testGetBytes_positionLargerThanMaxIntValue_throwsSQLException() throws Exce
                 sqlState(equalTo(SQLStateConstants.SQL_STATE_INVALID_STRING_LENGTH))));
     }
 
+    @Test
+    void testGetBytes_lengthLongerThanBlobSize() throws Exception {
+        byte[] data = DataGenerator.createRandomBytes(50);
+        populateBlob(conn, data);
+
+        try (var select = conn.prepareStatement(SELECT_BLOB)) {
+            select.setInt(1, 1);
+            try (var rs = select.executeQuery()) {
+                assertTrue(rs.next(), "Expected a row in result set");
+                Blob blob = rs.getBlob(1);
+
+                assertArrayEquals(data, blob.getBytes(1, data.length + 10),
+                        "Expected array equal to original even when requesting larger length");
+            }
+        }
+    }
+
+    @Test
+    void testGetBytes_offsetBeyondEndOfBlob() throws Exception {
+        byte[] data = DataGenerator.createRandomBytes(50);
+        populateBlob(conn, data);
+
+        try (var select = conn.prepareStatement(SELECT_BLOB)) {
+            select.setInt(1, 1);
+            try (var rs = select.executeQuery()) {
+                assertTrue(rs.next(), "Expected a row in result set");
+                Blob blob = rs.getBlob(1);
+
+                // The JDBC API does not state if a position beyond the end-of-blob should be an error condition or not.
+                // Interestingly, it does consider it an error condition for getBinaryStream(long, long), so consider
+                // this "undefined" behaviour.
+                assertArrayEquals(new byte[0], blob.getBytes(data.length + 5, 10), "Expected empty array");
+            }
+        }
+    }
+
+
     /**
      * Checks if the blob close on commit is done successfully.
      * <p>