Revert "Prepare for release (#2580)"

This reverts commit bd91c4c.
pgjdbc · Aug 3, 2022 · d6cc134 · d6cc134
1 parent bd91c4c
commit d6cc134
Show file tree

Hide file tree

Showing 13 changed files with 260 additions and 197 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,29 +5,15 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 ### Changed
+chore: skip publishing pgjdbc-osgi-test to Central
+chore: bump Gradle to 7.5
+test: update JUnit to 5.8.2
 
 ### Added
-
-### Fixed
-
-[42.4.1] (2022-08-01 16:24:20 -0400)
-### Security
-- fix: CVE-2022-31197 Fixes SQL generated in PgResultSet.refresh() to escape column identifiers so as to prevent SQL injection.
-  - Previously, the column names for both key and data columns in the table were copied as-is into the generated
-  SQL. This allowed a malicious table with column names that include statement terminator to be parsed and
-  executed as multiple separate commands.
-  - Also adds a new test class ResultSetRefreshTest to verify this change.
-  - Reported by [Sho Kato](https://github.com/kato-sho)
-
-### Changed
-- chore: skip publishing pgjdbc-osgi-test to Central
-- chore: bump Gradle to 7.5
-- test: update JUnit to 5.8.2
-
-### Added
-- chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
-- chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
-- chore: support building pgjdbc with Java 17
+chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
+chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
+chore: support building pgjdbc with Java 17
+feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads)
 
 ### Fixed
 
@@ -723,5 +709,4 @@ thrown to caller to be dealt with so no need to log at this verbosity by pgjdbc
 [42.3.4]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.4...REL42.3.5
 [42.3.5]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.5...REL42.3.6
 [42.3.6]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.6...REL42.4.0
-[42.4.0]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.0...REL42.4.1
-[Unreleased]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.1...HEAD
+[Unreleased]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.0...HEAD
diff --git a/README.md b/README.md
@@ -118,6 +118,7 @@ In addition to the standard connection parameters the driver supports a number o
 | loginTimeout                  | Integer | 0       | Specify how long to wait for establishment of a database connection.|
 | connectTimeout                | Integer | 10      | The timeout value used for socket connect operations. |
 | socketTimeout                 | Integer | 0       | The timeout value used for socket read operations. |
+| sslResponseTimeout            | Integer | 5000    | Socket timeout waiting for a response from a request for SSL upgrade from the server. |                                                                                                                                                                        
 | tcpKeepAlive                  | Boolean | false   | Enable or disable TCP keep-alive. |
 | tcpNoDelay                    | Boolean | true    | Enable or disable TCP no delay. |
 | ApplicationName               | String  | PostgreSQL JDBC Driver    | The application name (require server version >= 9.0). If assumeMinServerVersion is set to >= 9.0 this will be sent in the startup packets, otherwise after the connection is made |

diff --git a/docs/_posts/2022-08-03-42.4.1-release.md b/docs/_posts/2022-08-03-42.4.1-release.md
diff --git a/docs/documentation/head/connect.md b/docs/documentation/head/connect.md
@@ -327,6 +327,10 @@ Connection conn = DriverManager.getConnection(url);
 	detecting network problems. The timeout is specified in seconds and a
 	value of zero means that it is disabled.
 
+* **sslResponseTimeout** = int
+    The timeout value in milliseconds that we wait for a response from the server
+    after requesting the connection be upgraded to SSL.
+
 * **cancelSignalTimeout** = int
 
 	Cancel command is sent out of band over its own connection, so cancel message can itself get

diff --git a/pgjdbc/src/main/java/org/postgresql/PGProperty.java b/pgjdbc/src/main/java/org/postgresql/PGProperty.java
@@ -672,6 +672,15 @@ public enum PGProperty {
       null,
       "A class, implementing javax.security.auth.callback.CallbackHandler that can handle PassworCallback for the ssl password."),
 
