Re: JDBC Support for standard_conforming_strings - Mailing list pgsql-jdbc

Kris Jurka wrote:
> Right.  The parameters will either be sent out of line which won't
> require escaping or for the V2 path they will be sent in the query, but
> it isn't the array or bytea codes' responsibility to escape them
> correctly.  The code in core.v2.SimpleParameterList@setStringParameter
> should handle that for us.

Okay, thanks! Just wanted to have confirmation on that.

I am sending a work-in-progress patch now, as I have some more questions
on how to proceed.

Progress so far:
I have added a new method "getStandardConformingStrings" to
ProtocolConnection, with tracking for standard_conforming_strings in the
protocol connection implementation and the connection factory
implementation classes (v2 + v3).
BaseConnection has two new methods escapeString and getStringLiteral
(escapeString in style of PQescapeStringConn, getStringLiteral for
performance reasons to escape the string and add enclosing single-quotes in
one pass). BaseConnection.escapeString is for example used in
AbstractJdbc2DatabaseMetaData. The getStringLiteral method should be used
in core.v2.SimpleParameterList@setStringParameter, but see below for that.

I have also added support for standardConformingStrings to
Parser.parseSingleQuotes.

So far, the v3 Protocol path should work correctly with
standard_conforming_strings = true (except for the query parsing code in
AbstractJdbc2Statement).

Open items:
- Teach rest of parsing code about standard_conforming_strings
- Support E'' escaped string syntax
- Find a solution for the "V2 problem"

Question 1:
-----------
The V2 code as well as the query-parsing code in
AbstractJdbc2DatabaseMetaData is either mostly static or has otherwise no
reference to the connection object. There are two ways to tackle this:
either pass around standardConformingStrings (the variable name could be
stdStrings for brevity), or give the code access to the current connection.
For the parsing code, it's probably easier to just pass the variable to the
two or three methods. Objections?

For the V2 code, this leads me to question 2.

Question 2:
-----------
The problem with the V2 protocol code is, that tracking the state of
standard_conforming_strings is not really possible, AFAICS. Currently I
just read standard_conforming_strings at startup, and that's it. So as soon
as someone changes the variable, quoting will be incorrect, which is a
security issue. Therefore my new suggestion is to used the E'' string
syntax in the V2 protocol path if the server version is >= 8.2. The version
of the server cannot change during the connection, so we are save here.

For question 1, I would then suggest to add an argument to the V2Query
constructor that tells whether E'' should be used instead of '', or not.

For parsing inside the JDBC driver, we would still rely on the initially
reported value of standard_conforming_strings. We could also add an URL
parameter to specify what should be set initially.

Comments, ideas?
Best Regards
Michael Paesold
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/BaseConnection.java
pgjdbc.af39e026875b/org/postgresql/core/BaseConnection.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/BaseConnection.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/BaseConnection.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 143,148 ****
--- 143,169 ----
       */
      public byte[] encodeString(String str) throws SQLException;

+     /**
+      * Escapes a string for use as string-literal within an SQL command. The
+      * method chooses the applicable escaping rules based on the value of
+      * {@link ProtocolConnection#getStandardConformingStrings()}.
+      *
+      * @param str a string value
+      * @return the escaped representation of the string
+      * @throws SQLException if the string contains a <tt>\0</tt> character
+      */
+     public String escapeString(String str) throws SQLException;
+
+     /**
+      * Escapes a string for use as string-literal within an SQL command and
+      * returns the string-literal including beginning and ending single-quotes.
+      *
+      * @param str a string value
+      * @return the SQL string-literal for the given argument string
+      * @throws SQLException if the string contains a <tt>\0</tt> character
+      */
+     public String getStringLiteral(String str) throws SQLException;
+
      // Ew. Quick hack to give access to the connection-specific utils implementation.
      public TimestampUtils getTimestampUtils();

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/Parser.java pgjdbc.af39e026875b/org/postgresql/core/Parser.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/Parser.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/Parser.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 24,43 ****
       * one. The caller must call the method a second time for the second
       * part of the quoted string.
       */
!     public static int parseSingleQuotes(final char[] query, int offset) {
!         while (++offset < query.length)
          {
!             switch (query[offset])
              {
!             case '\\':
!                 ++offset;
!                 break;
!             case '\'':
!                 return offset;
!             default:
!                 break;
              }
          }
          return query.length;
      }

--- 24,63 ----
       * one. The caller must call the method a second time for the second
       * part of the quoted string.
       */
