From b2947c5de5c40d3d8676f485cae8855af2f6c3b6 Mon Sep 17 00:00:00 2001 From: Terry Chow <32403408+tkyc@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:26:30 -0800 Subject: [PATCH] Re-added support for stored procedure 'exec' escape syntax in CallableStatements (#2325) * EXEC system stored procedure regression fix * Additional test * Additional test * Indenting * Switched error string to TestResource error string * CR comments * Test update p1 * Test update p2 * CR comment changes; Test update * call escape syntax check * CR changes * formatting --- .../sqlserver/jdbc/SQLServerBulkCopy.java | 11 ++- .../jdbc/SQLServerPreparedStatement.java | 37 +++++++- .../CallableStatementTest.java | 95 +++++++++++++++++-- .../unit/statement/BatchExecutionTest.java | 11 ++- 4 files changed, 137 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java index c2f51aef5..e5fa8825e 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java @@ -2064,7 +2064,8 @@ private void writeNullToTdsWriter(TDSWriter tdsWriter, int srcJdbcType, private void writeColumnToTdsWriter(TDSWriter tdsWriter, int bulkPrecision, int bulkScale, int bulkJdbcType, boolean bulkNullable, // should it be destNullable instead? - int srcColOrdinal, int destColOrdinal, boolean isStreaming, Object colValue, Calendar cal) throws SQLServerException { + int srcColOrdinal, int destColOrdinal, boolean isStreaming, Object colValue, + Calendar cal) throws SQLServerException { SSType destSSType = destColumnMetadata.get(destColOrdinal).ssType; bulkPrecision = validateSourcePrecision(bulkPrecision, bulkJdbcType, @@ -2987,8 +2988,8 @@ private Object readColumnFromResultSet(int srcColOrdinal, int srcJdbcType, boole /** * Reads the given column from the result set current row and writes the data to tdsWriter. */ - private void writeColumn(TDSWriter tdsWriter, int srcColOrdinal, int destColOrdinal, - Object colValue, Calendar cal) throws SQLServerException { + private void writeColumn(TDSWriter tdsWriter, int srcColOrdinal, int destColOrdinal, Object colValue, + Calendar cal) throws SQLServerException { String destName = destColumnMetadata.get(destColOrdinal).columnName; int srcPrecision, srcScale, destPrecision, srcJdbcType; SSType destSSType = null; @@ -3640,8 +3641,8 @@ private boolean writeBatchData(TDSWriter tdsWriter, TDSCommand command, // Loop for each destination column. The mappings is a many to one mapping // where multiple source columns can be mapped to one destination column. for (ColumnMapping columnMapping : columnMappings) { - writeColumn(tdsWriter, columnMapping.sourceColumnOrdinal, columnMapping.destinationColumnOrdinal, null, - null // cell + writeColumn(tdsWriter, columnMapping.sourceColumnOrdinal, columnMapping.destinationColumnOrdinal, + null, null // cell // value is // retrieved // inside diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index d5b9b5499..7789b3726 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -29,6 +29,7 @@ import java.util.Map.Entry; import java.util.Vector; import java.util.logging.Level; +import java.util.regex.Pattern; import com.microsoft.sqlserver.jdbc.SQLServerConnection.CityHash128Key; import com.microsoft.sqlserver.jdbc.SQLServerConnection.PreparedStatementHandle; @@ -70,6 +71,10 @@ public class SQLServerPreparedStatement extends SQLServerStatement implements IS /** Processed SQL statement text, may not be same as what user initially passed. */ final String userSQL; + private boolean isExecEscapeSyntax; + + private boolean isCallEscapeSyntax; + /** Parameter positions in processed SQL statement text. */ final int[] userSQLParamPositions; @@ -128,6 +133,17 @@ private void setPreparedStatementHandle(int handle) { */ private boolean useBulkCopyForBatchInsert; + /** + * Regex for JDBC 'call' escape syntax + */ + private static final Pattern callEscapePattern = Pattern + .compile("^\\s*(?i)\\{(\\s*\\??\\s*=?\\s*)call (.+)\\s*\\(?\\?*,?\\)?\\s*}\\s*$"); + + /** + * Regex for 'exec' escape syntax + */ + private static final Pattern execEscapePattern = Pattern.compile("^\\s*(?i)(?:exec|execute)\\b"); + /** Returns the prepared statement SQL */ @Override public String toString() { @@ -253,6 +269,8 @@ private boolean resetPrepStmtHandle(boolean discardCurrentCacheItem) { procedureName = parsedSQL.procedureName; bReturnValueSyntax = parsedSQL.bReturnValueSyntax; userSQL = parsedSQL.processedSQL; + isExecEscapeSyntax = isExecEscapeSyntax(sql); + isCallEscapeSyntax = isCallEscapeSyntax(sql); userSQLParamPositions = parsedSQL.parameterPositions; initParams(userSQLParamPositions.length); useBulkCopyForBatchInsert = conn.getUseBulkCopyForBatchInsert(); @@ -1210,7 +1228,16 @@ else if (needsPrepare && !connection.getEnablePrepareOnFirstPreparedStatementCal */ boolean callRPCDirectly(Parameter[] params) throws SQLServerException { int paramCount = SQLServerConnection.countParams(userSQL); - return (null != procedureName && paramCount != 0 && !isTVPType(params)); + + // In order to execute sprocs directly the following must be true: + // 1. There must be a sproc name + // 2. There must be parameters + // 3. Parameters must not be a TVP type + // 4. Compliant CALL escape syntax + // If isExecEscapeSyntax is true, EXEC escape syntax is used then use prior behaviour to + // execute the procedure + return (null != procedureName && paramCount != 0 && !isTVPType(params) && isCallEscapeSyntax + && !isExecEscapeSyntax); } /** @@ -1230,6 +1257,14 @@ private boolean isTVPType(Parameter[] params) throws SQLServerException { return false; } + private boolean isExecEscapeSyntax(String sql) { + return execEscapePattern.matcher(sql).find(); + } + + private boolean isCallEscapeSyntax(String sql) { + return callEscapePattern.matcher(sql).find(); + } + /** * Executes sp_prepare to prepare a parameterized statement and sets the prepared statement handle * diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/callablestatement/CallableStatementTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/callablestatement/CallableStatementTest.java index 3a480548d..6570abf72 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/callablestatement/CallableStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/callablestatement/CallableStatementTest.java @@ -81,7 +81,7 @@ public class CallableStatementTest extends AbstractTest { /** * Setup before test - * + * * @throws SQLException */ @BeforeAll @@ -201,7 +201,7 @@ public void testCallableStatementSpPrepare() throws SQLException { /** * Tests CallableStatement.getString() with uniqueidentifier parameter - * + * * @throws SQLException */ @Test @@ -226,7 +226,7 @@ public void getStringGUIDTest() throws SQLException { /** * test for setNull(index, varchar) to behave as setNull(index, nvarchar) when SendStringParametersAsUnicode is true - * + * * @throws SQLException */ @Test @@ -302,7 +302,7 @@ public void testGetObjectAsLocalDateTime() throws SQLException { /** * Tests getObject(n, java.time.OffsetDateTime.class) and getObject(n, java.time.OffsetTime.class). - * + * * @throws SQLException */ @Test @@ -332,7 +332,7 @@ public void testGetObjectAsOffsetDateTime() throws SQLException { /** * recognize parameter names with and without leading '@' - * + * * @throws SQLException */ @Test @@ -1067,9 +1067,92 @@ public void testRegisteringOutputByIndexandAcquiringOutputParamByName() throws S } } + @Test + public void testExecuteSystemStoredProcedureNamedParametersAndIndexedParameterNoResultset() throws SQLException { + String call0 = "EXEC sp_getapplock @Resource=?, @LockTimeout='0', @LockMode='Exclusive', @LockOwner='Session'"; + String call1 = "\rEXEC\r\rsp_getapplock @Resource=?, @LockTimeout='0', @LockMode='Exclusive', @LockOwner='Session'"; + String call2 = " EXEC sp_getapplock @Resource=?, @LockTimeout='0', @LockMode='Exclusive', @LockOwner='Session'"; + String call3 = "\tEXEC\t\t\tsp_getapplock @Resource=?, @LockTimeout='0', @LockMode='Exclusive', @LockOwner='Session'"; + + try (CallableStatement cstmt0 = connection.prepareCall(call0); + CallableStatement cstmt1 = connection.prepareCall(call1); + CallableStatement cstmt2 = connection.prepareCall(call2); + CallableStatement cstmt3 = connection.prepareCall(call3);) { + cstmt0.setString(1, "Resource-" + UUID.randomUUID()); + cstmt0.execute(); + + cstmt1.setString(1, "Resource-" + UUID.randomUUID()); + cstmt1.execute(); + + cstmt2.setString(1, "Resource-" + UUID.randomUUID()); + cstmt2.execute(); + + cstmt3.setString(1, "Resource-" + UUID.randomUUID()); + cstmt3.execute(); + } + } + + @Test + public void testExecSystemStoredProcedureNamedParametersAndIndexedParameterResultSet() throws SQLException { + String call = "exec sp_sproc_columns_100 ?, @ODBCVer=3, @fUsePattern=0"; + + try (CallableStatement cstmt = connection.prepareCall(call)) { + cstmt.setString(1, "sp_getapplock"); + + try (ResultSet rs = cstmt.executeQuery()) { + while (rs.next()) { + assertTrue(TestResource.getResource("R_resultSetEmpty"), !rs.getString(4).isEmpty()); + } + } + } + } + + @Test + public void testExecSystemStoredProcedureNoIndexedParametersResultSet() throws SQLException { + String call = "execute sp_sproc_columns_100 sp_getapplock, @ODBCVer=3, @fUsePattern=0"; + + try (CallableStatement cstmt = connection.prepareCall(call); ResultSet rs = cstmt.executeQuery()) { + while (rs.next()) { + assertTrue(TestResource.getResource("R_resultSetEmpty"), !rs.getString(4).isEmpty()); + } + } + } + + @Test + public void testExecDocumentedSystemStoredProceduresIndexedParameters() throws SQLException { + String serverName; + String testTableName = "testTable"; + Integer integer = new Integer(1); + + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery("SELECT @@SERVERNAME")) { + rs.next(); + serverName = rs.getString(1); + } + + String[] sprocs = {"EXEC sp_column_privileges ?", "exec sp_catalogs ?", "execute sp_column_privileges ?", + "EXEC sp_column_privileges_ex ?", "EXECUTE sp_columns ?", "execute sp_datatype_info ?", + "EXEC sp_sproc_columns ?", "EXECUTE sp_server_info ?", "exec sp_special_columns ?", + "execute sp_statistics ?", "EXEC sp_table_privileges ?", "exec sp_tables ?"}; + + Object[] params = {testTableName, serverName, testTableName, serverName, testTableName, integer, + "sp_column_privileges", integer, testTableName, testTableName, testTableName, testTableName}; + + int paramIndex = 0; + + for (String sproc : sprocs) { + try (CallableStatement cstmt = connection.prepareCall(sproc)) { + cstmt.setObject(1, params[paramIndex]); + cstmt.execute(); + paramIndex++; + } catch (Exception e) { + fail("Failed executing '" + sproc + "' with indexed parameter '" + params[paramIndex]); + } + } + } + /** * Cleanup after test - * + * * @throws SQLException */ @AfterAll diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java index 3384f278c..0dace62b2 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java @@ -170,7 +170,7 @@ public void testValidTimezoneForTimestampBatchInsertWithBulkCopy() throws Except public void testValidTimezonesDstTimestampBatchInsertWithBulkCopy() throws Exception { Calendar gmtCal = Calendar.getInstance(TimeZone.getTimeZone("GMT")); - for (String tzId: TimeZone.getAvailableIDs()) { + for (String tzId : TimeZone.getAvailableIDs()) { TimeZone.setDefault(TimeZone.getTimeZone(tzId)); long ms = 1696127400000L; // DST @@ -191,8 +191,8 @@ public void testValidTimezonesDstTimestampBatchInsertWithBulkCopy() throws Excep } // Insert Timestamp using bulkcopy for batch insert - try (Connection con = DriverManager.getConnection( - connectionString + ";useBulkCopyForBatchInsert=true;sendTemporalDataTypesAsStringForBulkCopy=false;"); + try (Connection con = DriverManager.getConnection(connectionString + + ";useBulkCopyForBatchInsert=true;sendTemporalDataTypesAsStringForBulkCopy=false;"); PreparedStatement pstmt = con.prepareStatement("INSERT INTO " + timestampTable1 + " VALUES(?)")) { Timestamp timestamp = new Timestamp(ms); @@ -235,8 +235,9 @@ public void testBatchInsertTimestampNoTimezoneDoubleConversion() throws Exceptio long ms = 1578743412000L; // Insert Timestamp using prepared statement when useBulkCopyForBatchInsert=true - try (Connection con = DriverManager.getConnection(connectionString - + ";useBulkCopyForBatchInsert=true;sendTemporalDataTypesAsStringForBulkCopy=false;"); Statement stmt = con.createStatement(); + try (Connection con = DriverManager.getConnection( + connectionString + ";useBulkCopyForBatchInsert=true;sendTemporalDataTypesAsStringForBulkCopy=false;"); + Statement stmt = con.createStatement(); PreparedStatement pstmt = con.prepareStatement("INSERT INTO " + timestampTable2 + " VALUES(?)")) { TestUtils.dropTableIfExists(timestampTable2, stmt);