Обсуждение: JDBC Support for standard_conforming_strings
Hi all, Regarding standard_conforming_strings, I am currently reading through the code and looking at all places that need to understand string escaping rules. I have found two issues where I need more advice: arrays (org.postgresql.jdbc2.AbstractJdbc2Array) bytea (org.postgresql.util.PGbytea) As far as I understand, quoting in arrays and bytea only changes as far as the types are passing through the backend parser as string-literals. Correct? Since the result of AbstractJdbc2Array.toString() and PGbytea.toPGString() will be fed to PreparedStatement.setString() in the end, we do not have to change the array or bytea code. Is this reasoning valid? Best Regards, Michael Paesold
On Sun, 5 Nov 2006, Michael Paesold wrote: > Regarding standard_conforming_strings, I am currently reading through the > code and looking at all places that need to understand string escaping rules. > > I have found two issues where I need more advice: > > arrays (org.postgresql.jdbc2.AbstractJdbc2Array) > bytea (org.postgresql.util.PGbytea) > > As far as I understand, quoting in arrays and bytea only changes as far as > the types are passing through the backend parser as string-literals. Correct? > > Since the result of AbstractJdbc2Array.toString() and PGbytea.toPGString() > will be fed to PreparedStatement.setString() in the end, we do not have to > change the array or bytea code. Is this reasoning valid? 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. Kris Jurka
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;
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;
On Sat, 11 Nov 2006, Michael Paesold wrote: > I'm still waiting for feedback. Kris? Dave? I'll hopefully be able to look at it tomorrow. Kris Jurka
Michael Paesold wrote:
>
> I am sending a work-in-progress patch now, as I have some more questions
> on how to proceed.
>
> Progress so far:
Looks good so far. So two typos in comments. Incremental diff attached.
> 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.
>
Doesn't really matter how you pass this around. The patch's usage of an
additional boolean parameter looks fine to me.
> 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.
I'm not super worried about this. If an administrator changes the value
in postgresql.conf and HUPs the postmaster they really can't expect
every client to pick this up immediately. Even when we are monitoring
the state (V3) we won't know it has changed until after we execute the
next query (with the wrong escaping setting). If an interface allows
the user to enter arbitrary SQL such that they can change this setting
then it's not really a security issue because they don't need to break
escaping to sneak a query in because they can already execute any query
they want.
> 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.
This is a good idea.
> 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.
>
I'd like to see a generic way of setting any GUC parameter at startup,
instead of just this one. The existing methods of setting this per user
or per database should be OK.
diff -rc origstandard/org/postgresql/core/Parser.java standard/org/postgresql/core/Parser.java
*** origstandard/org/postgresql/core/Parser.java Sun Nov 12 22:18:04 2006
--- standard/org/postgresql/core/Parser.java Sun Nov 12 15:52:19 2006
***************
*** 28,34 ****
boolean standardConformingStrings) {
if (standardConformingStrings)
{
! // don NOT treat backslashes as escape characters
while (++offset < query.length)
{
switch (query[offset])
--- 28,34 ----
boolean standardConformingStrings) {
if (standardConformingStrings)
{
! // do NOT treat backslashes as escape characters
while (++offset < query.length)
{
switch (query[offset])
diff -rc origstandard/org/postgresql/jdbc2/AbstractJdbc2Connection.java
standard/org/postgresql/jdbc2/AbstractJdbc2Connection.java
*** origstandard/org/postgresql/jdbc2/AbstractJdbc2Connection.java Sun Nov 12 22:18:04 2006
--- standard/org/postgresql/jdbc2/AbstractJdbc2Connection.java Sun Nov 12 16:07:29 2006
***************
*** 969,975 ****
{
// 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)
{
--- 969,975 ----
{
// With standard_conforming_string off, escape backslashes and
// single-quotes, but still escape single-quotes by doubling, to
! // avoid a security hazard if the reported value of
// standard_conforming_strings is incorrect.
for (int i = 0; i < str.length(); ++i)
{
hi
I am using Postgres 8.0.0 and we found this issue "ERROR: index "patient_pkey" is not a btree".
I have been informed that we should shift to Postgres 8.0.9
I discussed this with my team member and they are asking if we can upgrade to the latest Postgres version
i.e. 8.1.5
I have some questions regarding this:
1.will this vesion solve the problem that I have mentioned?
2. If we install postgres 8.1.5 instead of Postgres 8.0.0 I ll have to build my c++ application again right?
3. I am currently using postgresql-8.0-310.jdbc3.jar, for java applications. Would I have to change this jar as well? and if yes where can I find it?
thanks,
regards
Surabhi
Hi Kris, I was lacking time to work on this as I was quite busy at work as well as with my family, but I am going to finish this in the next days. Kris Jurka wrote: > Looks good so far. So two typos in comments. Incremental diff attached. Thanks, included in my local patch. > ... > Doesn't really matter how you pass this around. The patch's usage of an > additional boolean parameter looks fine to me. Ok. >> 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. > > I'm not super worried about this. If an administrator changes the value > in postgresql.conf and HUPs the postmaster they really can't expect > every client to pick this up immediately. Even when we are monitoring > the state (V3) we won't know it has changed until after we execute the > next query (with the wrong escaping setting). If an interface allows > the user to enter arbitrary SQL such that they can change this setting > then it's not really a security issue because they don't need to break > escaping to sneak a query in because they can already execute any query > they want. I mostly thought about cases where applications assume they can set standard_conforming_strings, or when administrators change the settings while an application server (with pooled connections) is running. We should at least document this issue. >> 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. > > This is a good idea. It think it's the safest approach for V2Query. I guess we must really prefix the string literal with " E". Otherwise cases like "BETWEEN?AND?" will break for string parameters. >> 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. >> > > I'd like to see a generic way of setting any GUC parameter at startup, > instead of just this one. The existing methods of setting this per user > or per database should be OK. Ok, I will leave that alone for now. Best Regards Michael Paesold
Another message that did not reach the list because of large patches. I've put the patches here: http://www.ejurka.com/pgsql/patches/standard-strings/ Kris Jurka Michael Paesold wrote: > Here is a new version of my patch (set) implementing support for > standard_conforming_strings. > > There are three patches now, split for easier review: > 01-standard-strings.patch: > the main patch (to be applied first) > 02-metadata-escapes.patch: > updates literal \\ escapes in AbstractJdbc2DatabaseMetaData > 03-testsuite-escapes.patch: > updates literal \\ escapes in the test-suite and expands the > test for string literals for server versions >= 8.2. > > With all three patches applied, the driver passes the test suite against > an 8.2 server with both standard_conforming_strings turned on or off, as > well as connecting using the V2 or V3 protocol. > > Some notes: > - The AbstractJdbc2DatabaseMetaData class made a lot of use of "x LIKE > 'pg\\\\_%'" constructs. With standard_conforming_strings turned off, > this must be written as 'pg\\_%'. Since many of the instances are in > static initialization context, there is no access to that setting, > though. So I changed all of those to "x ~ '^pg_'" (or similar) instead, > which works without escaping. Tom Lane recommended this usage pattern > for the information_schema on hackers, once. So I guess it should be OK > for the JDBC driver, too. > Nevertheless, the changes should be reviewed thoroughly, because some > code paths are only executed for older version of Postgres. > > - The query splitting code now also supports parsing the escaped string > syntax (E''). The parsing code in parseSql and modifyJdbcCall does not > yet. I thought this was of less importance. > Perhaps it could be done more easily by reusing or combining the main > parsing code (the code already has comments suggesting that). Since the > release of 8.2 is imminent, I would prefer to leave it as-is for now and > rather get testing on the more important parts. > > - The V2 query path now uses the E'' escape string syntax for string > constants as soon as a server version >= 8.2 is discovered. This > protects the driver from changes to standard_conforming_strings, which > it cannot detect using the V2 protocol. The escape string syntax is also > used to send bytea data to the server. > > I am currently not aware of any other issues. Comments are welcome, > testing is appreciated. > > Best Regards > Michael Paesold >
Kris Jurka wrote: > Another message that did not reach the list because of large patches. > I've put the patches here: > > http://www.ejurka.com/pgsql/patches/standard-strings/ > > Kris Jurka Thanks, should we send larger patches gzip-compressed? Michael Paesold
On Mon, 27 Nov 2006, Michael Paesold wrote: > Thanks, should we send larger patches gzip-compressed? > Works for me. The list has a 30k size limit so compression or posting elsewhere is necessary. Kris Jurka
> Michael Paesold wrote: >> Here is a new version of my patch (set) implementing support for >> standard_conforming_strings. >> Applied. Thanks! Kris Jurka