Message-ID: <gh-pgjdbc-pgjdbc-3171-c2016500009@github.com>
From: "sehrope (@sehrope)" <noreply+sehrope@github.com>
To: "pgjdbc/pgjdbc" <noreply+pgjdbc-pgjdbc@github.com>
Date: Sat, 23 Mar 2024 13:46:13 +0000
Subject: Re: [pgjdbc/pgjdbc] PR #3171: validates resultsetParams in PGStatement constructor. uses assertThro…
In-Reply-To: <gh-pgjdbc-pgjdbc-3171@github.com>
References: <gh-pgjdbc-pgjdbc-3171@github.com>
List-Id: <gh-pgjdbc-pgjdbc.github.com>
X-GitHub-Author-Login: sehrope
X-GitHub-Comment-Id: 2016500009
X-GitHub-Comment-Type: issue_comment
X-GitHub-Issue: 3171
X-GitHub-Repo: pgjdbc/pgjdbc
X-GitHub-Type: comment
X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/3171#issuecomment-2016500009
Content-Type: text/plain; charset=utf-8

Very nice.

Comparing this branch against the state of the repo just before the prior attempt at this (i.e. pretend the previous commit didn't happen) shows a nice clean diff and makes it easy to see both the change and its correctness:

```diff
$ git diff 24f2c7eea4764cd4e09c2c71bc8c38ee5d4178e5 pgjdbc/src/main/java/
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
index 315781d4..ab574c18 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
@@ -1408,16 +1408,14 @@ public class PgConnection implements BaseConnection {
   public PreparedStatement prepareStatement(String sql, int resultSetType, int resultSetConcurrency,
       int resultSetHoldability) throws SQLException {
     checkClosed();
-    return new PgPreparedStatement(this, sql, resultSetType, resultSetConcurrency,
-        resultSetHoldability);
+    return new PgPreparedStatement(this, sql, resultSetType, resultSetConcurrency, resultSetHoldability);
   }
 
   @Override
   public CallableStatement prepareCall(String sql, int resultSetType, int resultSetConcurrency,
       int resultSetHoldability) throws SQLException {
     checkClosed();
-    return new PgCallableStatement(this, sql, resultSetType, resultSetConcurrency,
-        resultSetHoldability);
+    return new PgCallableStatement(this, sql, resultSetType, resultSetConcurrency, resultSetHoldability);
   }
 
   @Override
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
index 944d56eb..98dd512e 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
@@ -161,11 +161,26 @@ public class PgStatement implements Statement, BaseStatement {
       throws SQLException {
     this.connection = c;
     forceBinaryTransfers |= c.getForceBinary();
+    // validation check for allowed values of resultset type
+    if (rsType != ResultSet.TYPE_FORWARD_ONLY && rsType != ResultSet.TYPE_SCROLL_INSENSITIVE && rsType != ResultSet.TYPE_SCROLL_SENSITIVE) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet type"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     resultsettype = rsType;
+    // validation check for allowed values of resultset concurrency
+    if (rsConcurrency != ResultSet.CONCUR_READ_ONLY && rsConcurrency != ResultSet.CONCUR_UPDATABLE) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet concurrency"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     concurrency = rsConcurrency;
     setFetchSize(c.getDefaultFetchSize());
     setPrepareThreshold(c.getPrepareThreshold());
     setAdaptiveFetch(c.getAdaptiveFetch());
+    // validation check for allowed values of resultset holdability
+    if (rsHoldability != ResultSet.HOLD_CURSORS_OVER_COMMIT && rsHoldability != ResultSet.CLOSE_CURSORS_AT_COMMIT) {
+      throw new PSQLException(GT.tr("Unknown value for ResultSet holdability"),
+          PSQLState.INVALID_PARAMETER_VALUE);
+    }
     this.rsHoldability = rsHoldability;
   } 
```