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

Fixes all statement leaks in the driver. #455

Merged
merged 6 commits into from
Aug 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -1399,11 +1399,12 @@ public NClob getNClob(String parameterName) throws SQLException {
*/
/* L3 */ private int findColumn(String columnName) throws SQLServerException {
if (paramNames == null) {
SQLServerStatement s = null;
try {
// Note we are concatenating the information from the passed in sql, not any arguments provided by the user
// if the user can execute the sql, any fragments of it is potentially executed via the meta data call through injection
// is not a security issue.
SQLServerStatement s = (SQLServerStatement) connection.createStatement();
s = (SQLServerStatement) connection.createStatement();
ThreePartName threePartName = ThreePartName.parse(procedureName);
StringBuilder metaQuery = new StringBuilder("exec sp_sproc_columns ");
if (null != threePartName.getDatabasePart()) {
Expand Down Expand Up @@ -1440,6 +1441,10 @@ public NClob getNClob(String parameterName) throws SQLException {
catch (SQLException e) {
SQLServerException.makeFromDriverError(connection, this, e.toString(), null, false);
}
finally {
if (null != s)
s.close();
}
}

int l = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ private SQLServerResultSet getResultSetFromInternalQueries(String catalog,
SQLServerResultSet rs = null;
try {
rs = ((SQLServerStatement) connection.createStatement()).executeQueryInternal(query);

}
finally {
if (null != orgCat) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,8 @@ private void checkClosed() throws SQLServerException {
assert null != st;
stmtParent = st;
con = st.connection;
SQLServerStatement s = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i guess s doesnt need to be instantiated to null but i did it for consistency

SQLServerStatement stmt = null;
if (logger.isLoggable(java.util.logging.Level.FINE)) {
logger.fine(toString() + " created by (" + st.toString() + ")");
}
Expand All @@ -571,7 +573,7 @@ private void checkClosed() throws SQLServerException {
// If the CallableStatement/PreparedStatement is a stored procedure call
// then we can extract metadata using sp_sproc_columns
if (null != st.procedureName) {
SQLServerStatement s = (SQLServerStatement) con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
s = (SQLServerStatement) con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
String sProc = parseProcIdentifier(st.procedureName);
if (con.isKatmaiOrLater())
rsProcedureMeta = s.executeQueryInternal("exec sp_sproc_columns_100 " + sProc + ", @ODBCVer=3");
Expand Down Expand Up @@ -659,13 +661,11 @@ private void checkClosed() throws SQLServerException {

String tablesAndJoins = sbTablesAndJoins.toString();

Statement stmt = con.createStatement();
stmt = (SQLServerStatement) con.createStatement();
String sCom = "sp_executesql N'SET FMTONLY ON SELECT " + columns + " FROM " + tablesAndJoins + " '";

ResultSet rs = stmt.executeQuery(sCom);
parseQueryMetaFor2008(rs);
stmt.close();
rs.close();
}
}
}
Expand All @@ -679,6 +679,10 @@ private void checkClosed() throws SQLServerException {
catch(StringIndexOutOfBoundsException e){
SQLServerException.makeFromDriverError(con, stmtParent, e.toString(), null, false);
}
finally {
if (null != stmt)
stmt.close();
}
}

public boolean isWrapperFor(Class<?> iface) throws SQLException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ public class SQLServerPreparedStatement extends SQLServerStatement implements IS

/** The prepared statement handle returned by the server */
private int prepStmtHandle = 0;

/** Statement used for getMetadata(). Declared as a field to facilitate closing the statement. */
private SQLServerStatement internalStmt = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

having internalStmt as a field is the only way to close it after the user is done working with the ResultsetMetadata.


private void setPreparedStatementHandle(int handle) {
this.prepStmtHandle = handle;
Expand Down Expand Up @@ -271,6 +274,18 @@ final void closeInternal() {

// If we have a prepared statement handle, close it.
closePreparedHandle();

// Close the statement that was used to generate empty statement from getMetadata().
try {
if (null != internalStmt)
internalStmt.close();
} catch (SQLServerException e) {
if (loggerExternal.isLoggable(java.util.logging.Level.FINER))
loggerExternal.finer("Ignored error closing internal statement: " + e.getErrorCode() + " " + e.getMessage());
}
finally {
internalStmt = null;
}

// Clean up client-side state
batchParamValues = null;
Expand Down Expand Up @@ -1023,8 +1038,8 @@ else if (resultSet != null) {
ResultSet emptyResultSet = null;
try {
fmtSQL = replaceMarkerWithNull(fmtSQL);
SQLServerStatement stmt = (SQLServerStatement) connection.createStatement();
emptyResultSet = stmt.executeQueryInternal("set fmtonly on " + fmtSQL + "\nset fmtonly off");
internalStmt = (SQLServerStatement) connection.createStatement();
emptyResultSet = internalStmt.executeQueryInternal("set fmtonly on " + fmtSQL + "\nset fmtonly off");
}
catch (SQLException sqle) {
if (false == sqle.getMessage().equals(SQLServerException.getErrString("R_noResultset"))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ private String typeDisplay(int type) {
case XA_START:

if (!serverInfoRetrieved) {
Statement stmt = null;
try {
serverInfoRetrieved = true;
// data are converted to varchar as type variant returned by SERVERPROPERTY is not supported by driver
Expand All @@ -453,7 +454,7 @@ private String typeDisplay(int type) {
+ " convert(varchar(100), SERVERPROPERTY('ProductVersion')) as version,"
+ " SUBSTRING(@@VERSION, CHARINDEX('<', @@VERSION)+2, 2)";

Statement stmt = controlConnection.createStatement();
stmt = controlConnection.createStatement();
ResultSet rs = stmt.executeQuery(query);
rs.next();

Expand All @@ -475,14 +476,23 @@ else if (-1 != version.indexOf('.')) {
ArchitectureOS = Integer.parseInt(rs.getString(4));

rs.close();
stmt.close();
}
// Got caught in static analysis. Catch only the thrown exceptions, do not catch
// run time exceptions.
catch (Exception e) {
if (xaLogger.isLoggable(Level.WARNING))
xaLogger.warning(toString() + " Cannot retrieve server information: :" + e.getMessage());
}
finally {
if (null != stmt)
try {
stmt.close();
}
catch (SQLException e) {
if (xaLogger.isLoggable(Level.FINER))
xaLogger.finer(toString());
}
}
}

sContext = "START:";
Expand Down