Uploaded image for project: 'MariaDB Connector/J'
  1. MariaDB Connector/J
  2. CONJ-142

Using a semicolon in a string with "rewriteBatchedStatements=true" fails

Details

    • Bug
    • Status: Closed (View Workflow)
    • Minor
    • Resolution: Fixed
    • 1.1.8
    • 1.1.9
    • Other
    • None
    • Windows 7, 64 bit

    Description

      I can reproduce the error I am submitting here with this code block:

       
          public static void main(String[] args) throws SQLException {
              MariaDB db = new MariaDB();
              Connection conn = db.getConnection();
              PreparedStatement sqlInsert = conn.prepareStatement("insert into prep (text) values (?)");
              sqlInsert.setString(1, "aa");
              sqlInsert.addBatch();
              sqlInsert.setString(1, "b;b");
              sqlInsert.addBatch();
              sqlInsert.executeBatch();
              conn.commit();
          }
       

      The connection uses "rewriteBatchedStatements=true"
      As soon as I use a semicolon in the string, I get an "String index out of range: -1"

      1. if I switch back to connector 1.1.7, I get no error
      2. if I set rewriteBatchedStatements to false, I get no error
      3. the MariaDB query.log is empty, i.e. the statement does not reach the database. Which is to be expected since the error is an java.lang.StringIndexOutOfBoundsException
      4. Tested on MariaDB 10.0.15, connector 1.1.8

      If you need any more infos, let me know.

      Attachments

        Issue Links

          Activity

            satotatsu1973 tatsuo satoh added a comment -

            The following issues that fixes part has been resolved.

            org.mariadb.jdbc.MySQLPreparedStatement.java

            /**
             * <p>
             * CONJ-142 bugs
             * </p>
             * Using a semicolon in a string with "rewriteBatchedStatements=true" fails
             */
            @Override
            protected int getInsertIncipit(String sql) {
            	String sqlUpper = sql.toUpperCase();
            	if (!sqlUpper.startsWith("INSERT")) {
            		return -1;
            	}
            	int idx = sqlUpper.indexOf("VALUE");
            	int startIndex = sqlUpper.lastIndexOf(")", idx);
            	String mid = sqlUpper.substring(startIndex + 1, idx);
            	if (mid.trim().length() != 0) {
            		return -1;
            	}
            	int index = sqlUpper.indexOf("(", idx + 5);
            	mid = sqlUpper.substring(idx + 6, index);
            	if (mid.trim().length() != 0) {
            		return -1;
            	}
            	return index;
            }
             
             
            /**
             * If the batch array contains only rewriteable sql strings, returns the
             * rewritten statement.
             * 
             * @return the rewritten statement
             */
            private String rewrittenBatch() {
            	StringBuilder result = null;
            	if (isRewriteable) {
            		result = new StringBuilder("");
            		result.append(firstRewrite);
            		for (MySQLPreparedStatement mySQLPS : batchPreparedStatements) {
            			String query = mySQLPS.dQuery.toSQLForBatchInsert(firstRewrite);
            			result.append(query);
            			result.append(",");
            		}
            		result.deleteCharAt(result.length() - 1);
            	}
            	return (result == null ? null : result.toString());
            }

            org.mariadb.jdbc.internal.common.query.MySQLParameterizedQuery.java

            /**
             * <p>
             * CONJ-142 bug
             * </p>
             * Using a semicolon in a string with "rewriteBatchedStatements=true" fails
             */
            public String toSQLForBatchInsert(String firstRewrite) {
            	try {
            		final ByteArrayOutputStream os = new ByteArrayOutputStream();
            		writeTo(os, firstRewrite.getBytes("UTF-8").length);
            		String sql = new String(os.toByteArray(), Charset.forName("UTF8"));
            		return sql;
            	} catch (QueryException qe) {
            		return "";
            	} catch (IOException e) {
            		return "";
            	}
            }
             
            /**
             * <p>
             * CONJ-142 bug
             * </p>
             * Using a semicolon in a string with "rewriteBatchedStatements=true" fails
             */
            public void writeTo(final OutputStream os, int index) throws IOException,
            		QueryException {
             
            	if (queryPartsArray.length == 0) {
            		throw new AssertionError("Invalid query, queryParts was empty");
            	}
            	if (queryPartsArray[0].length > index) {
            		os.write(queryPartsArray[0], index, queryPartsArray[0].length
            				- index);
            	} else {
            		os.write(queryPartsArray[0]);
            	}
            	for (int i = 1; i < queryPartsArray.length; i++) {
            		parameters[i - 1].writeTo(os);
            		if (queryPartsArray[i].length != 0)
            			os.write(queryPartsArray[i]);
            	}
            }

            satotatsu1973 tatsuo satoh added a comment - The following issues that fixes part has been resolved. org.mariadb.jdbc.MySQLPreparedStatement.java /** * <p> * CONJ-142 bugs * </p> * Using a semicolon in a string with "rewriteBatchedStatements=true" fails */ @Override protected int getInsertIncipit(String sql) { String sqlUpper = sql.toUpperCase(); if (!sqlUpper.startsWith( "INSERT" )) { return - 1 ; } int idx = sqlUpper.indexOf( "VALUE" ); int startIndex = sqlUpper.lastIndexOf( ")" , idx); String mid = sqlUpper.substring(startIndex + 1 , idx); if (mid.trim().length() != 0 ) { return - 1 ; } int index = sqlUpper.indexOf( "(" , idx + 5 ); mid = sqlUpper.substring(idx + 6 , index); if (mid.trim().length() != 0 ) { return - 1 ; } return index; }     /** * If the batch array contains only rewriteable sql strings, returns the * rewritten statement. * * @return the rewritten statement */ private String rewrittenBatch() { StringBuilder result = null ; if (isRewriteable) { result = new StringBuilder( "" ); result.append(firstRewrite); for (MySQLPreparedStatement mySQLPS : batchPreparedStatements) { String query = mySQLPS.dQuery.toSQLForBatchInsert(firstRewrite); result.append(query); result.append( "," ); } result.deleteCharAt(result.length() - 1 ); } return (result == null ? null : result.toString()); } org.mariadb.jdbc.internal.common.query.MySQLParameterizedQuery.java /** * <p> * CONJ-142 bug * </p> * Using a semicolon in a string with "rewriteBatchedStatements=true" fails */ public String toSQLForBatchInsert(String firstRewrite) { try { final ByteArrayOutputStream os = new ByteArrayOutputStream(); writeTo(os, firstRewrite.getBytes( "UTF-8" ).length); String sql = new String(os.toByteArray(), Charset.forName( "UTF8" )); return sql; } catch (QueryException qe) { return "" ; } catch (IOException e) { return "" ; } }   /** * <p> * CONJ-142 bug * </p> * Using a semicolon in a string with "rewriteBatchedStatements=true" fails */ public void writeTo( final OutputStream os, int index) throws IOException, QueryException {   if (queryPartsArray.length == 0 ) { throw new AssertionError( "Invalid query, queryParts was empty" ); } if (queryPartsArray[ 0 ].length > index) { os.write(queryPartsArray[ 0 ], index, queryPartsArray[ 0 ].length - index); } else { os.write(queryPartsArray[ 0 ]); } for ( int i = 1 ; i < queryPartsArray.length; i++) { parameters[i - 1 ].writeTo(os); if (queryPartsArray[i].length != 0 ) os.write(queryPartsArray[i]); } }

            satotatsu1973 thanks for patches. I was wondering why you removed the semicolon check completely in getInsertIncipit.

            I looked at it and would suggest:

                /**
                 * Parses the input string to understand if it is an INSERT statement.
                 * Returns the position of the round bracket after the VALUE(S) SQL keyword,
                 * or -1 if it cannot understand it is an INSERT statement.
                 * Multiple statements cannot be parsed.
                 * @param sql the input SQL statement
                 * @return the position of the round bracket after the VALUE(S) SQL keyword,
                 * or -1 if it cannot be parsed as an INSERT statement
                 */
                protected int getInsertIncipit(String sql) {
                	String sqlUpper = sql.toUpperCase();
                	
                	if (! sqlUpper.startsWith("INSERT"))
                		return -1;
                	
                	int idx = sqlUpper.indexOf(" VALUE");
                	int startBracket = sqlUpper.indexOf("(", idx);
                	int endBracket = sqlUpper.indexOf(")", startBracket);
                	
                	// Check for semicolons. Allow them inside the VALUES() brackets, otherwise return -1
                	// there can be multiple, so let's loop through them
                	
                	int semicolonPos = sqlUpper.indexOf(';');
                	
                	while (semicolonPos > -1)
                	{
                		if (semicolonPos < startBracket || semicolonPos > endBracket)
                			return -1;
                		
                		semicolonPos = sqlUpper.indexOf(';', semicolonPos + 1);
                	}
                	
                	return startBracket;
                }

            Let me know the reasoning behind your patch and I'm happy to revert to that if it makes sense.

            ratzpo Rasmus Johansson (Inactive) added a comment - satotatsu1973 thanks for patches. I was wondering why you removed the semicolon check completely in getInsertIncipit. I looked at it and would suggest: /** * Parses the input string to understand if it is an INSERT statement. * Returns the position of the round bracket after the VALUE(S) SQL keyword, * or -1 if it cannot understand it is an INSERT statement. * Multiple statements cannot be parsed. * @param sql the input SQL statement * @return the position of the round bracket after the VALUE(S) SQL keyword, * or -1 if it cannot be parsed as an INSERT statement */ protected int getInsertIncipit(String sql) { String sqlUpper = sql.toUpperCase(); if (! sqlUpper.startsWith( "INSERT" )) return - 1 ; int idx = sqlUpper.indexOf( " VALUE" ); int startBracket = sqlUpper.indexOf( "(" , idx); int endBracket = sqlUpper.indexOf( ")" , startBracket); // Check for semicolons. Allow them inside the VALUES() brackets, otherwise return -1 // there can be multiple, so let's loop through them int semicolonPos = sqlUpper.indexOf( ';' ); while (semicolonPos > - 1 ) { if (semicolonPos < startBracket || semicolonPos > endBracket) return - 1 ; semicolonPos = sqlUpper.indexOf( ';' , semicolonPos + 1 ); } return startBracket; } Let me know the reasoning behind your patch and I'm happy to revert to that if it makes sense.
            satotatsu1973 tatsuo satoh added a comment - - edited

            Thank you for reply.

            I was except for the semicolon check going to try to support the following SQL, It is better to leave as it is considering the multiple SQL.

            SQL

            insert into table aaa (val1, val2) values (';', ?)

            /**
             * <p>
             * CONJ-142 bugs
             * </p>
             * Using a semicolon in a string with "rewriteBatchedStatements=true" fails
             */
            @Override
            protected int getInsertIncipit(String sql) {
            	String sqlUpper = sql.toUpperCase();
            	if ((!(sqlUpper.startsWith("INSERT"))) || (sqlUpper.indexOf(";") != -1)) {
            		return -1;
            	}
            	int idx = sqlUpper.indexOf("VALUE");
            	int startIndex = sqlUpper.lastIndexOf(")", idx);
            	String mid = sqlUpper.substring(startIndex + 1, idx);
            	if (mid.trim().length() != 0) {
            		return -1;
            	}
            	int index = sqlUpper.indexOf("(", idx + 5);
            	mid = sqlUpper.substring(idx + 6, index);
            	if (mid.trim().length() != 0) {
            		return -1;
            	}
            	return index;
            }

            satotatsu1973 tatsuo satoh added a comment - - edited Thank you for reply. I was except for the semicolon check going to try to support the following SQL, It is better to leave as it is considering the multiple SQL. SQL insert into table aaa (val1, val2) values ( ';' , ?) /** * <p> * CONJ-142 bugs * </p> * Using a semicolon in a string with "rewriteBatchedStatements=true" fails */ @Override protected int getInsertIncipit(String sql) { String sqlUpper = sql.toUpperCase(); if ((!(sqlUpper.startsWith( "INSERT" ))) || (sqlUpper.indexOf( ";" ) != - 1 )) { return - 1 ; } int idx = sqlUpper.indexOf( "VALUE" ); int startIndex = sqlUpper.lastIndexOf( ")" , idx); String mid = sqlUpper.substring(startIndex + 1 , idx); if (mid.trim().length() != 0 ) { return - 1 ; } int index = sqlUpper.indexOf( "(" , idx + 5 ); mid = sqlUpper.substring(idx + 6 , index); if (mid.trim().length() != 0 ) { return - 1 ; } return index; }

            We have also run into this bug. Any idea about when a new release with a fix will be available in Maven?

            tbfangel Thomas Bøgh Fangel added a comment - We have also run into this bug. Any idea about when a new release with a fix will be available in Maven?

            People

              ratzpo Rasmus Johansson (Inactive)
              Sektat Marcel Schneider
              Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.