Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed a bug where calling length() after obtaining a stream would close the stream for Clobs/NClobs #799

Merged
merged 27 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
bb254f2
filling clob if stream is not of type plpinputstream
rene-ye Aug 29, 2018
225c985
fix for simpleinputstream
rene-ye Aug 30, 2018
86a5887
Merge pull request #46 from Microsoft/dev
rene-ye Aug 30, 2018
8f6d197
Added more streaming tests. Changed Clob to use different charset ins…
rene-ye Sep 5, 2018
a6ad39a
cleaning up implementation
rene-ye Sep 7, 2018
9619543
removing unused imports
rene-ye Sep 7, 2018
1ebc48c
added length check to tests and updated length() for clobs/nclobs
rene-ye Sep 7, 2018
72b002c
enabled streaming for getAsciiStream
rene-ye Sep 7, 2018
6b7956d
cleaning up ascii stream implementation.
rene-ye Sep 7, 2018
0785932
length() for NClob
rene-ye Sep 10, 2018
d272502
reverting exposing private methods as protected methods
rene-ye Sep 10, 2018
0cc0b90
update test
rene-ye Sep 12, 2018
26df3df
upped clob/nclob max length, added test for calling length() after re…
rene-ye Sep 14, 2018
4ffee2e
changes addressing reviews
rene-ye Sep 19, 2018
557b4a1
returning bufferedinputstream instead of inputstream
rene-ye Sep 19, 2018
9a0fd0c
formatting
rene-ye Sep 19, 2018
1844970
normalize nclob getAsciiStream behavior
rene-ye Sep 19, 2018
e613240
test fix
rene-ye Sep 19, 2018
c692c68
use getCharacterStream for nclobs
rene-ye Sep 19, 2018
c3fcc9d
removing redundant code
rene-ye Sep 19, 2018
7ebaa44
removing unused imports
rene-ye Sep 21, 2018
1702730
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into is…
rene-ye Sep 21, 2018
df7e79b
changing enum name
rene-ye Sep 21, 2018
9ac2149
refractor while loop
rene-ye Sep 21, 2018
8260ce9
corrected spelling of 'recieved' to 'received'
rene-ye Sep 21, 2018
d3179aa
place rs in try-catch
rene-ye Sep 21, 2018
8746b95
free lobs when not needed
rene-ye Sep 24, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ class PLPInputStream extends BaseInputStream {
static final int PLP_TERMINATOR = 0x00000000;
private final static byte[] EMPTY_PLP_BYTES = new byte[0];

// Stated length of the PLP stream payload; -1 if unknown length.
int payloadLength;

private static final int PLP_EOS = -1;
private int currentChunkRemain;

Expand Down
51 changes: 36 additions & 15 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerClob.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@

package com.microsoft.sqlserver.jdbc;

import static java.nio.charset.StandardCharsets.US_ASCII;
import static java.nio.charset.StandardCharsets.UTF_16LE;

import java.io.BufferedInputStream;
import java.io.BufferedReader;
import java.io.ByteArrayInputStream;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -20,6 +17,7 @@
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
import java.io.Writer;
import java.nio.charset.Charset;
import java.sql.Clob;
import java.sql.SQLException;
import java.text.MessageFormat;
Expand Down Expand Up @@ -152,7 +150,7 @@ abstract class SQLServerClobBase extends SQLServerLob implements Serializable {

// The value of the CLOB that this Clob object represents.
// This value is never null unless/until the free() method is called.
private String value;
protected String value;

private final SQLCollation sqlCollation;

Expand Down Expand Up @@ -181,8 +179,9 @@ final public String toString() {
// Unique id generator for each instance (used for logging).
static private final AtomicInteger baseID = new AtomicInteger(0);

private Charset defaultCharset = null;

// Returns unique id for each instance.

private static int nextInstanceID() {
return baseID.incrementAndGet();
}
Expand Down Expand Up @@ -281,9 +280,20 @@ public InputStream getAsciiStream() throws SQLException {
if (null != sqlCollation && !sqlCollation.supportsAsciiConversion())
DataTypes.throwConversionError(getDisplayClassName(), "AsciiStream");

getStringFromStream();
InputStream getterStream = new BufferedInputStream(
new ReaderInputStream(new StringReader(value), US_ASCII, value.length()));
// Need to use a BufferedInputStream since the stream returned by this method is assumed to support mark/reset
InputStream getterStream;
if (null == value && !activeStreams.isEmpty()) {
InputStream inputStream = (InputStream) activeStreams.get(0);
try {
inputStream.reset();
getterStream = inputStream;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we close 'inputStream' here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same input stream that the user gets, if we close it, the stream returned to the user gets closed.

} catch (IOException e) {
throw new SQLServerException(e.getMessage(), null, 0, e);
}
} else {
getStringFromStream();
getterStream = new ByteArrayInputStream(value.getBytes(java.nio.charset.StandardCharsets.US_ASCII));
}
activeStreams.add(getterStream);
return getterStream;
}
Expand All @@ -301,11 +311,18 @@ public Reader getCharacterStream() throws SQLException {
Reader getterStream = null;
if (null == value && !activeStreams.isEmpty()) {
InputStream inputStream = (InputStream) activeStreams.get(0);
getterStream = new BufferedReader(new InputStreamReader(inputStream, UTF_16LE));
try {
inputStream.reset();
} catch (IOException e) {
throw new SQLServerException(e.getMessage(), null, 0, e);
Copy link
Member

@cheenamalhotra cheenamalhotra Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use "SQLServerException.makeFromDriverError" instead. Also applies to other locations in the changes made.

}
Charset cs = (defaultCharset == null) ? typeInfo.getCharset() : defaultCharset;
getterStream = new BufferedReader(new InputStreamReader(inputStream, cs));
} else {
getStringFromStream();
getterStream = new StringReader(value);
activeStreams.add(getterStream);
}
activeStreams.add(getterStream);
return getterStream;
}

Expand Down Expand Up @@ -381,9 +398,8 @@ public String getSubString(long pos, int length) throws SQLException {
*/
public long length() throws SQLException {
checkClosed();

if (value == null && activeStreams.get(0) instanceof PLPInputStream) {
return (long) ((PLPInputStream) activeStreams.get(0)).payloadLength / 2;
if (value == null && activeStreams.get(0) instanceof BaseInputStream) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null == value

return (long) ((BaseInputStream) activeStreams.get(0)).payloadLength;
}
return value.length();
rene-ye marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -412,7 +428,8 @@ private void getStringFromStream() throws SQLServerException {
} catch (IOException e) {
throw new SQLServerException(e.getMessage(), null, 0, e);
}
value = new String((stream).getBytes(), typeInfo.getCharset());
Charset cs = (defaultCharset == null) ? typeInfo.getCharset() : defaultCharset;
value = new String((stream).getBytes(), cs);
}
}

Expand Down Expand Up @@ -661,6 +678,10 @@ public int setString(long pos, String str, int offset, int len) throws SQLExcept

return len;
}

protected void setDefaultCharset(Charset c) {
this.defaultCharset = c;
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ public final class SQLServerNClob extends SQLServerClobBase implements NClob {

SQLServerNClob(SQLServerConnection connection) {
super(connection, "", connection.getDatabaseCollation(), logger, null);
this.setDefaultCharset(java.nio.charset.StandardCharsets.UTF_16LE);
}

SQLServerNClob(BaseInputStream stream, TypeInfo typeInfo) throws SQLServerException, UnsupportedEncodingException {
super(null, stream, typeInfo.getSQLCollation(), logger, typeInfo);
this.setDefaultCharset(java.nio.charset.StandardCharsets.UTF_16LE);
}

@Override
Expand Down Expand Up @@ -65,7 +67,9 @@ public String getSubString(long pos, int length) throws SQLException {

@Override
public long length() throws SQLException {
return super.length();
// If streaming, every 2 bytes represents 1 character. If not, length() just returns string length
long length = super.length();
return (value == null) ? length / 2 : length;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ abstract class BaseInputStream extends InputStream {

// Flag indicating whether the stream consumes and discards data as it reads it
final boolean isStreaming;

// Stated length of the payload
int payloadLength;

/** Generate the logging ID */
private String parentLoggingInfo = "";
Expand Down Expand Up @@ -131,9 +134,6 @@ void resetHelper() throws IOException {

final class SimpleInputStream extends BaseInputStream {

// Stated length of the payload
private final int payloadLength;

/**
* Initializes the input stream.
*/
Expand Down
Loading