Fix for Bug#77183 (21181501), INSERT..VALUE..lead to invalidation of batch insert.

Author: fjssilva

CHANGES

@@ -3,6 +3,8 @@
 
 Version 5.1.49
 
+  - Fix for Bug#77183 (21181501), INSERT..VALUE..lead to invalidation of batch insert.
+
   - Fix for Bug#23143279, CLIENT HANG WHEN LOADBALANCESTRATEGY IS BESTRESPONSETIME.
 
   - WL#14007, Remove third-party libraries from sources and bundles.

src/com/mysql/jdbc/LocalizedErrorMessages.properties

@@ -461,7 +461,7 @@ ConnectionProperties.replicationConnectionGroup=Logical group of replication con
 ConnectionProperties.allowMasterDownConnections=By default, a replication-aware connection will fail to connect when configured master hosts are all unavailable at initial connection. Setting this property to 'true' allows to establish the initial connection, by failing over to the slave servers, in read-only state. It won't prevent subsequent failures when switching back to the master hosts i.e. by setting the replication connection to read/write state.
 ConnectionProperties.allowSlaveDownConnections=By default, a replication-aware connection will fail to connect when configured slave hosts are all unavailable at initial connection. Setting this property to 'true' allows to establish the initial connection. It won't prevent failures when switching to slaves i.e. by setting the replication connection to read-only state. The property 'readFromMasterWhenNoSlaves' should be used for this purpose. 
 ConnectionProperties.readFromMasterWhenNoSlaves=Replication-aware connections distribute load by using the master hosts when in read/write state and by using the slave hosts when in read-only state. If, when setting the connection to read-only state, none of the slave hosts are available, an SQLExeception is thrown back. Setting this property to 'true' allows to fail over to the master hosts, while setting the connection state to read-only, when no slave hosts are available at switch instant.
-ConnectionProperties.allowMultiQueries=Allow the use of ';' to delimit multiple queries during one statement (true/false), defaults to 'false', and does not affect the addBatch() and executeBatch() methods, which instead rely on rewriteBatchStatements.
+ConnectionProperties.allowMultiQueries=Allow the use of ';' to delimit multiple queries during one statement (true/false), defaults to 'false', and does not affect the addBatch() and executeBatch() methods, which instead rely on rewriteBatchedStatements.
 ConnectionProperties.allowNANandINF=Should the driver allow NaN or +/- INF values in PreparedStatement.setDouble()?
 ConnectionProperties.allowUrlInLoadLocal=Should the driver allow URLs in 'LOAD DATA LOCAL INFILE' statements?
 ConnectionProperties.alwaysSendSetIsolation=Should the driver always communicate with the database when Connection.setTransactionIsolation() is called? If set to false, the driver will only communicate with the database when the requested transaction isolation is different than the whichever is newer, the last value that was set via Connection.setTransactionIsolation(), or the value that was read from the server when the connection was established.  Note that useLocalSessionState=true will force the same behavior as alwaysSendSetIsolation=false, regardless of how alwaysSendSetIsolation is set.
@@ -569,7 +569,7 @@ ConnectionProperties.resourceId=A globally unique name that identifies the resou
 ConnectionProperties.resultSetSizeThreshold=If 'useUsageAdvisor' is true, how many rows should a result set contain before the driver warns that it is suspiciously large?
 ConnectionProperties.retainStatementAfterResultSetClose=Should the driver retain the Statement reference in a ResultSet after ResultSet.close() has been called. This is not JDBC-compliant after JDBC-4.0.
 ConnectionProperties.retriesAllDown=When using loadbalancing or failover, the number of times the driver should cycle through available hosts, attempting to connect.  Between cycles, the driver will pause for 250ms if no servers are available.
