From 4f70ec3e4e115166ad5e1058cf1802995c722dcb Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 5 Sep 2023 00:13:58 +0100 Subject: [PATCH] Fix for Bug#91351 (Bug#28225464), MysqlConnectionPoolDataSource - autocommit status lost if global autocommit = 0. Change-Id: I7a25b35b4040d2a075c2aba5b47f078c6a33a73e --- CHANGES | 2 + .../java/com/mysql/cj/CharsetSettings.java | 4 +- .../com/mysql/cj/NativeCharsetSettings.java | 9 +++-- .../java/com/mysql/cj/NativeSession.java | 6 +++ .../cj/protocol/a/NativeMessageBuilder.java | 6 +++ .../com/mysql/cj/jdbc/ConnectionImpl.java | 10 +++-- .../regression/DataSourceRegressionTest.java | 40 ++++++++++++++++++- .../java/testsuite/simple/DataSourceTest.java | 11 ++--- 8 files changed, 70 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index 64fe4257f..6bfb4037b 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,8 @@ Version 8.2.0 + - Fix for Bug#91351 (Bug#28225464), MysqlConnectionPoolDataSource - autocommit status lost if global autocommit = 0. + - WL#15197, Support WebauthN in fido authentication plugin. - Fix for Bug#107215 (Bug#34139593), ClassCastException: java.time.LocalDateTime cannot be cast to java.sql.Timestamp. diff --git a/src/main/core-api/java/com/mysql/cj/CharsetSettings.java b/src/main/core-api/java/com/mysql/cj/CharsetSettings.java index 66c26b4fd..a497abb63 100644 --- a/src/main/core-api/java/com/mysql/cj/CharsetSettings.java +++ b/src/main/core-api/java/com/mysql/cj/CharsetSettings.java @@ -54,13 +54,13 @@ public interface CharsetSettings { * * otherwise it will be set to utf8mb4_general_ci or utf8mb4_0900_ai_ci depending on server version. *

- * Since Protocol::HandshakeV10 and Protocol::HandshakeResponse41 has only one byte for the collation it's not possible to use indexes > 255 during the + * Since Protocol::HandshakeV10 and Protocol::HandshakeResponse41 use only one byte for the collation it's not possible to use indexes > 255 during the * handshake. * Also, ucs2, utf16, utf16le and utf32 character sets are impermissible here. Connector/J will try to use utf8mb4 instead. *

