Changed behaviour of \'

Started by Martin Pittalmost 15 years ago8 messagesbugs
Jump to latest
#1Martin Pitt
mpitt@debian.org

Hello again,

sorry for spamming you today, but I promise that this is the last
mail; it's the one remaining test case failure for me.

I have some test cases to verify the handling of the obsolete \'
escaping in different locales (cf. CVE-2006-2313).

Up to 9.0, \' was still allowed in safe locales, but not in unsafe ones (sorry
for German error messages, but they just complain about unsafe usage of \'):

----------------------- 8< ----------------
$ printf "set client_encoding='SJIS'; select '\\\\'a'" | psql -Atq template1
FEHLER: unsichere Verwendung von \' in Zeichenkettenkonstante
LINE 1: select '\'a'
^
HINT: Verwenden Sie '', um Quotes in Zeichenketten zu schreiben. \' ist in bestimmten Client-seitigen Kodierungen unsicher.

$ printf "set client_encoding='UTF8'; select '\\\\'a'" | psql -Atq template1
WARNUNG: nicht standardkonforme Verwendung von \' in Zeichenkettenkonstante
LINE 1: select '\'a'
^
HINT: Verwenden Sie '', um Quotes in Zeichenketten zu schreiben, oder verwenden Sie die Syntax für Escape-Zeichenketten (E'...').
'a
----------------------- 8< ----------------

(Note the last line here, where it outputs 'a).

9.1 still rejects \' in SJIS, but it now also rejects \' in
UTF-8:

----------------------- 8< ----------------
$ printf "set client_encoding='SJIS'; select '\\\\'a'" | psql -Atq template1
ERROR: unterminated quoted string at or near "'"
LINE 1: select '\'a'

$ printf "set client_encoding='UTF8'; select '\\\\'a'" | psql -Atq template1
ERROR: unterminated quoted string at or near "'"
LINE 1: select '\'a'
----------------------- 8< ----------------

Since HISTORY does not mention this, is that an explicit decision to
finally deprecate the old \' syntax (which would be great, as it makes
this thing a lot more robust and deterministic, but it might be worth
mentioning it in HISTORY), or an unintended side effect?

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

In reply to: Martin Pitt (#1)
Re: Changed behaviour of \'

On Tue, May 10, 2011 at 06:20:23PM +0200, Martin Pitt wrote:

Since HISTORY does not mention this, is that an explicit decision to
finally deprecate the old \' syntax (which would be great, as it makes
this thing a lot more robust and deterministic, but it might be worth
mentioning it in HISTORY), or an unintended side effect?

release notes clearly mentions is:
http://developer.postgresql.org/pgdocs/postgres/release-9-1.html

- Change the default value of standard_conforming_strings to on (Robert Haas)
This removes a long-standing incompatibility with the SQL standard;
escape_string_warning has produced warnings about this usage for years. E''
strings are the proper way to embed escapes in strings and are unaffected by
this change.

Best regards,

depesz

--
The best thing about modern society is how easy it is to avoid contact with it.
http://depesz.com/

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: hubert depesz lubaczewski (#2)
Re: Changed behaviour of \'

hubert depesz lubaczewski <depesz@depesz.com> writes:

On Tue, May 10, 2011 at 06:20:23PM +0200, Martin Pitt wrote:

Since HISTORY does not mention this, is that an explicit decision to
finally deprecate the old \' syntax (which would be great, as it makes
this thing a lot more robust and deterministic, but it might be worth
mentioning it in HISTORY), or an unintended side effect?

release notes clearly mentions is:
http://developer.postgresql.org/pgdocs/postgres/release-9-1.html
- Change the default value of standard_conforming_strings to on (Robert Haas)
This removes a long-standing incompatibility with the SQL standard;
escape_string_warning has produced warnings about this usage for years. E''
strings are the proper way to embed escapes in strings and are unaffected by
this change.

Hmm ... considering that's the first thing in the release notes, I'm
surprised Martin missed it. Maybe he was looking for something
mentioning backslashes ... should we add a bit that specifically says
that backslashes are now no-ops by default?

regards, tom lane

#4Martin Pitt
mpitt@debian.org
In reply to: hubert depesz lubaczewski (#2)
Re: Changed behaviour of \'

hubert depesz lubaczewski [2011-05-10 18:37 +0200]:

release notes clearly mentions is:
http://developer.postgresql.org/pgdocs/postgres/release-9-1.html

- Change the default value of standard_conforming_strings to on (Robert Haas)

[...]

Ah, of course. Thanks!

(grep failure, sorry)

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: Changed behaviour of \'

Tom Lane wrote:

hubert depesz lubaczewski <depesz@depesz.com> writes:

On Tue, May 10, 2011 at 06:20:23PM +0200, Martin Pitt wrote:

Since HISTORY does not mention this, is that an explicit decision to
finally deprecate the old \' syntax (which would be great, as it makes
this thing a lot more robust and deterministic, but it might be worth
mentioning it in HISTORY), or an unintended side effect?

release notes clearly mentions is:
http://developer.postgresql.org/pgdocs/postgres/release-9-1.html
- Change the default value of standard_conforming_strings to on (Robert Haas)
This removes a long-standing incompatibility with the SQL standard;
escape_string_warning has produced warnings about this usage for years. E''
strings are the proper way to embed escapes in strings and are unaffected by
this change.

Hmm ... considering that's the first thing in the release notes, I'm
surprised Martin missed it. Maybe he was looking for something
mentioning backslashes ... should we add a bit that specifically says
that backslashes are now no-ops by default?

I added the word "backslash" before escapes in the attached applied
patch.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Attachments:

/rtmp/difftext/x-diffDownload+2-2
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: Changed behaviour of \'

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

Hmm ... considering that's the first thing in the release notes, I'm
surprised Martin missed it. Maybe he was looking for something
mentioning backslashes ... should we add a bit that specifically says
that backslashes are now no-ops by default?

I added the word "backslash" before escapes in the attached applied
patch.

Actually, I had something more like this in mind ...

commit ea964a451e51a32b71d004d261874adb1e135066
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue May 10 23:44:33 2011 -0400

Be more explicit about the meaning of the change in standard_conforming_strings.

diff --git a/doc/src/sgml/release-9.1.sgml b/doc/src/sgml/release-9.1.sgml
index 7737381..280e0bb 100644
--- a/doc/src/sgml/release-9.1.sgml
+++ b/doc/src/sgml/release-9.1.sgml
@@ -58,8 +58,9 @@
       </para>
       <para>
-       This removes a long-standing incompatibility with the SQL
-       standard;  <link
+       By default, backslashes are now ordinary characters in string literals,
+       not escape characters.  This change removes a long-standing
+       incompatibility with the SQL standard.  <link
        linkend="guc-escape-string-warning"><varname>escape_string_warning</></link>
        has produced warnings about this usage for years.  <literal>E''</>
        strings are the proper way to embed backslash escapes in strings and are

regards, tom lane

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: Changed behaviour of \'

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

Hmm ... considering that's the first thing in the release notes, I'm
surprised Martin missed it. Maybe he was looking for something
mentioning backslashes ... should we add a bit that specifically says
that backslashes are now no-ops by default?

I added the word "backslash" before escapes in the attached applied
patch.

Actually, I had something more like this in mind ...

Great.

---------------------------------------------------------------------------

commit ea964a451e51a32b71d004d261874adb1e135066
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue May 10 23:44:33 2011 -0400

Be more explicit about the meaning of the change in standard_conforming_strings.

diff --git a/doc/src/sgml/release-9.1.sgml b/doc/src/sgml/release-9.1.sgml
index 7737381..280e0bb 100644
--- a/doc/src/sgml/release-9.1.sgml
+++ b/doc/src/sgml/release-9.1.sgml
@@ -58,8 +58,9 @@
</para>
<para>
-       This removes a long-standing incompatibility with the SQL
-       standard;  <link
+       By default, backslashes are now ordinary characters in string literals,
+       not escape characters.  This change removes a long-standing
+       incompatibility with the SQL standard.  <link
linkend="guc-escape-string-warning"><varname>escape_string_warning</></link>
has produced warnings about this usage for years.  <literal>E''</>
strings are the proper way to embed backslash escapes in strings and are

regards, tom lane

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#8Magnus Hagander
magnus@hagander.net
In reply to: Bruce Momjian (#7)
Re: Changed behaviour of \'

On Wed, May 11, 2011 at 16:25, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

Hmm ... considering that's the first thing in the release notes, I'm
surprised Martin missed it.  Maybe he was looking for something
mentioning backslashes ... should we add a bit that specifically says
that backslashes are now no-ops by default?

I added the word "backslash" before escapes in the attached applied
patch.

Actually, I had something more like this in mind ...

Great.

I wonder if this is something that's going to hit so many people that
we need to be even more clear than that. Like specifically saying that
changing it back to "off" will make legacy applicatoins work again.
Given that *so* many apps use it... And people don't look at warnings
;)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/