-ConnectionProperties.rewriteBatchedStatements=Should the driver use multiqueries (irregardless of the setting of "allowMultiQueries") as well as rewriting of prepared statements for INSERT into multi-value inserts when executeBatch() is called? Notice that this has the potential for SQL injection if using plain java.sql.Statements and your code doesn't sanitize input correctly. Notice that for prepared statements, server-side prepared statements can not currently take advantage of this rewrite option, and that if you don't specify stream lengths when using PreparedStatement.set*Stream(), the driver won't be able to determine the optimum number of parameters per batch and you might receive an error from the driver that the resultant packet is too large. Statement.getGeneratedKeys() for these rewritten statements only works when the entire batch includes INSERT statements. Please be aware using rewriteBatchedStatements=true with INSERT .. ON DUPLICATE KEY UPDATE that for rewritten statement server returns only one value as sum of all affected (or found) rows in batch and it isn't possible to map it correctly to initial statements; in this case driver returns 0 as a result of each batch statement if total count was 0, and the Statement.SUCCESS_NO_INFO as a result of each batch statement if total count was > 0.
+ConnectionProperties.rewriteBatchedStatements=Should the driver use multiqueries (irregardless of the setting of "allowMultiQueries") as well as rewriting of prepared statements for INSERT into multi-value inserts when executeBatch() is called? Notice that this has the potential for SQL injection if using plain java.sql.Statements and your code doesn't sanitize input correctly. Notice that for prepared statements, if you don't specify stream lengths when using PreparedStatement.set*Stream(), the driver won't be able to determine the optimum number of parameters per batch and you might receive an error from the driver that the resultant packet is too large. Statement.getGeneratedKeys() for these rewritten statements only works when the entire batch includes INSERT statements. Please be aware using rewriteBatchedStatements=true with INSERT .. ON DUPLICATE KEY UPDATE that for rewritten statement server returns only one value as sum of all affected (or found) rows in batch and it isn't possible to map it correctly to initial statements; in this case driver returns 0 as a result of each batch statement if total count was 0, and the Statement.SUCCESS_NO_INFO as a result of each batch statement if total count was > 0.
 ConnectionProperties.rollbackOnPooledClose=Should the driver issue a rollback() when the logical connection in a pool is closed?
 ConnectionProperties.roundRobinLoadBalance=When autoReconnect is enabled, and failoverReadonly is false, should we pick hosts to connect to on a round-robin basis?
 ConnectionProperties.runningCTS13=Enables workarounds for bugs in Sun's JDBC compliance testsuite version 1.3

src/com/mysql/jdbc/PreparedStatement.java

@@ -409,22 +409,23 @@ private String extractValuesClause(String sql, String quoteCharStr) throws SQLEx
             int indexOfValues = -1;
             int valuesSearchStart = this.statementStartPos;
 