* * @param reset - * reset the charsets configuration; needed for changeUser call. + * reset the charsets configuration; needed for changeUser and resetServerState call. * * @return MySQL collation index to be used during the handshake. */ diff --git a/src/main/core-impl/java/com/mysql/cj/NativeCharsetSettings.java b/src/main/core-impl/java/com/mysql/cj/NativeCharsetSettings.java index 2ee3fc268..c469ca8e0 100644 --- a/src/main/core-impl/java/com/mysql/cj/NativeCharsetSettings.java +++ b/src/main/core-impl/java/com/mysql/cj/NativeCharsetSettings.java @@ -218,10 +218,11 @@ public int configurePreHandshake(boolean reset) { && (encoding = this.characterEncoding.getValue()) == null) { // If none of "passwordCharacterEncoding", "connectionCollation" or "characterEncoding" is specified then use UTF-8. // It would be better to use the server default collation here, to avoid unnecessary SET NAMES queries after the handshake if server - // default charset if not utf8, but we can not do it until server Bug#32729185 is fixed. Server cuts collation index to lower byte and, for example, - // if the server is started with character-set-server=utf8mb4 and collation-server=utf8mb4_is_0900_ai_ci (collation index 257) the Protocol::HandshakeV10 - // will contain character_set=1, "big5_chinese_ci". This is true not only for MySQL 8.0, where built-in collations with indexes > 255 were first introduced, - // but also other server series would be affected when configured with custom collations, for which the reserved collation id range is >= 1024. + // default charset if not utf8, but we can not do it until server Bug#32729185 is fixed. Server cuts collation index to lower byte and, for + // example, if the server is started with character-set-server=utf8mb4 and collation-server=utf8mb4_is_0900_ai_ci (collation index 257) the + // Protocol::HandshakeV10 will contain character_set=1, "big5_chinese_ci". This is true not only for MySQL 8.0, where built-in collations with + // indexes > 255 were first introduced, but also other server series would be affected when configured with custom collations, for which the + // reserved collation id range is >= 1024. this.sessionCollationIndex = MYSQL_COLLATION_INDEX_utf8mb4_0900_ai_ci; } } diff --git a/src/main/core-impl/java/com/mysql/cj/NativeSession.java b/src/main/core-impl/java/com/mysql/cj/NativeSession.java index 271715932..03b86a1bd 100644 --- a/src/main/core-impl/java/com/mysql/cj/NativeSession.java +++ b/src/main/core-impl/java/com/mysql/cj/NativeSession.java @@ -827,4 +827,10 @@ public synchronized Timer getCancelTimer() { return this.cancelTimer; } + public void resetSessionState() { + checkClosed(); + NativePacketPayload message = this.commandBuilder.buildComResetConnection(((NativeProtocol) this.protocol).getSharedSendPacket()); + this.protocol.sendCommand(message, false, 0); + } + } diff --git a/src/main/protocol-impl/java/com/mysql/cj/protocol/a/NativeMessageBuilder.java b/src/main/protocol-impl/java/com/mysql/cj/protocol/a/NativeMessageBuilder.java index e0c4ecb53..3653b7cbc 100644 --- a/src/main/protocol-impl/java/com/mysql/cj/protocol/a/NativeMessageBuilder.java +++ b/src/main/protocol-impl/java/com/mysql/cj/protocol/a/NativeMessageBuilder.java @@ -360,4 +360,10 @@ public NativePacketPayload buildComStmtExecute(NativePacketPayload sharedPacket, return packet; } + public NativePacketPayload buildComResetConnection(NativePacketPayload sharedPacket) { + NativePacketPayload packet = sharedPacket != null ? sharedPacket : new NativePacketPayload(1); + packet.writeInteger(IntegerDataType.INT1, NativeConstants.COM_RESET_CONNECTION); + return packet; + } + } diff --git a/src/main/user-impl/java/com/mysql/cj/jdbc/ConnectionImpl.java b/src/main/user-impl/java/com/mysql/cj/jdbc/ConnectionImpl.java index 3fe7c0bf3..5b8219568 100644 --- a/src/main/user-impl/java/com/mysql/cj/jdbc/ConnectionImpl.java +++ b/src/main/user-impl/java/com/mysql/cj/jdbc/ConnectionImpl.java @@ -545,9 +545,8 @@ public void changeUser(String userName, String newPassword) throws SQLException this.password = newPassword; this.session.getServerSession().getCharsetSettings().configurePostHandshake(true); - this.session.setSessionVariables(); - + handleAutoCommitDefaults(); setupServerForTruncationChecks(); } } @@ -1749,7 +1748,12 @@ public void releaseSavepoint(Savepoint arg0) throws SQLException { @Override public void resetServerState() throws SQLException { if (!this.propertySet.getBooleanProperty(PropertyKey.paranoid).getValue() && this.session != null) { - changeUser(this.user, this.password); + this.session.getServerSession().getCharsetSettings().configurePreHandshake(true); + this.session.resetSessionState(); + this.session.getServerSession().getCharsetSettings().configurePostHandshake(true); + this.session.setSessionVariables(); + handleAutoCommitDefaults(); + setupServerForTruncationChecks(); } } diff --git a/src/test/java/testsuite/regression/DataSourceRegressionTest.java b/src/test/java/testsuite/regression/DataSourceRegressionTest.java index 29f21b913..ac4f31a18 100644 --- a/src/test/java/testsuite/regression/DataSourceRegressionTest.java +++ b/src/test/java/testsuite/regression/DataSourceRegressionTest.java @@ -577,7 +577,7 @@ public void testBug72632() throws Exception { * @throws Exception */ @Test - void testBug104954() throws Exception { + public void testBug104954() throws Exception { createDatabase("testBug104954"); createDatabase("`testBug104954?bug=104954`"); @@ -604,4 +604,42 @@ void testBug104954() throws Exception { } } + /** + * Tests fix for Bug#91351 (Bug#28225464), MysqlConnectionPoolDataSource - autocommit status lost if global autocommit = 0. + * + * @throws Exception + */ + @Test + public void testBug91351() throws Exception { + try { + createTable("testBug91351", "(txt VARCHAR(100))"); + this.stmt.executeUpdate("SET GLOBAL autocommit=0"); // Pre-condition: global autocommit=0. + + final String testDbUrl = dbUrl + (dbUrl.contains("?") ? "&" : "?"); + boolean isParanoid = false; + do { + MysqlConnectionPoolDataSource ds = new MysqlConnectionPoolDataSource(); + ds.setUrl(testDbUrl + "paranoid=" + isParanoid); + Connection testConn = ds.getPooledConnection().getConnection(); + testConn.createStatement().execute("SET SESSION wait_timeout=5"); // Otherwise test would hang when tearing down created artifacts. + + PreparedStatement testPstmt = null; + String query = "INSERT INTO testBug91351 VALUES (?)"; + testPstmt = testConn.prepareStatement(query); + testPstmt.setString(1, "MySQL Connector/J"); + testPstmt.executeUpdate(); + + testConn.close(); + + this.rs = this.stmt.executeQuery("SELECT * FROM testBug91351"); + assertTrue(this.rs.next()); + assertEquals("MySQL Connector/J", this.rs.getString(1)); + + testConn.close(); + } while (isParanoid = !isParanoid); + } finally { + this.stmt.executeUpdate("SET GLOBAL autocommit=1"); + } + } + } diff --git a/src/test/java/testsuite/simple/DataSourceTest.java b/src/test/java/testsuite/simple/DataSourceTest.java index 911db0734..eecce456d 100644 --- a/src/test/java/testsuite/simple/DataSourceTest.java +++ b/src/test/java/testsuite/simple/DataSourceTest.java @@ -158,27 +158,22 @@ public void testChangeUserAndCharsets() throws Exception { Connection connToMySQL = pooledConnection.getConnection(); this.rs = connToMySQL.createStatement().executeQuery("SELECT @@character_set_results"); assertTrue(this.rs.next()); - assertNull(this.rs.getString(1)); this.rs = connToMySQL.createStatement().executeQuery("SHOW SESSION VARIABLES LIKE 'character_set_client'"); assertTrue(this.rs.next()); - - // Because of utf8mb4 - assertTrue(this.rs.getString(2).startsWith("utf8")); + assertTrue(this.rs.getString(2).startsWith("utf8")); // Because of utf8mb4. connToMySQL.close(); connToMySQL = pooledConnection.getConnection(); this.rs = connToMySQL.createStatement().executeQuery("SELECT @@character_set_results"); assertTrue(this.rs.next()); - assertEquals(null, this.rs.getString(1)); + assertNull(this.rs.getString(1)); this.rs = connToMySQL.createStatement().executeQuery("SHOW SESSION VARIABLES LIKE 'character_set_client'"); assertTrue(this.rs.next()); - - // Because of utf8mb4 - assertTrue(this.rs.getString(2).startsWith("utf8")); + assertTrue(this.rs.getString(2).startsWith("utf8")); // Because of utf8mb4. pooledConnection.getConnection().close(); }