From 223dfafa342a6451356343658424f6ea1da2e61b Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 14 May 2024 00:59:07 +0100 Subject: [PATCH] Fix for Bug#114989 (Bug#36612566), Setting null value in setClientInfo throws an NPE. Change-Id: Ie1a2be98cc2079ecb7572e9c1bb3548583ce9444 --- CHANGES | 2 + .../com/mysql/cj/jdbc/ClientInfoProvider.java | 11 +-- .../mysql/cj/jdbc/ClientInfoProviderSP.java | 11 +-- .../cj/jdbc/CommentClientInfoProvider.java | 57 +++++------- .../regression/ConnectionRegressionTest.java | 87 +++++++++++++++++++ 5 files changed, 125 insertions(+), 43 deletions(-) diff --git a/CHANGES b/CHANGES index 134787afa..53eaf2727 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,8 @@ Version 9.0.0 + - Fix for Bug#114989 (Bug#36612566), Setting null value in setClientInfo throws an NPE. + - WL#16376, Set 'caching_sha2_password' as default fallback authentication plugin. - WL#16342, Update MySQL error codes mapping. diff --git a/src/main/user-api/java/com/mysql/cj/jdbc/ClientInfoProvider.java b/src/main/user-api/java/com/mysql/cj/jdbc/ClientInfoProvider.java index 2a154de85..d83f0838d 100644 --- a/src/main/user-api/java/com/mysql/cj/jdbc/ClientInfoProvider.java +++ b/src/main/user-api/java/com/mysql/cj/jdbc/ClientInfoProvider.java @@ -20,6 +20,7 @@ package com.mysql.cj.jdbc; +import java.sql.Connection; import java.sql.SQLClientInfoException; import java.sql.SQLException; import java.util.Properties; @@ -43,7 +44,7 @@ public interface ClientInfoProvider { * @throws SQLException * if initialization fails. */ - void initialize(java.sql.Connection conn, Properties configurationProps) throws SQLException; + void initialize(Connection conn, Properties configurationProps) throws SQLException; /** * Called once by the driver when the connection this provider instance @@ -74,7 +75,7 @@ public interface ClientInfoProvider { * @return client info as Properties * @see java.sql.Connection#getClientInfo() */ - Properties getClientInfo(java.sql.Connection conn) throws SQLException; + Properties getClientInfo(Connection conn) throws SQLException; /** * Returns the client info for the connection that this provider @@ -95,7 +96,7 @@ public interface ClientInfoProvider { * @return the client info by given property name * @see java.sql.Connection#getClientInfo(java.lang.String) */ - String getClientInfo(java.sql.Connection conn, String name) throws SQLException; + String getClientInfo(Connection conn, String name) throws SQLException; /** * Sets the client info for the connection that this provider @@ -116,7 +117,7 @@ public interface ClientInfoProvider { * * @see java.sql.Connection#setClientInfo(java.util.Properties) */ - void setClientInfo(java.sql.Connection conn, Properties properties) throws SQLClientInfoException; + void setClientInfo(Connection conn, Properties properties) throws SQLClientInfoException; /** * Sets the client info for the connection that this provider @@ -139,6 +140,6 @@ public interface ClientInfoProvider { * * @see java.sql.Connection#setClientInfo(java.lang.String,java.lang.String) */ - void setClientInfo(java.sql.Connection conn, String name, String value) throws SQLClientInfoException; + void setClientInfo(Connection conn, String name, String value) throws SQLClientInfoException; } diff --git a/src/main/user-impl/java/com/mysql/cj/jdbc/ClientInfoProviderSP.java b/src/main/user-impl/java/com/mysql/cj/jdbc/ClientInfoProviderSP.java index b9b88db8e..c9658cc16 100644 --- a/src/main/user-impl/java/com/mysql/cj/jdbc/ClientInfoProviderSP.java +++ b/src/main/user-impl/java/com/mysql/cj/jdbc/ClientInfoProviderSP.java @@ -20,6 +20,7 @@ package com.mysql.cj.jdbc; +import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLClientInfoException; @@ -45,7 +46,7 @@ public class ClientInfoProviderSP implements ClientInfoProvider { private final Lock lock = new ReentrantLock(); @Override - public void initialize(java.sql.Connection conn, Properties configurationProps) throws SQLException { + public void initialize(Connection conn, Properties configurationProps) throws SQLException { this.lock.lock(); try { String identifierQuote = ((JdbcConnection) conn).getSession().getIdentifierQuoteString(); @@ -93,7 +94,7 @@ public void destroy() throws SQLException { } @Override - public Properties getClientInfo(java.sql.Connection conn) throws SQLException { + public Properties getClientInfo(Connection conn) throws SQLException { this.lock.lock(); try { ResultSet rs = null; @@ -121,7 +122,7 @@ public Properties getClientInfo(java.sql.Connection conn) throws SQLException { } @Override - public String getClientInfo(java.sql.Connection conn, String name) throws SQLException { + public String getClientInfo(Connection conn, String name) throws SQLException { this.lock.lock(); try { ResultSet rs = null; @@ -150,7 +151,7 @@ public String getClientInfo(java.sql.Connection conn, String name) throws SQLExc } @Override - public void setClientInfo(java.sql.Connection conn, Properties properties) throws SQLClientInfoException { + public void setClientInfo(Connection conn, Properties properties) throws SQLClientInfoException { this.lock.lock(); try { Enumeration propNames = properties.propertyNames(); @@ -172,7 +173,7 @@ public void setClientInfo(java.sql.Connection conn, Properties properties) throw } @Override - public void setClientInfo(java.sql.Connection conn, String name, String value) throws SQLClientInfoException { + public void setClientInfo(Connection conn, String name, String value) throws SQLClientInfoException { this.lock.lock(); try { this.setClientInfoSp.setString(1, name); diff --git a/src/main/user-impl/java/com/mysql/cj/jdbc/CommentClientInfoProvider.java b/src/main/user-impl/java/com/mysql/cj/jdbc/CommentClientInfoProvider.java index fe3c2b8f8..1555f28bd 100644 --- a/src/main/user-impl/java/com/mysql/cj/jdbc/CommentClientInfoProvider.java +++ b/src/main/user-impl/java/com/mysql/cj/jdbc/CommentClientInfoProvider.java @@ -20,10 +20,11 @@ package com.mysql.cj.jdbc; +import java.sql.Connection; import java.sql.SQLClientInfoException; import java.sql.SQLException; -import java.util.Enumeration; import java.util.Properties; +import java.util.stream.Collectors; /** * An implementation of ClientInfoProvider that exposes the client info as a comment prepended to all statements issued by the driver. @@ -36,7 +37,7 @@ public class CommentClientInfoProvider implements ClientInfoProvider { private Properties clientInfo; @Override - public synchronized void initialize(java.sql.Connection conn, Properties configurationProps) throws SQLException { + public synchronized void initialize(Connection conn, Properties configurationProps) throws SQLException { this.clientInfo = new Properties(); } @@ -46,54 +47,44 @@ public synchronized void destroy() throws SQLException { } @Override - public synchronized Properties getClientInfo(java.sql.Connection conn) throws SQLException { - return this.clientInfo; + public synchronized Properties getClientInfo(Connection conn) throws SQLException { + Properties clientInfoOut = new Properties(); + clientInfoOut.putAll(this.clientInfo); + return clientInfoOut; } @Override - public synchronized String getClientInfo(java.sql.Connection conn, String name) throws SQLException { + public synchronized String getClientInfo(Connection conn, String name) throws SQLException { return this.clientInfo.getProperty(name); } @Override - public synchronized void setClientInfo(java.sql.Connection conn, Properties properties) throws SQLClientInfoException { + public synchronized void setClientInfo(Connection conn, Properties properties) throws SQLClientInfoException { this.clientInfo = new Properties(); - - Enumeration propNames = properties.propertyNames(); - - while (propNames.hasMoreElements()) { - String name = (String) propNames.nextElement(); - - this.clientInfo.put(name, properties.getProperty(name)); + if (properties != null) { + this.clientInfo.putAll(properties); } - setComment(conn); } @Override - public synchronized void setClientInfo(java.sql.Connection conn, String name, String value) throws SQLClientInfoException { - this.clientInfo.setProperty(name, value); + public synchronized void setClientInfo(Connection conn, String name, String value) throws SQLClientInfoException { + if (value == null) { + this.clientInfo.remove(name); + } else { + this.clientInfo.setProperty(name, value); + } setComment(conn); } - private synchronized void setComment(java.sql.Connection conn) { - StringBuilder commentBuf = new StringBuilder(); - - Enumeration propNames = this.clientInfo.propertyNames(); - - while (propNames.hasMoreElements()) { - String name = (String) propNames.nextElement(); - - if (commentBuf.length() > 0) { - commentBuf.append(", "); - } - - commentBuf.append("" + name); - commentBuf.append("="); - commentBuf.append("" + this.clientInfo.getProperty(name)); + private synchronized void setComment(Connection conn) throws SQLClientInfoException { + String clientInfoComment = this.clientInfo.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(", ")); + try { + conn.unwrap(JdbcConnection.class).setStatementComment(clientInfoComment); + } catch (SQLException e) { + SQLClientInfoException clientInfoEx = new SQLClientInfoException(); + clientInfoEx.initCause(e); } - - ((com.mysql.cj.jdbc.JdbcConnection) conn).setStatementComment(commentBuf.toString()); } } diff --git a/src/test/java/testsuite/regression/ConnectionRegressionTest.java b/src/test/java/testsuite/regression/ConnectionRegressionTest.java index 99958fd0a..3ee1bfe6f 100644 --- a/src/test/java/testsuite/regression/ConnectionRegressionTest.java +++ b/src/test/java/testsuite/regression/ConnectionRegressionTest.java @@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -12090,4 +12091,90 @@ private void testBug23143279RunTest(String lbStrategy) throws Exception { testConn.close(); } + /** + * Test fix for Bug#114989 (36612566), Setting null value in setClientInfo throws an NPE. + * + * @throws Exception + */ + @Test + void testBug114989() throws Exception { + final String clientInfoKey1 = "testBug114989-key1"; + final String clientInfoValue1 = "testBug114989-value1"; + final String clientInfoKey2 = "testBug114989-key2"; + final String clientInfoValue2 = "testBug114989-value2"; + + Properties props = new Properties(); + props.setProperty(PropertyKey.clientInfoProvider.getKeyName(), CommentClientInfoProvider.class.getName()); + try (Connection testConn = getConnectionWithProps(props)) { + // Start with empty client info. + Properties clientInfoOut = testConn.getClientInfo(); + assertTrue(clientInfoOut.isEmpty()); + + // Add 2 properties via Properties. + Properties clientInfoIn = new Properties(); + clientInfoIn.setProperty(clientInfoKey1, clientInfoValue1); + clientInfoIn.setProperty(clientInfoKey2, clientInfoValue2); + testConn.setClientInfo(clientInfoIn); + clientInfoOut = testConn.getClientInfo(); + assertNotSame(clientInfoIn, clientInfoOut); + assertEquals(2, clientInfoOut.size()); + assertTrue(clientInfoOut.containsKey(clientInfoKey1)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey1)); + assertTrue(clientInfoOut.containsKey(clientInfoKey2)); + assertEquals(clientInfoValue2, testConn.getClientInfo(clientInfoKey2)); + + // Clear the client info map. + testConn.setClientInfo(new Properties()); + clientInfoOut = testConn.getClientInfo(); + assertTrue(clientInfoOut.isEmpty()); + + // Add one client info name=value pair. + testConn.setClientInfo(clientInfoKey1, clientInfoValue1); + clientInfoOut = testConn.getClientInfo(); + assertFalse(clientInfoOut.isEmpty()); + assertEquals(1, clientInfoOut.size()); + assertTrue(clientInfoOut.containsKey(clientInfoKey1)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey1)); + + // Add another client info name=value pair. + testConn.setClientInfo(clientInfoKey2, clientInfoValue2); + clientInfoOut = testConn.getClientInfo(); + assertFalse(clientInfoOut.isEmpty()); + assertEquals(2, clientInfoOut.size()); + assertTrue(clientInfoOut.containsKey(clientInfoKey1)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey1)); + assertTrue(clientInfoOut.containsKey(clientInfoKey2)); + assertEquals(clientInfoValue2, testConn.getClientInfo(clientInfoKey2)); + + // Returned Properties is never the same. + assertNotSame(testConn.getClientInfo(), testConn.getClientInfo()); + + // Replace one of the client info properties value. + testConn.setClientInfo(clientInfoKey2, clientInfoValue1); + clientInfoOut = testConn.getClientInfo(); + assertFalse(clientInfoOut.isEmpty()); + assertEquals(2, clientInfoOut.size()); + assertTrue(clientInfoOut.containsKey(clientInfoKey1)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey1)); + assertTrue(clientInfoOut.containsKey(clientInfoKey2)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey2)); + + // Undefined client info property name returns null. + assertNull(testConn.getClientInfo("undefined")); + + // Clear one of the client info properties. + testConn.setClientInfo(clientInfoKey2, null); + clientInfoOut = testConn.getClientInfo(); + assertFalse(clientInfoOut.isEmpty()); + assertEquals(1, clientInfoOut.size()); + assertTrue(clientInfoOut.containsKey(clientInfoKey1)); + assertEquals(clientInfoValue1, testConn.getClientInfo(clientInfoKey1)); + + // Clear the client info map. + testConn.setClientInfo(null); + clientInfoOut = testConn.getClientInfo(); + assertTrue(clientInfoOut.isEmpty()); + } + } + }