+            // VALUE is a synonym for VALUES
             while (indexOfValues == -1) {
                 if (quoteCharStr.length() > 0) {
-                    indexOfValues = StringUtils.indexOfIgnoreCase(valuesSearchStart, sql, "VALUES", quoteCharStr, quoteCharStr,
+                    indexOfValues = StringUtils.indexOfIgnoreCase(valuesSearchStart, sql, "VALUE", quoteCharStr, quoteCharStr,
                             StringUtils.SEARCH_MODE__MRK_COM_WS);
                 } else {
-                    indexOfValues = StringUtils.indexOfIgnoreCase(valuesSearchStart, sql, "VALUES");
+                    indexOfValues = StringUtils.indexOfIgnoreCase(valuesSearchStart, sql, "VALUE");
                 }
 
                 if (indexOfValues > 0) {
-                    /* check if the char immediately preceding VALUES may be part of the table name */
+                    /* check if the char immediately preceding VALUE[S] may be part of the table name */
                     char c = sql.charAt(indexOfValues - 1);
                     if (!(Character.isWhitespace(c) || c == ')' || c == '`')) {
                         valuesSearchStart = indexOfValues + 6;
                         indexOfValues = -1;
                     } else {
-                        /* check if the char immediately following VALUES may be whitespace or open parenthesis */
+                        /* check if the char immediately following VALUE[S] may be whitespace or open parenthesis */
                         c = sql.charAt(indexOfValues + 6);
                         if (!(Character.isWhitespace(c) || c == '(')) {
                             valuesSearchStart = indexOfValues + 6;
@@ -440,7 +441,9 @@ private String extractValuesClause(String sql, String quoteCharStr) throws SQLEx
                 return null;
             }
 
-            int indexOfFirstParen = sql.indexOf('(', indexOfValues + 6);
+            // VALUE vs VALUES length
+            int valLength = sql.length() > indexOfValues + 5 && Character.toUpperCase(sql.charAt(indexOfValues + 5)) == 'S' ? 6 : 5;
+            int indexOfFirstParen = sql.indexOf('(', indexOfValues + valLength);
 
             if (indexOfFirstParen == -1) {
                 return null;

src/testsuite/regression/StatementRegressionTest.java

@@ -8917,6 +8917,68 @@ public Void call() throws Exception {
                 con.close();
             }
         } while (useSPS = !useSPS);
+    }
+
+    /**
+     * Test fix for Bug#77183 (21181501), INSERT..VALUE..lead to invalidation of batch insert.
+     * 
+     * @throws Exception
+     */
+    public void testBug77183() throws Exception {
+        createTable("testBug77183", "(c1 INT, c2 INT)");
+
+        boolean useSPS = false;
+        boolean rwBS = false;
+        boolean useRep = false;
+        boolean useValue = false;
+
+        do {
+            Properties props = new Properties();
+            props.setProperty("useServerPrepStmts", Boolean.toString(useSPS));
+            props.setProperty("rewriteBatchedStatements", Boolean.toString(rwBS));
+            props.setProperty("statementInterceptors", TestBug77183StatementInterceptor.class.getName());
+
+            Connection testConn = getConnectionWithProps(props);
+
+            this.pstmt = testConn.prepareStatement((useRep ? "REPLACE" : "INSERT") + " INTO testBug77183 " + (useValue ? "VALUE" : "VALUES") + " (?, ?)");
+            for (int i = 1; i <= 3; i++) {
+                this.pstmt.setInt(1, i);
+                this.pstmt.setInt(2, i);
+                this.pstmt.addBatch();
+            }
+            this.pstmt.executeBatch();
+
+            testConn.close();
+        } while ((useSPS = !useSPS) || (rwBS = !rwBS) || (useRep = !useRep) || (useValue = !useValue));
+
+        assertEquals(32, TestBug77183StatementInterceptor.countInterceptions);
+    }
+
+    public static class TestBug77183StatementInterceptor extends BaseStatementInterceptor {
+        public static int countInterceptions = 0;
+
+        @Override
+        public ResultSetInternalMethods preProcess(String sql, com.mysql.jdbc.Statement interceptedStatement, com.mysql.jdbc.Connection connection)
+                throws SQLException {
+            if (sql == null) {
+                sql = "";
+            }
+            if (sql.length() == 0 && interceptedStatement instanceof com.mysql.jdbc.PreparedStatement) {
+                sql = ((com.mysql.jdbc.PreparedStatement) interceptedStatement).asSql();
+            }
+            if (interceptedStatement instanceof PreparedStatement) {
+                countInterceptions++;
 
+                final boolean useSPS = connection.getUseServerPreparedStmts();
+                final boolean rwBS = connection.getRewriteBatchedStatements();
+                final String testCase = String.format("Case [SPS: %s, RwBS: %s, Query: %s]", useSPS ? "Y" : "N", rwBS ? "Y" : "N", sql);
+                final int numParamSets = sql.length() - sql.replace("(", "").length();
+                final int numPlaceholders = sql.length() - sql.replace("?", "").length();
+
+                assertEquals(testCase, rwBS ? 3 : 1, numParamSets);
+                assertEquals(testCase, useSPS ? rwBS ? 6 : 2 : 0, numPlaceholders);
+            }
+            return super.preProcess(sql, interceptedStatement, connection);
+        }
     }
 }