#719 Fix NPE for use after free of FBClob

FirebirdSQL · Dec 5, 2022 · 21b2c0e · 21b2c0e
1 parent bd30bdf
commit 21b2c0e
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 22 deletions.
diff --git a/src/docs/asciidoc/release_notes.adoc b/src/docs/asciidoc/release_notes.adoc
@@ -936,6 +936,7 @@ The replacement only trims trailing spaces.
 ** `getWantsViews(String[])` -- there is no replacement
 * `RowValueBuilder` was moved to another package and (will be) made package private
 // TODO Change once refactor of FBDatabaseMetaData is complete
+* `FBBlob.getGdsHelper()` has been made package private
 
 [#breaking-changes-statement-state]
 ==== Additional statement state `PREPARING`

diff --git a/src/main/org/firebirdsql/jdbc/FBBlob.java b/src/main/org/firebirdsql/jdbc/FBBlob.java
@@ -396,7 +396,15 @@ boolean isNew() {
         }
     }
 
-    public GDSHelper getGdsHelper() {
+    /**
+     * Returns the GDSHelper of this blob.
+     *
+     * @return GDSHelper
+     * @throws SQLException
+     *         If this blob has been freed (closed)
+     */
+    GDSHelper getGdsHelper() throws SQLException {
+        checkClosed();
         return gdsHelper;
     }
 

diff --git a/src/main/org/firebirdsql/jdbc/FBClob.java b/src/main/org/firebirdsql/jdbc/FBClob.java
@@ -86,13 +86,13 @@ public String getSubString(long pos, int length) throws SQLException {
 
 	@Override
 	public Reader getCharacterStream() throws SQLException {
-		String encoding = getWrappedBlob().getGdsHelper().getJavaEncoding();
 		InputStream inputStream = wrappedBlob.getBinaryStream();
+		String encoding = wrappedBlob.getGdsHelper().getJavaEncoding();
 		if (encoding == null) {
 			return new InputStreamReader(inputStream);
 		} else {
 			try {
-				return new InputStreamReader(wrappedBlob.getBinaryStream(), encoding);
+				return new InputStreamReader(inputStream, encoding);
 			} catch (IOException ioe) {
 				throw new FBSQLException(ioe);
 			}
@@ -101,11 +101,7 @@ public Reader getCharacterStream() throws SQLException {
 
 	@Override
 	public InputStream getAsciiStream() throws SQLException {
-		InputStream inputStream = null;
-		if (wrappedBlob != null) {
-			inputStream = wrappedBlob.getBinaryStream();
-		}
-		return inputStream;
+		return wrappedBlob.getBinaryStream();
 	}
 
 	/**
@@ -164,9 +160,9 @@ public OutputStream setAsciiStream(long pos) throws SQLException {
 
 	@Override
 	public Writer setCharacterStream(long position) throws SQLException {
-		String encoding = wrappedBlob.getGdsHelper().getJavaEncoding();
 		// FIXME: This is wrong for multibyte charactersets; doesn't matter right now as setBinaryStream isn't implemented for position > 1
 		OutputStream outputStream = wrappedBlob.setBinaryStream(position);
+		String encoding = wrappedBlob.getGdsHelper().getJavaEncoding();
 		if (encoding == null) {
 			return new OutputStreamWriter(outputStream);
 		} else {
@@ -187,7 +183,7 @@ public void free() throws SQLException {
 	public Reader getCharacterStream(long pos, long length) throws SQLException {
 	    // FIXME: This is wrong for multibyte charactersets; doesn't matter right now as getBinaryStream isn't implemented
 		InputStream inputStream = wrappedBlob.getBinaryStream(pos, length);
-		String encoding = getWrappedBlob().getGdsHelper().getJavaEncoding();
+		String encoding = wrappedBlob.getGdsHelper().getJavaEncoding();
 		if (encoding == null) {
 			return new InputStreamReader(inputStream);
 		} else {

diff --git a/src/main/org/firebirdsql/jdbc/field/FBBlobField.java b/src/main/org/firebirdsql/jdbc/field/FBBlobField.java
@@ -282,12 +282,7 @@ public void setBlob(FBBlob blob) throws SQLException {
 
     @Override
     public void setClob(FBClob clob) throws SQLException {
-        if (clob != null) {
-            FBBlob blob = clob.getWrappedBlob();
-            setBlob(blob);
-        } else {
-            setNull();
-        }
+        setBlob(clob != null ? clob.getWrappedBlob() : null);
     }
 
     @Override

diff --git a/src/main/org/firebirdsql/jdbc/field/FBLongVarCharField.java b/src/main/org/firebirdsql/jdbc/field/FBLongVarCharField.java
@@ -170,12 +170,7 @@ public void setBlob(FBBlob blob) throws SQLException {
 
     @Override
     public void setClob(FBClob clob) throws SQLException {
-        if (clob != null) {
-            FBBlob blob = clob.getWrappedBlob();
-            setBlob(blob);
-        } else {
-            setNull();
-        }
+        setBlob(clob != null ? clob.getWrappedBlob() : null);
     }
 
     @Override

diff --git a/src/test/org/firebirdsql/jdbc/FBClobTest.java b/src/test/org/firebirdsql/jdbc/FBClobTest.java
@@ -363,6 +363,22 @@ void testReadClob_utf8_none() throws Exception {
         }
     }
 
+    @Test
+    void testSQLExceptionAfterFree() throws Exception {
+        final String TEST_VALUE = "TEST_STRING";
+        addTestValues(con, 1, TEST_VALUE, PLAIN_BLOB);
+        try (Statement stmt = con.createStatement();
+             ResultSet resultSet = stmt.executeQuery("SELECT " + PLAIN_BLOB + " FROM test_clob")) {
+            resultSet.next();
+            Clob clob = resultSet.getClob(1);
+            clob.free();
+
+            assertThrows(SQLException.class, clob::getCharacterStream);
+            assertThrows(SQLException.class, () -> clob.getCharacterStream(1, 1));
+            assertThrows(SQLException.class, () -> { try (Writer ignored = clob.setCharacterStream(1)) { } });
+        }
+    }
+
     private String readStringViaGetBytes(Connection con, String colName, String javaEncoding) throws SQLException,
             UnsupportedEncodingException {
         try (PreparedStatement selectStmt = con.prepareStatement("SELECT " + colName + " FROM test_clob");