Bug #75441 com.mysql.jdbc.CallableStatement.extractProcedureName fragile
Submitted: 8 Jan 2015 0:03 Modified: 12 Jan 2015 13:02
Reporter: Brendan O'Shea Email Updates:
Status: Verified Impact on me:
None 
Category:Connector / J Severity:S3 (Non-critical)
Version:5.1.33 OS:Any
Assigned to: Alexander Soklakov CPU Architecture:Any

[8 Jan 2015 0:03] Brendan O'Shea
Description:
lines 946-952 of com.mysql.jdbc.CallableStatement.extractProcedureName does: 

        int endCallIndex = StringUtils.indexOfIgnoreCase(sanitizedSql, "CALL ");
        int offset = 5;

        if (endCallIndex == -1) {
            endCallIndex = StringUtils.indexOfIgnoreCase(sanitizedSql, "SELECT ");
            offset = 7;
        }

Note the trailing space in 'CALL ' and 'SELECT '. Other legal white space (<CR>,<TAB>, etc.) cause the 'indexOf' to return -1 and the method to throw an uninformative exception. 

Stack trace in my case: 
Caused by: java.sql.SQLException: Unable to retrieve metadata for procedure.
	at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:996) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:935) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:924) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.SQLError.createSQLException(SQLError.java:870) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.CallableStatement.extractProcedureName(CallableStatement.java:974) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.CallableStatement.determineParameterTypes(CallableStatement.java:780) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.CallableStatement.<init>(CallableStatement.java:598) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.JDBC4CallableStatement.<init>(JDBC4CallableStatement.java:42) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:1.7.0_45]
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57) ~[?:1.7.0_45]
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:1.7.0_45]
	at java.lang.reflect.Constructor.newInstance(Constructor.java:526) ~[?:1.7.0_45]
	at com.mysql.jdbc.Util.handleNewInstance(Util.java:377) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.CallableStatement.getInstance(CallableStatement.java:500) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.ConnectionImpl.parseCallableStatement(ConnectionImpl.java:3933) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.ConnectionImpl.prepareCall(ConnectionImpl.java:4000) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at com.mysql.jdbc.ConnectionImpl.prepareCall(ConnectionImpl.java:3976) ~[mysql-connector-java-5.1.33.jar:5.1.33]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.7.0_45]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) ~[?:1.7.0_45]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.7.0_45]
	at java.lang.reflect.Method.invoke(Method.java:606) ~[?:1.7.0_45]
	at org.apache.ibatis.datasource.pooled.PooledConnection.invoke(PooledConnection.java:246) ~[mybatis-3.2.8.jar:3.2.8]
	at com.sun.proxy.$Proxy43.prepareCall(Unknown Source) ~[?:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.7.0_45]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) ~[?:1.7.0_45]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.7.0_45]
	at java.lang.reflect.Method.invoke(Method.java:606) ~[?:1.7.0_45]
	at org.apache.ibatis.logging.jdbc.ConnectionLogger.invoke(ConnectionLogger.java:61) ~[mybatis-3.2.8.jar:3.2.8]
	at com.sun.proxy.$Proxy43.prepareCall(Unknown Source) ~[?:?]

How to repeat:
Simplest would be to have a unit test that created a CallableStatement object with this.originalSql set to
CALL
		delete_entity(?, ?, ?, ?, ?);

('hexdump -C' equivalent:
00000000  43 41 4c 4c 0a 09 09 64  65 6c 65 74 65 5f 65 6e  |CALL...delete_en|
00000010  74 69 74 79 28 3f 2c 20  3f 2c 20 3f 2c 20 3f 2c  |tity(?, ?, ?, ?,|
00000020  20 3f 29 3b                                       | ?);|

)

Then call 'extractProcedureName'; Expected : 'delete_entity'; Actual : java.sql.SQLException: Unable to retrieve metadata for procedure.

Suggested fix:
Change 'StringUtils.stripComments' to do more sanitization on the input (replacing all whitespace characters (carriage return, new line, tab, etc.) with a single space). 

This would also fix DatabaseMetadata.getCallStmtParameterTypes which from looking at the code appears to make the same assumptions on line 1723 & 1730. 

Other users of StringUtils.stripComments don't appear to depend on spaces. 

If this is done, the name of the method should change to reflect its expanded scope.
[12 Jan 2015 13:02] Alexander Soklakov
Hi Brendan,

Thank you for the report. Verified by code review.