!     public static int parseSingleQuotes(final char[] query, int offset,
!                                         boolean standardConformingStrings) {
!         if (standardConformingStrings)
          {
!             // don NOT treat backslashes as escape characters
!             while (++offset < query.length)
!             {
!                 switch (query[offset])
!                 {
!                 case '\'':
!                     return offset;
!                 default:
!                     break;
!                 }
!             }
!         }
!         else
!         {
!             // treat backslashes as escape characters
!             while (++offset < query.length)
              {
!                 switch (query[offset])
!                 {
!                 case '\\':
!                     ++offset;
!                     break;
!                 case '\'':
!                     return offset;
!                 default:
!                     break;
!                 }
              }
          }
+
          return query.length;
      }

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/ProtocolConnection.java
pgjdbc.af39e026875b/org/postgresql/core/ProtocolConnection.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/ProtocolConnection.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/ProtocolConnection.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 67,72 ****
--- 67,84 ----
       * @return the current encoding in use by this connection
       */
      Encoding getEncoding();
+
+     /**
+      * Returns wether the server treats string-literals according to the SQL
+      * standard or if it uses traditional PostgreSQL escaping rules. Versions
+      * up to 8.1 always treated backslashes as escape characters in
+      * string-literals. Since 8.2, this depends on the value of the
+      * <tt>standard_conforming_strings<tt> server variable.
+      *
+      * @return true if the server treats string literals according to the SQL
+      *   standard
+      */
+     boolean getStandardConformingStrings();

      /**
       * Get the current transaction state of this connection.
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/ConnectionFactoryImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v2/ConnectionFactoryImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/ConnectionFactoryImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v2/ConnectionFactoryImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 471,475 ****
--- 471,487 ----

          if (logger.logDebug())
              logger.debug("Connection encoding (using JVM's nomenclature): " + protoConnection.getEncoding());
+
+         if (dbVersion.compareTo("8.1") >= 0)
+         {
+             // Server versions since 8.1 report standard_conforming_strings
+             results = runSetupQuery(protoConnection, "select current_setting('standard_conforming_strings')", true);
+             String value = protoConnection.getEncoding().decode(results[0]);
+             protoConnection.setStandardConformingStrings(value.equalsIgnoreCase("on"));
+         }
+         else
+         {
+             protoConnection.setStandardConformingStrings(false);
+         }
      }
  }
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/ProtocolConnectionImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v2/ProtocolConnectionImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/ProtocolConnectionImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v2/ProtocolConnectionImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 51,56 ****
--- 51,61 ----
          return serverVersion;
      }

+     public synchronized boolean getStandardConformingStrings()
+     {
+         return standardConformingStrings;
+     }
+
      public synchronized int getTransactionState()
      {
          return transactionState;
***************
*** 166,171 ****
--- 171,180 ----
          this.cancelKey = cancelKey;
      }

+     synchronized void setStandardConformingStrings(boolean value) {
+         standardConformingStrings = value;
+     }
+
      //
      // Package-private accessors called by the query executor
      //
***************
*** 197,202 ****
--- 206,212 ----
      private int cancelPid;
      private int cancelKey;

+     private boolean standardConformingStrings;
      private int transactionState;
      private SQLWarning warnings;

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/QueryExecutorImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v2/QueryExecutorImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/QueryExecutorImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v2/QueryExecutorImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 35,45 ****
      //

      public Query createSimpleQuery(String sql) {
!         return new V2Query(sql, false);
      }

      public Query createParameterizedQuery(String sql) {
!         return new V2Query(sql, true);
      }

      //
--- 35,45 ----
      //

      public Query createSimpleQuery(String sql) {
!         return new V2Query(sql, false, protoConnection.getStandardConformingStrings());
      }

      public Query createParameterizedQuery(String sql) {
!         return new V2Query(sql, true, protoConnection.getStandardConformingStrings());
      }

      //
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/SimpleParameterList.java
pgjdbc.af39e026875b/org/postgresql/core/v2/SimpleParameterList.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/SimpleParameterList.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v2/SimpleParameterList.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 68,74 ****
              char ch = value.charAt(i);
              if (ch == '\0')
                  throw new PSQLException(GT.tr("Zero bytes may not occur in string parameters."),
PSQLState.INVALID_PARAMETER_VALUE);
!             if (ch == '\\' || ch == '\'')
                  sbuf.append('\\');
              sbuf.append(ch);
          }
--- 68,74 ----
              char ch = value.charAt(i);
              if (ch == '\0')
                  throw new PSQLException(GT.tr("Zero bytes may not occur in string parameters."),
PSQLState.INVALID_PARAMETER_VALUE);
!             if (ch == '\\' || ch == '\'') //HERE quoting
                  sbuf.append('\\');
              sbuf.append(ch);
          }
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/V2Query.java
pgjdbc.af39e026875b/org/postgresql/core/v2/V2Query.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v2/V2Query.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v2/V2Query.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 17,23 ****
   * Query implementation for all queries via the V2 protocol.
   */
  class V2Query implements Query {
!     V2Query(String query, boolean withParameters) {
          if (!withParameters)
          {
              fragments = new String[] { query };
--- 17,23 ----
   * Query implementation for all queries via the V2 protocol.
   */
  class V2Query implements Query {
!     V2Query(String query, boolean withParameters, boolean standardConformingStrings) {
          if (!withParameters)
          {
              fragments = new String[] { query };
***************
*** 36,42 ****
              switch (aChars[i])
              {
              case '\'': // single-quotes
!                 i = Parser.parseSingleQuotes(aChars, i);
                  break;

              case '"': // double-quotes
--- 36,42 ----
              switch (aChars[i])
              {
              case '\'': // single-quotes
!                 i = Parser.parseSingleQuotes(aChars, i, standardConformingStrings);
                  break;

              case '"': // double-quotes
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/ConnectionFactoryImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v3/ConnectionFactoryImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/ConnectionFactoryImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v3/ConnectionFactoryImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 470,475 ****
--- 470,484 ----
                          throw new PSQLException(GT.tr("Protocol error.  Session setup failed."),
PSQLState.CONNECTION_UNABLE_TO_CONNECT);
                      pgStream.setEncoding(Encoding.getDatabaseEncoding("UNICODE"));
                  }
+                 else if (name.equals("standard_conforming_strings"))
+                 {
+                     if (value.equals("on"))
+                         protoConnection.setStandardConformingStrings(true);
+                     else if (value.equals("off"))
+                         protoConnection.setStandardConformingStrings(false);
+                     else
+                         throw new PSQLException(GT.tr("Protocol error.  Session setup failed."),
PSQLState.CONNECTION_UNABLE_TO_CONNECT);
+                 }

                  break;

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/ProtocolConnectionImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v3/ProtocolConnectionImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/ProtocolConnectionImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v3/ProtocolConnectionImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 30,35 ****
--- 30,37 ----
          this.database = database;
          this.logger = logger;
          this.executor = new QueryExecutorImpl(this, pgStream, info, logger);
+         // default value for server versions that don't report standard_conforming_strings
+         this.standardConformingStrings = false;
      }

      public String getHost() {
***************
*** 52,57 ****
--- 54,64 ----
          return serverVersion;
      }

+     public synchronized boolean getStandardConformingStrings()
+     {
+         return standardConformingStrings;
+     }
+
      public synchronized int getTransactionState()
      {
          return transactionState;
***************
*** 183,189 ****
      {
          transactionState = state;
      }
!
      public int getProtocolVersion()
      {
          return 3;
--- 190,201 ----
      {
          transactionState = state;
      }
!
!     synchronized void setStandardConformingStrings(boolean value)
!     {
!         standardConformingStrings = value;
!     }
!
      public int getProtocolVersion()
      {
          return 3;
***************
*** 193,198 ****
--- 205,211 ----
      private int cancelPid;
      private int cancelKey;

+     private boolean standardConformingStrings;
      private int transactionState;
      private SQLWarning warnings;

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/QueryExecutorImpl.java
pgjdbc.af39e026875b/org/postgresql/core/v3/QueryExecutorImpl.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/core/v3/QueryExecutorImpl.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/core/v3/QueryExecutorImpl.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 56,62 ****
          return parseQuery(sql, true);
      }

!     private static Query parseQuery(String query, boolean withParameters) {
          // Parse query and find parameter placeholders;
          // also break the query into separate statements.

--- 56,62 ----
          return parseQuery(sql, true);
      }

!     private Query parseQuery(String query, boolean withParameters) {
          // Parse query and find parameter placeholders;
          // also break the query into separate statements.

***************
*** 66,71 ****
--- 66,73 ----
          int fragmentStart = 0;
          int inParen = 0;

+         boolean standardConformingStrings = protoConnection.getStandardConformingStrings();
+
          char []aChars = query.toCharArray();

          for (int i = 0; i < aChars.length; ++i)
***************
*** 73,79 ****
              switch (aChars[i])
              {
              case '\'': // single-quotes
!                 i = Parser.parseSingleQuotes(aChars, i);
                  break;

              case '"': // double-quotes
--- 75,81 ----
              switch (aChars[i])
              {
              case '\'': // single-quotes
!                 i = Parser.parseSingleQuotes(aChars, i, standardConformingStrings);
                  break;

              case '"': // double-quotes
***************
*** 1360,1365 ****
--- 1362,1381 ----
                          handler.handleError(new PSQLException(GT.tr("The server''s DateStyle parameter was changed to
{0}.The JDBC driver requires DateStyle to begin with ISO for correct operation.", value),
PSQLState.CONNECTION_FAILURE));
                          endQuery = true;
                      }
+
+                     if (name.equals("standard_conforming_strings"))
+                     {
+                         if (value.equals("on"))
+                             protoConnection.setStandardConformingStrings(true);
+                         else if (value.equals("off"))
+                             protoConnection.setStandardConformingStrings(false);
+                         else
+                         {
+                             protoConnection.close(); // we're screwed now; we don't know how to escape string
literals
+                             handler.handleError(new PSQLException(GT.tr("The server''s standard_conforming_strings
parameterwas reported as {0}. The JDBC driver expected on or off.", value), PSQLState.CONNECTION_FAILURE)); 
+                             endQuery = true;
+                         }
+                     }
                  }
                  break;

diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2Connection.java
pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2Connection.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2Connection.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2Connection.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 938,943 ****
--- 938,993 ----
          }
      }

+     public String escapeString(String str) throws SQLException {
+         return escapeStringInternal(str, false);
+     }
+
+     public String getStringLiteral(String str) throws SQLException {
+         return escapeStringInternal(str, true);
+     }
+
+     private String escapeStringInternal(String str, boolean addQuotes) throws SQLException {
+         StringBuffer sbuf = new StringBuffer(2 + str.length() * 11 / 10); // Add 10% for escaping.
+
+         if (addQuotes)
+             sbuf.append('\'');
+
+         if (protoConnection.getStandardConformingStrings())
+         {
+             // With standard_conforming_strings on, escape only single-quotes.
+             for (int i = 0; i < str.length(); ++i)
+             {
+                 char ch = str.charAt(i);
+                 if (ch == '\0')
+                     throw new PSQLException(GT.tr("Zero bytes may not occur in string parameters."),
PSQLState.INVALID_PARAMETER_VALUE);
+                 if (ch == '\'')
+                     sbuf.append('\'');
+                 sbuf.append(ch);
+             }
+         }
+         else
+         {
+             // With standard_conforming_string off, escape backslashes and
+             // single-quotes, but still escape single-quotes by doubling, to
+             // avoid a security hazord if the reported value of
+             // standard_conforming_strings is incorrect.
+             for (int i = 0; i < str.length(); ++i)
+             {
+                 char ch = str.charAt(i);
+                 if (ch == '\0')
+                     throw new PSQLException(GT.tr("Zero bytes may not occur in string parameters."),
PSQLState.INVALID_PARAMETER_VALUE);
+                 if (ch == '\\' || ch == '\'')
+                     sbuf.append(ch);
+                 sbuf.append(ch);
+             }
+         }
+
+         if (addQuotes)
+             sbuf.append('\'');
+
+         return sbuf.toString();
+     }
+
      /*
       * This returns the java.sql.Types type for a PG type oid
       *
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java
pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2DatabaseMetaData.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 1566,1584 ****
      /**
       * Escape single quotes with another single quote.
       */
!     protected static String escapeQuotes(String s) {
!         StringBuffer sb = new StringBuffer();
!         int length = s.length();
!         for (int i = 0; i < length; i++)
!         {
!             char c = s.charAt(i);
!             if (c == '\'' || c == '\\')
!             {
!                 sb.append('\\');
!             }
!             sb.append(c);
!         }
!         return sb.toString();
      }

      /*
--- 1566,1573 ----
      /**
       * Escape single quotes with another single quote.
       */
!     protected String escapeQuotes(String s) throws SQLException {
!         return connection.escapeString(s);
      }

      /*
diff -crN pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2Statement.java
pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
*** pgjdbc.1cc1d2edf1a6/org/postgresql/jdbc2/AbstractJdbc2Statement.java    2006-11-06 10:24:03.000000000 +0100
--- pgjdbc.af39e026875b/org/postgresql/jdbc2/AbstractJdbc2Statement.java    2006-11-06 10:24:03.000000000 +0100
***************
*** 878,884 ****
                  if (c == '\'')       // end of string?
                      state = IN_SQLCODE;
                  else if (c == '\\')      // a backslash?
!                     state = BACKSLASH;

                  newsql.append(c);
                  break;
--- 878,884 ----
                  if (c == '\'')       // end of string?
                      state = IN_SQLCODE;
                  else if (c == '\\')      // a backslash?
!                     state = BACKSLASH;   // HERE quoting

                  newsql.append(c);
                  break;
***************
*** 2253,2259 ****
                      inQuotes = !inQuotes;
                      ++i;
                  }
!                 else if (inQuotes && ch == '\\')
                  {
                      // Backslash in string constant, skip next character.
                      i += 2;
--- 2253,2259 ----
                      inQuotes = !inQuotes;
                      ++i;
                  }
!                 else if (inQuotes && ch == '\\') //HERE quoting
                  {
                      // Backslash in string constant, skip next character.
                      i += 2;