From 475848596cba6a4172dde02cb214aa6b6202ff3d Mon Sep 17 00:00:00 2001 From: rene-ye Date: Wed, 3 Jan 2018 11:40:16 -0800 Subject: [PATCH 01/16] Blob fix Fills the contents of a blob and makes it availible for use after the RS or statement has been closed. Addresses the TDS issue from #567. --- .../sqlserver/jdbc/SQLServerBlob.java | 9 +++++++ .../sqlserver/jdbc/SQLServerResultSet.java | 27 ++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java index 47927d6a6..094c9f863 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java @@ -236,6 +236,15 @@ public long length() throws SQLException { return value.length; } + /** + * Public function for the result set to maintain blobs it has created + * @throws SQLException + */ + public void fillByteArray() throws SQLException { + if(!isClosed) + getBytesFromStream(); + } + /** * Converts stream to byte[] * @throws SQLServerException diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index 9b7933705..0d958553b 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -26,7 +26,9 @@ import java.sql.SQLWarning; import java.sql.SQLXML; import java.text.MessageFormat; +import java.util.ArrayList; import java.util.Calendar; +import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -126,6 +128,7 @@ final void setCurrentRowType(RowType rowType) { * occurs */ private Closeable activeStream; + private List streamObjects = new ArrayList(); /** * A window of fetchSize quickly accessible rows for scrollable result sets @@ -576,7 +579,10 @@ private void closeInternal() { // Mark this ResultSet as closed, then clean up. isClosed = true; - + + //fill all unclosed objects which rely on the stream + fillBlobs(); + // Discard the current fetch buffer contents. discardFetchBuffer(); @@ -2664,6 +2670,7 @@ public Blob getBlob(int i) throws SQLServerException { checkClosed(); Blob value = (Blob) getValue(i, JDBCType.BLOB); loggerExternal.exiting(getClassNameLogging(), "getBlob", value); + streamObjects.add(value); return value; } @@ -2672,6 +2679,7 @@ public Blob getBlob(String colName) throws SQLServerException { checkClosed(); Blob value = (Blob) getValue(findColumn(colName), JDBCType.BLOB); loggerExternal.exiting(getClassNameLogging(), "getBlob", value); + streamObjects.add(value); return value; } @@ -6514,6 +6522,23 @@ final void doServerFetch(int fetchType, scrollWindow.reset(); } } + + /* + * Iterates through the list of objects which rely on the stream that's about to be closed, filling them with their data + * Will skip over closed blobs, implemented in SQLServerBlob + */ + private void fillBlobs() { + for(Object o : streamObjects) { + if(o instanceof SQLServerBlob) { + try { + ((SQLServerBlob) o).fillByteArray(); + } catch (SQLException e) { + if (logger.isLoggable(java.util.logging.Level.FINER)) + logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); + } + } + } + } /** * Discards the contents of the current fetch buffer. From 5f96797f8308975b808e27a7fa63000686c65b63 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 4 Jan 2018 11:05:12 -0800 Subject: [PATCH 02/16] changed function accessability --- src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java index 094c9f863..3ddb57aac 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java @@ -237,10 +237,10 @@ public long length() throws SQLException { } /** - * Public function for the result set to maintain blobs it has created + * Function for the result set to maintain blobs it has created * @throws SQLException */ - public void fillByteArray() throws SQLException { + void fillByteArray() throws SQLException { if(!isClosed) getBytesFromStream(); } From 83da731f18e597e8ff812b2e8fa107d8a956979c Mon Sep 17 00:00:00 2001 From: rene-ye Date: Fri, 26 Jan 2018 10:02:33 -0800 Subject: [PATCH 03/16] Length fix Fix to issue #611, where calling length() would close the stream. --- src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java index 3ddb57aac..6d23038f9 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java @@ -232,6 +232,8 @@ public byte[] getBytes(long pos, */ public long length() throws SQLException { checkClosed(); + if (value == null) + return (long)((PLPInputStream)activeStreams.get(0)).payloadLength; getBytesFromStream(); return value.length; } From 923c2207ea1d283f675305f5327fc273eed863b5 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 8 Feb 2018 11:13:37 -0800 Subject: [PATCH 04/16] Kerberos DC fix for automatic credential discarding --- .../sqlserver/jdbc/SQLServerConnection.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index c8232d71a..6b5fc59ea 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -3428,19 +3428,24 @@ final boolean doExecute() throws SQLServerException { } } finally { - if (integratedSecurity) { - if (null != authentication) - authentication.ReleaseClientContext(); - authentication = null; - + if (integratedSecurity) { if (null != ImpersonatedUserCred) { try { - ImpersonatedUserCred.dispose(); + if (ImpersonatedUserCred.getRemainingLifetime() <= 0) { + if (null != authentication) + authentication.ReleaseClientContext(); + authentication = null; + ImpersonatedUserCred.dispose(); + } } catch (GSSException e) { if (connectionlogger.isLoggable(Level.FINER)) connectionlogger.finer(toString() + " Release of the credentials failed GSSException: " + e); } + } else { + if (null != authentication) + authentication.ReleaseClientContext(); + authentication = null; } } } From 25301e6fa042661d4ee1c9dbc8d9cd23ebff8eb7 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Mon, 5 Feb 2018 13:45:14 -0800 Subject: [PATCH 05/16] Added information to error message To debug a random mismatch error which can't be reproduced locally. --- .../sqlserver/jdbc/SQLServerConnection.java | 17 +++++++++++------ .../testframework/util/ComparisonUtil.java | 10 +++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index c8232d71a..6b5fc59ea 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -3428,19 +3428,24 @@ final boolean doExecute() throws SQLServerException { } } finally { - if (integratedSecurity) { - if (null != authentication) - authentication.ReleaseClientContext(); - authentication = null; - + if (integratedSecurity) { if (null != ImpersonatedUserCred) { try { - ImpersonatedUserCred.dispose(); + if (ImpersonatedUserCred.getRemainingLifetime() <= 0) { + if (null != authentication) + authentication.ReleaseClientContext(); + authentication = null; + ImpersonatedUserCred.dispose(); + } } catch (GSSException e) { if (connectionlogger.isLoggable(Level.FINER)) connectionlogger.finer(toString() + " Release of the credentials failed GSSException: " + e); } + } else { + if (null != authentication) + authentication.ReleaseClientContext(); + authentication = null; } } } diff --git a/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java b/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java index c942f8c9a..b297a3924 100644 --- a/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java +++ b/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java @@ -92,16 +92,16 @@ public static void compareExpectedAndActual(int dataType, else switch (dataType) { case java.sql.Types.BIGINT: - assertTrue((((Long) expectedValue).longValue() == ((Long) actualValue).longValue()), "Unexpected bigint value"); + assertTrue((((Long) expectedValue).longValue() == ((Long) actualValue).longValue()), "Unexpected bigint value. Expected:" + ((Long) expectedValue).longValue() + " Actual:" + ((Long) actualValue).longValue()); break; case java.sql.Types.INTEGER: - assertTrue((((Integer) expectedValue).intValue() == ((Integer) actualValue).intValue()), "Unexpected int value"); + assertTrue((((Integer) expectedValue).intValue() == ((Integer) actualValue).intValue()), "Unexpected int value. Expected:" + ((Integer) expectedValue).intValue() + " Actual:" + ((Integer) actualValue).intValue()); break; case java.sql.Types.SMALLINT: case java.sql.Types.TINYINT: - assertTrue((((Short) expectedValue).shortValue() == ((Short) actualValue).shortValue()), "Unexpected smallint/tinyint value"); + assertTrue((((Short) expectedValue).shortValue() == ((Short) actualValue).shortValue()), "Unexpected smallint/tinyint value. Expected:" + ((Short) expectedValue).shortValue() + " Actual:" + ((Short) actualValue).shortValue()); break; case java.sql.Types.BIT: @@ -115,11 +115,11 @@ public static void compareExpectedAndActual(int dataType, break; case java.sql.Types.DOUBLE: - assertTrue((((Double) expectedValue).doubleValue() == ((Double) actualValue).doubleValue()), "Unexpected float value"); + assertTrue((((Double) expectedValue).doubleValue() == ((Double) actualValue).doubleValue()), "Unexpected double value. Expected:" + ((Double) expectedValue).doubleValue() + " Actual:" + ((Double) actualValue).doubleValue()); break; case java.sql.Types.REAL: - assertTrue((((Float) expectedValue).floatValue() == ((Float) actualValue).floatValue()), "Unexpected real value"); + assertTrue((((Float) expectedValue).floatValue() == ((Float) actualValue).floatValue()), "Unexpected real/float value. Expected:" + ((Float) expectedValue).floatValue() + " Actual:" + ((Float) actualValue).floatValue()); break; case java.sql.Types.VARCHAR: From 91bf4d36423cd3ae8f6c88e6de78a79581b712d4 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 10:07:54 -0800 Subject: [PATCH 06/16] minor stream verification --- src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java index 6d23038f9..be5a9e465 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java @@ -232,7 +232,7 @@ public byte[] getBytes(long pos, */ public long length() throws SQLException { checkClosed(); - if (value == null) + if (value == null && activeStreams.get(0) instanceof PLPInputStream) return (long)((PLPInputStream)activeStreams.get(0)).payloadLength; getBytesFromStream(); return value.length; From 3ba5f1be4e54361148915671cbbeaba6a0c7b9c2 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 13:18:13 -0800 Subject: [PATCH 07/16] Reimplemented Blob fixes removed arraylist to avoid costly traversal. Blobs now fill anytime the resultset moves its cursor. --- .../sqlserver/jdbc/SQLServerResultSet.java | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index 7c9c710bb..cb5442511 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -128,7 +128,7 @@ final void setCurrentRowType(RowType rowType) { * occurs */ private Closeable activeStream; - private List streamObjects = new ArrayList(); + private Blob activeBlob; /** * A window of fetchSize quickly accessible rows for scrollable result sets @@ -436,6 +436,8 @@ public T unwrap(Class iface) throws SQLException { // can be done with it other than closing it. if (null != rowErrorException) throw rowErrorException; + + fillBlobs(); } public boolean isClosed() throws SQLException { @@ -675,6 +677,7 @@ final Column getColumn(int columnIndex) throws SQLServerException { // before moving to another one. if (null != activeStream) { try { + fillBlobs(); activeStream.close(); } catch (IOException e) { @@ -2663,7 +2666,7 @@ public Blob getBlob(int i) throws SQLServerException { checkClosed(); Blob value = (Blob) getValue(i, JDBCType.BLOB); loggerExternal.exiting(getClassNameLogging(), "getBlob", value); - streamObjects.add(value); + activeBlob = value; return value; } @@ -2672,7 +2675,7 @@ public Blob getBlob(String colName) throws SQLServerException { checkClosed(); Blob value = (Blob) getValue(findColumn(colName), JDBCType.BLOB); loggerExternal.exiting(getClassNameLogging(), "getBlob", value); - streamObjects.add(value); + activeBlob = value; return value; } @@ -6517,20 +6520,32 @@ final void doServerFetch(int fetchType, } /* - * Iterates through the list of objects which rely on the stream that's about to be closed, filling them with their data + * Iterates through the list of objects which rely on the stream that's about to be closed, ing them with their data * Will skip over closed blobs, implemented in SQLServerBlob */ private void fillBlobs() { - for(Object o : streamObjects) { - if(o instanceof SQLServerBlob) { - try { - ((SQLServerBlob) o).fillByteArray(); + if (activeBlob != null && activeBlob instanceof SQLServerBlob) { + try { + ((SQLServerBlob)activeBlob).fillByteArray(); + } catch (SQLException e) { + if (logger.isLoggable(java.util.logging.Level.FINER)) + logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); + } finally { + activeBlob = null; + } + } + + /*for (int i = 0; i < streamObjects.size(); i++) { + if(streamObjects.get(i) instanceof SQLServerBlob) { + try { + ((SQLServerBlob) streamObjects.get(i)).fillByteArray(); + streamObjects.remove(i); } catch (SQLException e) { if (logger.isLoggable(java.util.logging.Level.FINER)) logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); } } - } + }*/ } /** From 9a1300ed1bc1036771e0f4b9a3f8c07007ee8a67 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 13:18:47 -0800 Subject: [PATCH 08/16] Additional tests added tests which specifically test blob streams and behavior after the RS is closed. --- .../sqlserver/jdbc/unit/lobs/lobsTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java index af9499fa9..26a4ec266 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java @@ -413,6 +413,92 @@ public void testUpdatorClob() throws Exception { String types[] = {"varchar(max)"}; testUpdateLobs(types, Clob.class); } + + @Test + @DisplayName("readBlobStreamAfterClosingRS") + public void readBlobStreamAfterClosingRS() throws Exception { + String types[] = {"varbinary(max)"}; + table = createTable(table, types, false); // create empty table + int size = 10000; + + byte[] data = new byte[size]; + ThreadLocalRandom.current().nextBytes(data); + + Blob blob = null; + InputStream stream = null; + PreparedStatement ps = conn.prepareStatement("INSERT INTO " + table.getEscapedTableName() + " VALUES(?)"); + blob = conn.createBlob(); + blob.setBytes(1, data); + ps.setBlob(1, blob); + ps.executeUpdate(); + + byte[] chunk = new byte[size]; + ResultSet rs = stmt.executeQuery("select * from " + table.getEscapedTableName()); + rs.next(); + + blob = rs.getBlob(1); + stream = blob.getBinaryStream(); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int read = 0; + while ((read = stream.read(chunk)) > 0) + buffer.write(chunk, 0, read); + assertEquals(chunk.length, size); + rs.close(); + stream = blob.getBinaryStream(); + buffer = new ByteArrayOutputStream(); + read = 0; + while ((read = stream.read(chunk)) > 0) + buffer.write(chunk, 0, read); + assertEquals(chunk.length, size); + + if (null != blob) + blob.free(); + dropTables(table); + } + + @Test + @DisplayName("readMultipleBlobStreamsThenCloseRS") + public void readMultipleBlobStreamsThenCloseRS() throws Exception { + String types[] = {"varbinary(max)"}; + table = createTable(table, types, false); + int size = 10000; + + byte[] data = new byte[size]; + Blob[] blobs = {null, null, null}; + InputStream stream = null; + for (int i = 0; i < 3; i++) + { + PreparedStatement ps = conn.prepareStatement("INSERT INTO " + table.getEscapedTableName() + " VALUES(?)"); + blobs[i] = conn.createBlob(); + ThreadLocalRandom.current().nextBytes(data); + blobs[i].setBytes(1, data); + ps.setBlob(1, blobs[i]); + ps.executeUpdate(); + } + byte[] chunk = new byte[size]; + ResultSet rs = stmt.executeQuery("select * from " + table.getEscapedTableName()); + for (int i = 0; i < 3; i++) + { + rs.next(); + blobs[i] = rs.getBlob(1); + stream = blobs[i].getBinaryStream(); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int read = 0; + while ((read = stream.read(chunk)) > 0) + buffer.write(chunk, 0, read); + assertEquals(chunk.length, size); + } + rs.close(); + for (int i = 0; i < 3; i++) + { + stream = blobs[i].getBinaryStream(); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int read = 0; + while ((read = stream.read(chunk)) > 0) + buffer.write(chunk, 0, read); + assertEquals(chunk.length, size); + } + } private void testUpdateLobs(String types[], Class lobClass) throws Exception { From 19e833912ff85991285bbdc352d6c02efe5a0dbf Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 13:23:44 -0800 Subject: [PATCH 09/16] merge conflict #1 --- .../microsoft/sqlserver/jdbc/SQLServerResultSet.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index cb5442511..6e8e064a4 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -6534,18 +6534,6 @@ private void fillBlobs() { activeBlob = null; } } - - /*for (int i = 0; i < streamObjects.size(); i++) { - if(streamObjects.get(i) instanceof SQLServerBlob) { - try { - ((SQLServerBlob) streamObjects.get(i)).fillByteArray(); - streamObjects.remove(i); - } catch (SQLException e) { - if (logger.isLoggable(java.util.logging.Level.FINER)) - logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); - } - } - }*/ } /** From 09fe7e4ae5021e6d553116b5ecfdcc0615ec096b Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 14:22:11 -0800 Subject: [PATCH 10/16] Revert "Merge branch 'allChangesForTesting' into blobStream" This reverts commit bc074f23860686ce79887b36cb5b64fe23372551, reversing changes made to 19e833912ff85991285bbdc352d6c02efe5a0dbf. --- .../jdbc/SQLServerCallableStatement.java | 2 -- .../sqlserver/jdbc/SQLServerConnection.java | 17 ++++++----------- .../sqlserver/jdbc/SQLServerResultSet.java | 2 +- .../testframework/util/ComparisonUtil.java | 10 +++++----- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerCallableStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerCallableStatement.java index c30eab2fa..5ad8689c8 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerCallableStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerCallableStatement.java @@ -1450,8 +1450,6 @@ public NClob getNClob(String parameterName) throws SQLException { int l = 0; if (paramNames != null) l = paramNames.size(); - if (l == 0)//Server didn't return anything, user might not have access - return 1;//attempting to look up the first column will return no access exception // handle `@name` as well as `name`, since `@name` is what's returned // by DatabaseMetaData#getProcedureColumns diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 587ffb75e..432aa9e32 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -3420,24 +3420,19 @@ final boolean doExecute() throws SQLServerException { } } finally { - if (integratedSecurity) { + if (integratedSecurity) { + if (null != authentication) + authentication.ReleaseClientContext(); + authentication = null; + if (null != ImpersonatedUserCred) { try { - if (ImpersonatedUserCred.getRemainingLifetime() <= 0) { - if (null != authentication) - authentication.ReleaseClientContext(); - authentication = null; - ImpersonatedUserCred.dispose(); - } + ImpersonatedUserCred.dispose(); } catch (GSSException e) { if (connectionlogger.isLoggable(Level.FINER)) connectionlogger.finer(toString() + " Release of the credentials failed GSSException: " + e); } - } else { - if (null != authentication) - authentication.ReleaseClientContext(); - authentication = null; } } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index a89ac419b..6e8e064a4 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -6520,7 +6520,7 @@ final void doServerFetch(int fetchType, } /* - * Iterates through the list of objects which rely on the stream that's about to be closed, filling them with their data + * Iterates through the list of objects which rely on the stream that's about to be closed, ing them with their data * Will skip over closed blobs, implemented in SQLServerBlob */ private void fillBlobs() { diff --git a/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java b/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java index b297a3924..c942f8c9a 100644 --- a/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java +++ b/src/test/java/com/microsoft/sqlserver/testframework/util/ComparisonUtil.java @@ -92,16 +92,16 @@ public static void compareExpectedAndActual(int dataType, else switch (dataType) { case java.sql.Types.BIGINT: - assertTrue((((Long) expectedValue).longValue() == ((Long) actualValue).longValue()), "Unexpected bigint value. Expected:" + ((Long) expectedValue).longValue() + " Actual:" + ((Long) actualValue).longValue()); + assertTrue((((Long) expectedValue).longValue() == ((Long) actualValue).longValue()), "Unexpected bigint value"); break; case java.sql.Types.INTEGER: - assertTrue((((Integer) expectedValue).intValue() == ((Integer) actualValue).intValue()), "Unexpected int value. Expected:" + ((Integer) expectedValue).intValue() + " Actual:" + ((Integer) actualValue).intValue()); + assertTrue((((Integer) expectedValue).intValue() == ((Integer) actualValue).intValue()), "Unexpected int value"); break; case java.sql.Types.SMALLINT: case java.sql.Types.TINYINT: - assertTrue((((Short) expectedValue).shortValue() == ((Short) actualValue).shortValue()), "Unexpected smallint/tinyint value. Expected:" + ((Short) expectedValue).shortValue() + " Actual:" + ((Short) actualValue).shortValue()); + assertTrue((((Short) expectedValue).shortValue() == ((Short) actualValue).shortValue()), "Unexpected smallint/tinyint value"); break; case java.sql.Types.BIT: @@ -115,11 +115,11 @@ public static void compareExpectedAndActual(int dataType, break; case java.sql.Types.DOUBLE: - assertTrue((((Double) expectedValue).doubleValue() == ((Double) actualValue).doubleValue()), "Unexpected double value. Expected:" + ((Double) expectedValue).doubleValue() + " Actual:" + ((Double) actualValue).doubleValue()); + assertTrue((((Double) expectedValue).doubleValue() == ((Double) actualValue).doubleValue()), "Unexpected float value"); break; case java.sql.Types.REAL: - assertTrue((((Float) expectedValue).floatValue() == ((Float) actualValue).floatValue()), "Unexpected real/float value. Expected:" + ((Float) expectedValue).floatValue() + " Actual:" + ((Float) actualValue).floatValue()); + assertTrue((((Float) expectedValue).floatValue() == ((Float) actualValue).floatValue()), "Unexpected real value"); break; case java.sql.Types.VARCHAR: From b071f239fb1e6f92bdb354021f4721eaab0dd13b Mon Sep 17 00:00:00 2001 From: rene-ye Date: Thu, 22 Feb 2018 14:23:31 -0800 Subject: [PATCH 11/16] comment edit --- .../java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index 6e8e064a4..a89ac419b 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -6520,7 +6520,7 @@ final void doServerFetch(int fetchType, } /* - * Iterates through the list of objects which rely on the stream that's about to be closed, ing them with their data + * Iterates through the list of objects which rely on the stream that's about to be closed, filling them with their data * Will skip over closed blobs, implemented in SQLServerBlob */ private void fillBlobs() { From 4d91bc9f14be9a4302523cef5ebf3a3132991187 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Fri, 23 Feb 2018 10:23:23 -0800 Subject: [PATCH 12/16] slight adjustments to implementation No longer fills blobs on every checkClosed() call. Only attempts to fill them in scenarios where the cursor moves or another change of state which causes the RS to lose its active stream. --- .../sqlserver/jdbc/SQLServerResultSet.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index a89ac419b..e5ae74ea1 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -436,8 +436,6 @@ public T unwrap(Class iface) throws SQLException { // can be done with it other than closing it. if (null != rowErrorException) throw rowErrorException; - - fillBlobs(); } public boolean isClosed() throws SQLException { @@ -582,9 +580,6 @@ private void closeInternal() { // Mark this ResultSet as closed, then clean up. isClosed = true; - //fill all unclosed objects which rely on the stream - fillBlobs(); - // Discard the current fetch buffer contents. discardFetchBuffer(); @@ -619,7 +614,7 @@ public void close() throws SQLServerException { public int findColumn(String columnName) throws SQLServerException { loggerExternal.entering(getClassNameLogging(), "findColumn", columnName); checkClosed(); - + // In order to be as accurate as possible when locating column name // indexes, as well as be deterministic when running on various client // locales, we search for column names using the following scheme: @@ -756,6 +751,7 @@ private Column loadColumn(int index) throws SQLServerException { /* ----------------- JDBC API methods ------------------ */ private void moverInit() throws SQLServerException { + fillBlobs(); cancelInsert(); cancelUpdates(); } @@ -1901,6 +1897,7 @@ Column getterGetColumn(int index) throws SQLServerException { if (logger.isLoggable(java.util.logging.Level.FINER)) logger.finer(toString() + " Getting Column:" + index); + fillBlobs(); return loadColumn(index); } @@ -6524,7 +6521,7 @@ final void doServerFetch(int fetchType, * Will skip over closed blobs, implemented in SQLServerBlob */ private void fillBlobs() { - if (activeBlob != null && activeBlob instanceof SQLServerBlob) { + if (null != activeBlob && activeBlob instanceof SQLServerBlob) { try { ((SQLServerBlob)activeBlob).fillByteArray(); } catch (SQLException e) { @@ -6547,6 +6544,9 @@ private void fillBlobs() { * fetch buffer is considered to be discarded. */ private void discardFetchBuffer() { + //fills blobs before discarding anything + fillBlobs(); + // Clear the TDSReader mark at the start of the fetch buffer fetchBuffer.clearStartMark(); From 48b9b85254aefe8d24b498910e11940bc2288873 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Fri, 23 Feb 2018 13:18:57 -0800 Subject: [PATCH 13/16] SimpleInputStream fix wasNull() moves the cursor for some reason, need to fill blobs. --- .../java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index e5ae74ea1..fab346e31 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -1040,6 +1040,7 @@ public boolean next() throws SQLServerException { public boolean wasNull() throws SQLServerException { loggerExternal.entering(getClassNameLogging(), "wasNull"); checkClosed(); + fillBlobs(); loggerExternal.exiting(getClassNameLogging(), "wasNull", lastValueWasNull); return lastValueWasNull; } @@ -1413,7 +1414,7 @@ public int getRow() throws SQLServerException { logger.finer(toString() + logCursorState()); checkClosed(); - + // DYNAMIC (scrollable) cursors do not support getRow() since they do not have any // concept of absolute position. if (isDynamic() && !isForwardOnly()) From ca0e6c0368cb40ea6554160441c164dbc61b45d1 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Wed, 28 Feb 2018 15:32:01 -0800 Subject: [PATCH 14/16] Added a free() test testing expected behavior if users release a blob() object --- .../sqlserver/jdbc/unit/lobs/lobsTest.java | 61 +++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java index 26a4ec266..232eb49b7 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/lobsTest.java @@ -221,6 +221,59 @@ else if (lobClass == Blob.class) } } + @Test + @DisplayName("testFreedBlobs") + private void testFreedBlobs(Class lobClass, + boolean isResultSet) throws SQLException { + String types[] = {"varbinary(max)"}; + try { + table = createTable(table, types, false); // create empty table + int size = 10000; + + byte[] data = new byte[size]; + ThreadLocalRandom.current().nextBytes(data); + + Blob blob = null; + InputStream stream = null; + for (int i = 0; i < 5; i++) + { + PreparedStatement ps = conn.prepareStatement("INSERT INTO " + table.getEscapedTableName() + " VALUES(?)"); + blob = conn.createBlob(); + blob.setBytes(1, data); + ps.setBlob(1, blob); + ps.executeUpdate(); + } + + byte[] chunk = new byte[size]; + ResultSet rs = stmt.executeQuery("select * from " + table.getEscapedTableName()); + for (int i = 0; i < 5; i++) + { + rs.next(); + + blob = rs.getBlob(1); + stream = blob.getBinaryStream(); + while (stream.available() > 0) + stream.read(); + blob.free(); + try { + stream = blob.getBinaryStream(); + } catch (SQLException e) { + assertTrue(e.getMessage().contains("This Blob object has been freed.")); + } + } + rs.close(); + try { + stream = blob.getBinaryStream(); + } catch (SQLException e) { + assertTrue(e.getMessage().contains("This Blob object has been freed.")); + } + } + catch (Exception e) { + this.dropTables(table); + e.printStackTrace(); + } + } + @Test @DisplayName("testMultipleCloseCharacterStream") public void testMultipleCloseCharacterStream() throws Exception { @@ -464,9 +517,9 @@ public void readMultipleBlobStreamsThenCloseRS() throws Exception { int size = 10000; byte[] data = new byte[size]; - Blob[] blobs = {null, null, null}; + Blob[] blobs = {null, null, null, null, null}; InputStream stream = null; - for (int i = 0; i < 3; i++) + for (int i = 0; i < 5; i++)//create 5 blobs { PreparedStatement ps = conn.prepareStatement("INSERT INTO " + table.getEscapedTableName() + " VALUES(?)"); blobs[i] = conn.createBlob(); @@ -477,7 +530,7 @@ public void readMultipleBlobStreamsThenCloseRS() throws Exception { } byte[] chunk = new byte[size]; ResultSet rs = stmt.executeQuery("select * from " + table.getEscapedTableName()); - for (int i = 0; i < 3; i++) + for (int i = 0; i < 5; i++) { rs.next(); blobs[i] = rs.getBlob(1); @@ -489,7 +542,7 @@ public void readMultipleBlobStreamsThenCloseRS() throws Exception { assertEquals(chunk.length, size); } rs.close(); - for (int i = 0; i < 3; i++) + for (int i = 0; i < 5; i++) { stream = blobs[i].getBinaryStream(); ByteArrayOutputStream buffer = new ByteArrayOutputStream(); From 625a721264c26209feffdf1fb8417bc4197a14fe Mon Sep 17 00:00:00 2001 From: rene-ye Date: Tue, 6 Mar 2018 10:57:52 -0800 Subject: [PATCH 15/16] code style changes formatting --- .../sqlserver/jdbc/SQLServerBlob.java | 6 +++-- .../sqlserver/jdbc/SQLServerResultSet.java | 27 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java index be5a9e465..9dc401717 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java @@ -232,8 +232,9 @@ public byte[] getBytes(long pos, */ public long length() throws SQLException { checkClosed(); - if (value == null && activeStreams.get(0) instanceof PLPInputStream) + if (value == null && activeStreams.get(0) instanceof PLPInputStream) { return (long)((PLPInputStream)activeStreams.get(0)).payloadLength; + } getBytesFromStream(); return value.length; } @@ -243,8 +244,9 @@ public long length() throws SQLException { * @throws SQLException */ void fillByteArray() throws SQLException { - if(!isClosed) + if(!isClosed) { getBytesFromStream(); + } } /** diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index fab346e31..acd1864ff 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -26,9 +26,7 @@ import java.sql.SQLWarning; import java.sql.SQLXML; import java.text.MessageFormat; -import java.util.ArrayList; import java.util.Calendar; -import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -614,7 +612,7 @@ public void close() throws SQLServerException { public int findColumn(String columnName) throws SQLServerException { loggerExternal.entering(getClassNameLogging(), "findColumn", columnName); checkClosed(); - + // In order to be as accurate as possible when locating column name // indexes, as well as be deterministic when running on various client // locales, we search for column names using the following scheme: @@ -1414,7 +1412,7 @@ public int getRow() throws SQLServerException { logger.finer(toString() + logCursorState()); checkClosed(); - + // DYNAMIC (scrollable) cursors do not support getRow() since they do not have any // concept of absolute position. if (isDynamic() && !isForwardOnly()) @@ -6522,17 +6520,18 @@ final void doServerFetch(int fetchType, * Will skip over closed blobs, implemented in SQLServerBlob */ private void fillBlobs() { - if (null != activeBlob && activeBlob instanceof SQLServerBlob) { + if (null != activeBlob && activeBlob instanceof SQLServerBlob) { try { - ((SQLServerBlob)activeBlob).fillByteArray(); - } catch (SQLException e) { - if (logger.isLoggable(java.util.logging.Level.FINER)) - logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); - } finally { - activeBlob = null; - } + ((SQLServerBlob)activeBlob).fillByteArray(); + } catch (SQLException e) { + if (logger.isLoggable(java.util.logging.Level.FINER)) { + logger.finer(toString() + "Filling blobs before closing: " + e.getMessage()); + } + } finally { + activeBlob = null; + } } - } + } /** * Discards the contents of the current fetch buffer. @@ -6547,7 +6546,7 @@ private void fillBlobs() { private void discardFetchBuffer() { //fills blobs before discarding anything fillBlobs(); - + // Clear the TDSReader mark at the start of the fetch buffer fetchBuffer.clearStartMark(); From 2512abf2c157bb53e4d1257177ee3e85511ccc22 Mon Sep 17 00:00:00 2001 From: rene-ye Date: Tue, 6 Mar 2018 10:58:52 -0800 Subject: [PATCH 16/16] remove indent --- .../java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java index acd1864ff..cdd5e8278 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java @@ -577,7 +577,7 @@ private void closeInternal() { // Mark this ResultSet as closed, then clean up. isClosed = true; - + // Discard the current fetch buffer contents. discardFetchBuffer();