+  /**
+   * <p>After requesting an upgrade to SSL from the server there are reports of the server not responding due to a failover
+   * without a timeout here, the client can wait forever. This timeout will be set before the request and reset after </p>
+   */
+  SSL_RESPONSE_TIMEOUT(
+      "sslResponseTimeout",
+      "5000",
+      "Time in milliseconds we wait for a response from the server after requesting SSL upgrade"),
+
   /**
    * File containing the root certificate when validating server ({@code sslmode} = {@code
    * verify-ca} or {@code verify-full}). Default will be the file {@code root.crt} in {@code

diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java
@@ -532,6 +532,17 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     LOGGER.log(Level.FINEST, " FE=> SSLRequest");
 
+    int sslTimeout = PGProperty.SSL_RESPONSE_TIMEOUT.getInt(info);
+    int currentTimeout = pgStream.getSocket().getSoTimeout();
+
+    // if the current timeout is less than sslTimeout then
+    // use the smaller timeout. We could do something tricky
+    // here to not set it in that case but this is pretty readable
+    if (currentTimeout > 0 && currentTimeout < sslTimeout) {
+      sslTimeout = currentTimeout;
+    }
+
+    pgStream.getSocket().setSoTimeout(sslTimeout);
     // Send SSL request packet
     pgStream.sendInteger4(8);
     pgStream.sendInteger2(1234);
@@ -540,6 +551,8 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     // Now get the response from the backend, one of N, E, S.
     int beresp = pgStream.receiveChar();
+    pgStream.getSocket().setSoTimeout(currentTimeout);
+
     switch (beresp) {
       case 'E':
         LOGGER.log(Level.FINEST, " <=BE SSLError");

diff --git a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
@@ -354,6 +354,24 @@ public void setConnectTimeout(int connectTimeout) {
     PGProperty.CONNECT_TIMEOUT.set(properties, connectTimeout);
   }
 
+  /**
+   *
+   * @return SSL ResponseTimeout
+   * @see PGProperty#SSL_RESPONSE_TIMEOUT
+   */
+  public int getSslResponseTimeout() {
+    return PGProperty.SSL_RESPONSE_TIMEOUT.getIntNoCheck(properties);
+  }
+
+  /**
+   *
+   * @param sslResponseTimeout ssl response timeout
+   * @see PGProperty#SSL_RESPONSE_TIMEOUT
+   */
+  public void setSslResponseTimeout(int sslResponseTimeout) {
+    PGProperty.SSL_RESPONSE_TIMEOUT.set(properties,sslResponseTimeout);
+  }
+
   /**
    * @return protocol version
    * @see PGProperty#PROTOCOL_VERSION

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java
@@ -80,79 +80,79 @@ public int executeUpdate() throws SQLException {
 
   @Override
   public boolean executeWithFlags(int flags) throws SQLException {
-    boolean hasResultSet = super.executeWithFlags(flags);
-    int[] functionReturnType = this.functionReturnType;
-    if (!isFunction || !returnTypeSet || functionReturnType == null) {
-      return hasResultSet;
-    }
-
-    // If we are executing and there are out parameters
-    // callable statement function set the return data
-    if (!hasResultSet) {
-      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-          PSQLState.NO_DATA);
-    }
+    synchronized (this) {
+      boolean hasResultSet = super.executeWithFlags(flags);
+      int[] functionReturnType = this.functionReturnType;
+      if (!isFunction || !returnTypeSet || functionReturnType == null) {
+        return hasResultSet;
+      }
 
-    ResultSet rs = castNonNull(getResultSet());
-    if (!rs.next()) {
-      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-          PSQLState.NO_DATA);
-    }
+      // If we are executing and there are out parameters
+      // callable statement function set the return data
+      if (!hasResultSet) {
+        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+            PSQLState.NO_DATA);
+      }
 
-    // figure out how many columns
-    int cols = rs.getMetaData().getColumnCount();
+      ResultSet rs = castNonNull(getResultSet());
+      if (!rs.next()) {
+        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+            PSQLState.NO_DATA);
+      }
 
-    int outParameterCount = preparedParameters.getOutParameterCount();
+      // figure out how many columns
+      int cols = rs.getMetaData().getColumnCount();
 
-    if (cols != outParameterCount) {
-      throw new PSQLException(
-          GT.tr("A CallableStatement was executed with an invalid number of parameters"),
-          PSQLState.SYNTAX_ERROR);
-    }
+      int outParameterCount = preparedParameters.getOutParameterCount();
 
-    // reset last result fetched (for wasNull)
-    lastIndex = 0;
-
-    // allocate enough space for all possible parameters without regard to in/out
-    @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
-    this.callResult = callResult;
-
-    // move them into the result set
-    for (int i = 0, j = 0; i < cols; i++, j++) {
-      // find the next out parameter, the assumption is that the functionReturnType
-      // array will be initialized with 0 and only out parameters will have values
-      // other than 0. 0 is the value for java.sql.Types.NULL, which should not
-      // conflict
-      while (j < functionReturnType.length && functionReturnType[j] == 0) {
-        j++;
+      if (cols != outParameterCount) {
+        throw new PSQLException(
+            GT.tr("A CallableStatement was executed with an invalid number of parameters"),
+            PSQLState.SYNTAX_ERROR);
       }
 
-      callResult[j] = rs.getObject(i + 1);
-      int columnType = rs.getMetaData().getColumnType(i + 1);
+      // reset last result fetched (for wasNull)
+      lastIndex = 0;
+
+      // allocate enough space for all possible parameters without regard to in/out
+      @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
+      this.callResult = callResult;
+
+      // move them into the result set
+      for (int i = 0, j = 0; i < cols; i++, j++) {
+        // find the next out parameter, the assumption is that the functionReturnType
+        // array will be initialized with 0 and only out parameters will have values
+        // other than 0. 0 is the value for java.sql.Types.NULL, which should not
+        // conflict
+        while (j < functionReturnType.length && functionReturnType[j] == 0) {
+          j++;
+        }
 
-      if (columnType != functionReturnType[j]) {
-        // this is here for the sole purpose of passing the cts
-        if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
-          // return it as a float
-          Object result = callResult[j];
-          if (result != null) {
-            callResult[j] = ((Double) result).floatValue();
+        callResult[j] = rs.getObject(i + 1);
+        int columnType = rs.getMetaData().getColumnType(i + 1);
+
+        if (columnType != functionReturnType[j]) {
+          // this is here for the sole purpose of passing the cts
+          if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
+            // return it as a float
+            Object result = callResult[j];
+            if (result != null) {
+              callResult[j] = ((Double) result).floatValue();
+            }
+          } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
+            // For backwards compatibility reasons we support that ref cursors can be
+            // registered with both Types.OTHER and Types.REF_CURSOR so we allow
+            // this specific mismatch
+          } else {
+            throw new PSQLException(GT.tr(
+                "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
+                i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
+                PSQLState.DATA_TYPE_MISMATCH);
           }
-        } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
-          // For backwards compatibility reasons we support that ref cursors can be
-          // registered with both Types.OTHER and Types.REF_CURSOR so we allow
-          // this specific mismatch
-        } else {
-          throw new PSQLException(GT.tr(
-              "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
-              i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
-              PSQLState.DATA_TYPE_MISMATCH);
         }
-      }
 
-    }
-    rs.close();
-    synchronized (this) {
+      }
+      rs.close();
       result = null;
     }
     return false;

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
@@ -1454,8 +1454,13 @@ public boolean isValid(int timeout) throws SQLException {
           statement.execute("IDENTIFY_SYSTEM");
           statement.close();
         } else {
-          if (checkConnectionQuery == null) {
-            checkConnectionQuery = prepareStatement("");
+          PreparedStatement checkConnectionQuery;
+          synchronized (this) {
+            checkConnectionQuery = this.checkConnectionQuery;
+            if (checkConnectionQuery == null) {
+              checkConnectionQuery = prepareStatement("");
+              this.checkConnectionQuery = checkConnectionQuery;
+            }
           }
           checkConnectionQuery.executeUpdate();
         }

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java
@@ -2696,11 +2696,11 @@ public ResultSet getUDTs(@Nullable String catalog, @Nullable String schemaPatter
       long longTypOid = typeInfo.intOidToLong(typOid);
       int sqlType = typeInfo.getSQLType(typOid);
 
-      sqlwhen.append(" when oid = ").append(longTypOid).append(" then ").append(sqlType);
+      sqlwhen.append(" when base_type.oid = ").append(longTypOid).append(" then ").append(sqlType);
     }
     sql += sqlwhen.toString();
 
-    sql += " else " + java.sql.Types.OTHER + " end from pg_type where oid=t.typbasetype) "
+    sql += " else " + java.sql.Types.OTHER + " end from pg_type base_type where base_type.oid=t.typbasetype) "
         + "else null end as base_type "
         + "from pg_catalog.pg_type t, pg_catalog.pg_namespace n where t.typnamespace = n.oid and n.nspname != 'pg_catalog' and n.nspname != 'pg_toast'";