Re: JDBC Support for standard_conforming_strings - Mailing list pgsql-jdbc
From | Michael Paesold |
---|---|
Subject | Re: JDBC Support for standard_conforming_strings |
Date | |
Msg-id | 45563B85.4010501@gmx.at Whole thread Raw |
In response to | Re: JDBC Support for standard_conforming_strings (Michael Paesold <mpaesold@gmx.at>) |
Responses |
Re: JDBC Support for standard_conforming_strings
|
List | pgsql-jdbc |
I'm still waiting for feedback. Kris? Dave? Best Regards Michael Paesold --------------------------------------------------------------------- Michael Paesold wrote: > 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;
pgsql-jdbc by date: