>
> First, you can't just remove support for the escape syntax from \d
> commands without some discussion of whether or not that's the right
> thing to do, and I don't think it is. The cases where this will
> potentially cause a problem are limited to those where the input is
> invalidly encoded, and I don't think that's important enough to
> justify the surprise factor of having backslash commands behave
> differently from everything else.
>
> Second, even if it were OK to remove support for the escape syntax
> from \d commands, you failed to update the documentation you cribbed
> from my patch to match the behavior you implemented.
we can discus about programming style, but in this case I am sure. The
problem is \set command. We cannot ignore error in this case. In other
cases invalid escaping raises error, not in this case. So there is two
ways again:
a) remove escaped expansion from \command
b) implement \set command differently
>
> Third, you've reintroduced all of the code duplication that I
> eliminated in my version of this patch, as well as at least one bug -
> you've used free() where I believe you need PQfreemem().
you have a true.
I am also
> thinking that it doesn't make sense to push the result of
> PQescapeLiteral() or PQescapeIdentifier() as a new buffer, because we
> don't want to process variable expansions recursively. At first I
> thought this was a security hole, but on further reflection I don't
> think it is: it'll rescan as a quoted string anyway, so any
> colon-escapes will be ignored. But I believe it's unnecessary at any
> rate.
>
I think so it was a back door for scripting support in psql. It can
break backward compatibility!
> I would like to go ahead and commit my version of this patch and will
> do so later today if no one else objects.
yes, I have.
* your patch remove some feature without any warning and documentation
* your patch doesn't solve issue with \set command
Regards
Pavel
>
> ...Robert
>
*** ./doc/src/sgml/ref/psql-ref.sgml.orig 2009-12-25 00:36:39.000000000 +0100
--- ./doc/src/sgml/ref/psql-ref.sgml 2010-01-28 16:57:15.016331154 +0100
***************
*** 658,664 ****
<para>
If an unquoted argument begins with a colon (<literal>:</literal>),
it is taken as a <application>psql</> variable and the value of the
! variable is used as the argument instead.
</para>
<para>
--- 658,669 ----
<para>
If an unquoted argument begins with a colon (<literal>:</literal>),
it is taken as a <application>psql</> variable and the value of the
! variable is used as the argument instead. If the variable name is
! surrounded by single quotes (e.g. <literal>:'var'</literal>), it
! will be escaped as an SQL literal and the result will be used as
! the argument. If the variable name is surrounded by double quotes,
! it will be escaped as an SQL identifier and the result will be used
! as the argument.
</para>
<para>
***************
*** 2711,2728 ****
<para>
An additional useful feature of <application>psql</application>
variables is that you can substitute (<quote>interpolate</quote>)
! them into regular <acronym>SQL</acronym> statements. The syntax for
! this is again to prepend the variable name with a colon
(<literal>:</literal>):
<programlisting>
testdb=> <userinput>\set foo 'my_table'</userinput>
testdb=> <userinput>SELECT * FROM :foo;</userinput>
</programlisting>
! would then query the table <literal>my_table</literal>. The value of
! the variable is copied literally, so it can even contain unbalanced
! quotes or backslash commands. You must make sure that it makes sense
! where you put it. Variable interpolation will not be performed into
! quoted <acronym>SQL</acronym> entities.
</para>
<para>
--- 2716,2750 ----
<para>
An additional useful feature of <application>psql</application>
variables is that you can substitute (<quote>interpolate</quote>)
! them into regular <acronym>SQL</acronym> statements.
! <application>psql</application> provides special facilities for
! ensuring that values used as SQL literals and identifiers are
! properly escaped. The syntax for interpolating a value without
! any special escaping is again to prepend the variable name with a colon
(<literal>:</literal>):
<programlisting>
testdb=> <userinput>\set foo 'my_table'</userinput>
testdb=> <userinput>SELECT * FROM :foo;</userinput>
</programlisting>
! would then query the table <literal>my_table</literal>. Note that this
! may be unsafe: the value of the variable is copied literally, so it can
! even contain unbalanced quotes or backslash commands. You must make sure
! that it makes sense where you put it.
! </para>
!
! <para>
! When a value is to be used as an SQL literal or identifier, it is
! safest to arrange for it to be escaped. To escape the value of
! a variable as an SQL literal, write a colon followed by the variable
! name in single quotes. To escape the value an SQL identifier, write
! a colon followed by the variable name in double quotes. The previous
! example would be more safely written this way:
! <programlisting>
! testdb=> <userinput>\set foo 'my_table'</userinput>
! testdb=> <userinput>SELECT * FROM :"foo";</userinput>
! </programlisting>
! Variable interpolation will not be performed into quoted
! <acronym>SQL</acronym> entities.
</para>
<para>
***************
*** 2730,2769 ****
copy the contents of a file into a table column. First load the file into a
variable and then proceed as above:
<programlisting>
! testdb=> <userinput>\set content '''' `cat my_file.txt` ''''</userinput>
! testdb=> <userinput>INSERT INTO my_table VALUES (:content);</userinput>
! </programlisting>
! One problem with this approach is that <filename>my_file.txt</filename>
! might contain single quotes. These need to be escaped so that
! they don't cause a syntax error when the second line is processed. This
! could be done with the program <command>sed</command>:
! <programlisting>
! testdb=> <userinput>\set content '''' `sed -e "s/'/''/g" < my_file.txt` ''''</userinput>
! </programlisting>
! If you are using non-standard-conforming strings then you'll also need
! to double backslashes. This is a bit tricky:
! <programlisting>
! testdb=> <userinput>\set content '''' `sed -e "s/'/''/g" -e 's/\\/\\\\/g' < my_file.txt` ''''</userinput>
</programlisting>
! Note the use of different shell quoting conventions so that neither
! the single quote marks nor the backslashes are special to the shell.
! Backslashes are still special to <command>sed</command>, however, so
! we need to double them. (Perhaps
! at one point you thought it was great that all Unix commands use the
! same escape character.)
</para>
<para>
! Since colons can legally appear in SQL commands, the following rule
! applies: the character sequence
! <quote>:name</quote> is not changed unless <quote>name</> is the name
! of a variable that is currently set. In any case you can escape
! a colon with a backslash to protect it from substitution. (The
! colon syntax for variables is standard <acronym>SQL</acronym> for
embedded query languages, such as <application>ECPG</application>.
The colon syntax for array slices and type casts are
<productname>PostgreSQL</productname> extensions, hence the
! conflict.)
</para>
</refsect3>
--- 2752,2777 ----
copy the contents of a file into a table column. First load the file into a
variable and then proceed as above:
<programlisting>
! testdb=> <userinput>\set content `cat my_file.txt`</userinput>
! testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
</programlisting>
! (Note that this still won't work if my_file.txt contains NUL bytes.
! psql does not support embedded NUL bytes in variable values.)
</para>
<para>
! Since colons can legally appear in SQL commands, an apparent attempt
! at interpolation (such as <literal>:name</literal>,
! <literal>:'name'</literal>, or <literal>:"name"</literal>) is not
! changed unless the named variable is currently set. In any case you
! can escape a colon with a backslash to protect it from substitution.
! (The colon syntax for variables is standard <acronym>SQL</acronym> for
embedded query languages, such as <application>ECPG</application>.
The colon syntax for array slices and type casts are
<productname>PostgreSQL</productname> extensions, hence the
! conflict. The colon syntax for escaping a variable's value as an
! SQL literal or identifier is a <application>psql</application>
! extension.)
</para>
</refsect3>
*** ./src/bin/psql/psqlscan.l.orig 2010-01-02 17:57:59.000000000 +0100
--- ./src/bin/psql/psqlscan.l 2010-01-29 08:03:11.841025194 +0100
***************
*** 119,124 ****
--- 119,126 ----
static void emit(const char *txt, int len);
static bool is_utf16_surrogate_first(uint32 c);
+ static char *take_variable_name(const char *src, int len);
+
#define ECHO emit(yytext, yyleng)
%}
***************
*** 707,712 ****
--- 709,803 ----
}
}
+ :'[A-Za-z0-9_]+' {
+ /* Possible psql variable substitution */
+ const char *value;
+ char *varname = take_variable_name(yytext, yyleng);
+
+ value = GetVariable(pset.vars, varname);
+
+ if (value)
+ {
+ /* It is a variable, perform substitution */
+ char *escaped_value;
+
+ escaped_value = PQescapeLiteral(pset.db, value, strlen(value));
+ if (escaped_value != NULL)
+ {
+ push_new_buffer(escaped_value);
+ PQfreemem(escaped_value);
+ }
+ else
+ {
+ const char *error_message = PQerrorMessage(pset.db);
+
+ if (strlen(error_message))
+ psql_error("%s", error_message);
+
+ /*
+ * when we cannot copy escaped string, then copy
+ * the string as as
+ */
+ ECHO;
+ }
+ /* yy_scan_string already made buffer active */
+ }
+ else
+ {
+ /*
+ * if the variable doesn't exist we'll copy the
+ * string as is
+ */
+ ECHO;
+ }
+
+ free(varname);
+ }
+
+ :\"[A-Za-z0-9_]+\" {
+ /* Possible psql variable substitution */
+ const char *value;
+ char *varname = take_variable_name(yytext, yyleng);
+
+ value = GetVariable(pset.vars, varname);
+
+ if (value)
+ {
+ /* It is a variable, perform substitution */
+ char *escaped_value;
+
+ escaped_value = PQescapeIdentifier(pset.db, value, strlen(value));
+ if (escaped_value != NULL)
+ {
+ push_new_buffer(escaped_value);
+ PQfreemem(escaped_value);
+ }
+ else
+ {
+ const char *error_message = PQerrorMessage(pset.db);
+
+ if (strlen(error_message))
+ psql_error("%s", error_message);
+ /*
+ * when we cannot copy escaped string, then copy
+ * the string as as
+ */
+ ECHO;
+ }
+ /* yy_scan_string already made buffer active */
+ }
+ else
+ {
+ /*
+ * if the variable doesn't exist we'll copy the
+ * string as is
+ */
+ ECHO;
+ }
+
+ free(varname);
+ }
+
/*
* Back to backend-compatible rules.
*/
***************
*** 1740,1742 ****
--- 1831,1851 ----
{
return (c >= 0xD800 && c <= 0xDBFF);
}
+
+ /*
+ * remove colon and outer qoutes from text
+ *
+ */
+ static char *
+ take_variable_name(const char *src, int len)
+ {
+ char *result;
+
+ result = pg_malloc(len - 2);
+
+ /* skip first two chars (colon, quote) and stop before last char (quote) */
+ memcpy(result, src + 2, len - 3);
+ result[len - 3] = '\0';
+
+ return result;
+ }
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
