Allow psql variable substitution to occur in backtick command strings.

Previously, text between backquotes in a psql metacommand's arguments
was always passed to the shell literally.  That considerably hobbles
the usefulness of the feature for scripting, so we'd foreseen for a long
time that we'd someday want to allow substitution of psql variables into
the shell command.  IMO the addition of \if metacommands has brought us to
that point, since \if can greatly benefit from some sort of client-side
expression evaluation capability, and psql itself is not going to grow any
such thing in time for v10.  Hence, this patch.  It allows :VARIABLE to be
replaced by the exact contents of the named variable, while :'VARIABLE'
is replaced by the variable's contents suitably quoted to become a single
shell-command argument.  (The quoting rules for that are different from
those for SQL literals, so this is a bit of an abuse of the :'VARIABLE'
notation, but I doubt anyone will be confused.)

As with other situations in psql, no substitution occurs if the word
following a colon is not a known variable name.  That limits the risk of
compatibility problems for existing psql scripts; but the risk isn't zero,
so this needs to be called out in the v10 release notes.

Discussion: https://postgr.es/m/9561.1490895211@sss.pgh.pa.us

doc/src/sgml/ref/psql-ref.sgml

@@ -769,18 +769,33 @@ testdb=>
     quotes that single character, whatever it is.
     </para>
 
-    <para>
-    Within an argument, text that is enclosed in backquotes
-    (<literal>`</literal>) is taken as a command line that is passed to the
-    shell. The output of the command (with any trailing newline removed)
-    replaces the backquoted text.
-    </para>
-
     <para>
     If an unquoted colon (<literal>:</literal>) followed by a
     <application>psql</> variable name appears within an argument, it is
     replaced by the variable's value, as described in <xref
     linkend="APP-PSQL-interpolation" endterm="APP-PSQL-interpolation-title">.
+    The forms <literal>:'<replaceable>variable_name</>'</literal> and
+    <literal>:"<replaceable>variable_name</>"</literal> described there
+    work as well.
+    </para>
+
+    <para>
+    Within an argument, text that is enclosed in backquotes
+    (<literal>`</literal>) is taken as a command line that is passed to the
+    shell.  The output of the command (with any trailing newline removed)
+    replaces the backquoted text.  Within the text enclosed in backquotes,
+    no special quoting or other processing occurs, except that appearances
+    of <literal>:<replaceable>variable_name</></literal> where
+    <replaceable>variable_name</> is a <application>psql</> variable name
+    are replaced by the variable's value.  Also, appearances of
+    <literal>:'<replaceable>variable_name</>'</literal> are replaced by the
+    variable's value suitably quoted to become a single shell command
+    argument.  (The latter form is almost always preferable, unless you are
+    very sure of what is in the variable.)  Because carriage return and line
+    feed characters cannot be safely quoted on all platforms, the
+    <literal>:'<replaceable>variable_name</>'</literal> form prints an
+    error message and does not substitute the variable value when such
+    characters appear in the value.
     </para>
 
     <para>

src/bin/psql/common.c

@@ -116,19 +116,19 @@ setQFout(const char *fname)
  * If the specified variable exists, return its value as a string (malloc'd
  * and expected to be freed by the caller); else return NULL.
  *
- * If "escape" is true, return the value suitably quoted and escaped,
+ * If "quote" isn't PQUOTE_PLAIN, then return the value suitably quoted and
- * as an identifier or string literal depending on "as_ident".
+ * escaped for the specified quoting requirement.  (Failure in escaping
- * (Failure in escaping should lead to returning NULL.)
+ * should lead to printing an error and returning NULL.)
  *
  * "passthrough" is the pointer previously given to psql_scan_set_passthrough.
  * In psql, passthrough points to a ConditionalStack, which we check to
  * determine whether variable expansion is allowed.
  */
 char *
-psql_get_variable(const char *varname, bool escape, bool as_ident,
+psql_get_variable(const char *varname, PsqlScanQuoteType quote,
 				  void *passthrough)
 {
-	char	   *result;
+	char	   *result = NULL;
 	const char *value;
 
 	/* In an inactive \if branch, suppress all variable substitutions */
@@ -139,40 +139,74 @@ psql_get_variable(const char *varname, bool escape, bool as_ident,
 	if (!value)
 		return NULL;
 
-	if (escape)
+	switch (quote)
 	{
-		char	   *escaped_value;
+		case PQUOTE_PLAIN:
+			result = pg_strdup(value);
+			break;
+		case PQUOTE_SQL_LITERAL:
+		case PQUOTE_SQL_IDENT:
+			{
+				/*
+				 * For these cases, we use libpq's quoting functions, which
+				 * assume the string is in the connection's client encoding.
+				 */
+				char	   *escaped_value;
 
-		if (!pset.db)
+				if (!pset.db)
-		{
+				{
-			psql_error("cannot escape without active connection\n");
+					psql_error("cannot escape without active connection\n");
-			return NULL;
+					return NULL;
-		}
+				}
 
-		if (as_ident)
+				if (quote == PQUOTE_SQL_LITERAL)
-			escaped_value =
+					escaped_value =
-				PQescapeIdentifier(pset.db, value, strlen(value));
+						PQescapeLiteral(pset.db, value, strlen(value));
-		else
+				else
-			escaped_value =
+					escaped_value =
-				PQescapeLiteral(pset.db, value, strlen(value));
+						PQescapeIdentifier(pset.db, value, strlen(value));
 
-		if (escaped_value == NULL)
+				if (escaped_value == NULL)
-		{
+				{
-			const char *error = PQerrorMessage(pset.db);
+					const char *error = PQerrorMessage(pset.db);
 
-			psql_error("%s", error);
+					psql_error("%s", error);
-			return NULL;
+					return NULL;
-		}
+				}
 
-		/*
+				/*
-		 * Rather than complicate the lexer's API with a notion of which
+				 * Rather than complicate the lexer's API with a notion of
-		 * free() routine to use, just pay the price of an extra strdup().
+				 * which free() routine to use, just pay the price of an extra
-		 */
+				 * strdup().
-		result = pg_strdup(escaped_value);
+				 */
-		PQfreemem(escaped_value);
+				result = pg_strdup(escaped_value);
+				PQfreemem(escaped_value);
+				break;
+			}
+		case PQUOTE_SHELL_ARG:
+			{
+				/*
+				 * For this we use appendShellStringNoError, which is
+				 * encoding-agnostic, which is fine since the shell probably
+				 * is too.  In any case, the only special character is "'",
+				 * which is not known to appear in valid multibyte characters.
+				 */
+				PQExpBufferData buf;
+
+				initPQExpBuffer(&buf);
+				if (!appendShellStringNoError(&buf, value))
+				{
+					psql_error("shell command argument contains a newline or carriage return: \"%s\"\n",
+							   value);
+					free(buf.data);
+					return NULL;
+				}
+				result = buf.data;
+				break;
+			}
+
+			/* No default: we want a compiler warning for missing cases */
 	}
-	else
-		result = pg_strdup(value);
 
 	return result;
 }

src/bin/psql/common.h

@@ -12,11 +12,12 @@
 
 #include "libpq-fe.h"
 #include "fe_utils/print.h"
+#include "fe_utils/psqlscan.h"
 
 extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
 extern bool setQFout(const char *fname);
 
-extern char *psql_get_variable(const char *varname, bool escape, bool as_ident,
+extern char *psql_get_variable(const char *varname, PsqlScanQuoteType quote,
 				  void *passthrough);
 
 extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);

src/bin/psql/psqlscanslash.l

@@ -242,8 +242,7 @@ other			.
 															 yytext + 1,
 															 yyleng - 1);
 						value = cur_state->callbacks->get_variable(varname,
-																   false,
+																   PQUOTE_PLAIN,
-																   false,
 																   cur_state->cb_passthrough);
 						free(varname);
 
@@ -268,14 +267,16 @@ other			.
 				}
 
 :'{variable_char}+'	{
-					psqlscan_escape_variable(cur_state, yytext, yyleng, false);
+					psqlscan_escape_variable(cur_state, yytext, yyleng,
+											 PQUOTE_SQL_LITERAL);
 					*option_quote = ':';
 					unquoted_option_chars = 0;
 				}
 
 
 :\"{variable_char}+\"	{
-					psqlscan_escape_variable(cur_state, yytext, yyleng, true);
+					psqlscan_escape_variable(cur_state, yytext, yyleng,
+											 PQUOTE_SQL_IDENT);
 					*option_quote = ':';
 					unquoted_option_chars = 0;
 				}
@@ -337,9 +338,8 @@ other			.
 
 <xslashbackquote>{
 	/*
-	 * backticked text: copy everything until next backquote, then evaluate.
+	 * backticked text: copy everything until next backquote (expanding
-	 *
+	 * variable references, but doing nought else), then evaluate.
-	 * XXX Possible future behavioral change: substitute for :VARIABLE?
 	 */
 
 "`"				{
@@ -350,6 +350,44 @@ other			.
 					BEGIN(xslasharg);
 				}
 
+:{variable_char}+	{
+					/* Possible psql variable substitution */
+					if (cur_state->callbacks->get_variable == NULL)
+						ECHO;
+					else
+					{
+						char	   *varname;
+						char	   *value;
+
+						varname = psqlscan_extract_substring(cur_state,
+															 yytext + 1,
+															 yyleng - 1);
+						value = cur_state->callbacks->get_variable(varname,
+																   PQUOTE_PLAIN,
+																   cur_state->cb_passthrough);
+						free(varname);
+
+						if (value)
+						{
+							appendPQExpBufferStr(output_buf, value);
+							free(value);
+						}
+						else
+							ECHO;
+					}
+				}
+
+:'{variable_char}+'	{
+					psqlscan_escape_variable(cur_state, yytext, yyleng,
+											 PQUOTE_SHELL_ARG);
+				}
+
+:'{variable_char}*	{
+					/* Throw back everything but the colon */
+					yyless(1);
+					ECHO;
+				}
+
 {other}|\n		{ ECHO; }
 
 }

src/fe_utils/psqlscan.l

@@ -699,8 +699,7 @@ other			.
 														 yyleng - 1);
 					if (cur_state->callbacks->get_variable)
 						value = cur_state->callbacks->get_variable(varname,
-																   false,
+																   PQUOTE_PLAIN,
-																   false,
 																   cur_state->cb_passthrough);
 					else
 						value = NULL;
@@ -737,11 +736,13 @@ other			.
 				}
 
 :'{variable_char}+'	{
-					psqlscan_escape_variable(cur_state, yytext, yyleng, false);
+					psqlscan_escape_variable(cur_state, yytext, yyleng,
+											 PQUOTE_SQL_LITERAL);
 				}
 
 :\"{variable_char}+\"	{
-					psqlscan_escape_variable(cur_state, yytext, yyleng, true);
+					psqlscan_escape_variable(cur_state, yytext, yyleng,
+											 PQUOTE_SQL_IDENT);
 				}
 
 	/*
@@ -1415,7 +1416,7 @@ psqlscan_extract_substring(PsqlScanState state, const char *txt, int len)
  */
 void
 psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
-						 bool as_ident)
+						 PsqlScanQuoteType quote)
 {
 	char	   *varname;
 	char	   *value;
@@ -1423,7 +1424,7 @@ psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
 	/* Variable lookup. */
 	varname = psqlscan_extract_substring(state, txt + 2, len - 3);
 	if (state->callbacks->get_variable)
-		value = state->callbacks->get_variable(varname, true, as_ident,
+		value = state->callbacks->get_variable(varname, quote,
 											   state->cb_passthrough);
 	else
 		value = NULL;

src/fe_utils/string_utils.c

@@ -425,13 +425,30 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length,
  * arguments containing LF or CR characters.  A future major release should
  * reject those characters in CREATE ROLE and CREATE DATABASE, because use
  * there eventually leads to errors here.
+ *
+ * appendShellString() simply prints an error and dies if LF or CR appears.
+ * appendShellStringNoError() omits those characters from the result, and
+ * returns false if there were any.
  */
 void
 appendShellString(PQExpBuffer buf, const char *str)
+{
+	if (!appendShellStringNoError(buf, str))
+	{
+		fprintf(stderr,
+				_("shell command argument contains a newline or carriage return: \"%s\"\n"),
+				str);
+		exit(EXIT_FAILURE);
+	}
+}
+
+bool
+appendShellStringNoError(PQExpBuffer buf, const char *str)
 {
 #ifdef WIN32
 	int			backslash_run_length = 0;
 #endif
+	bool		ok = true;
 	const char *p;
 
 	/*
@@ -442,7 +459,7 @@ appendShellString(PQExpBuffer buf, const char *str)
 		strspn(str, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_./:") == strlen(str))
 	{
 		appendPQExpBufferStr(buf, str);
-		return;
+		return ok;
 	}
 
 #ifndef WIN32
@@ -451,10 +468,8 @@ appendShellString(PQExpBuffer buf, const char *str)
 	{
 		if (*p == '\n' || *p == '\r')
 		{
-			fprintf(stderr,
+			ok = false;
-					_("shell command argument contains a newline or carriage return: \"%s\"\n"),
+			continue;
-					str);
-			exit(EXIT_FAILURE);
 		}
 
 		if (*p == '\'')
@@ -481,10 +496,8 @@ appendShellString(PQExpBuffer buf, const char *str)
 	{
 		if (*p == '\n' || *p == '\r')
 		{
-			fprintf(stderr,
+			ok = false;
-					_("shell command argument contains a newline or carriage return: \"%s\"\n"),
+			continue;
-					str);
-			exit(EXIT_FAILURE);
 		}
 
 		/* Change N backslashes before a double quote to 2N+1 backslashes. */
@@ -524,6 +537,8 @@ appendShellString(PQExpBuffer buf, const char *str)
 	}
 	appendPQExpBufferStr(buf, "^\"");
 #endif   /* WIN32 */
+
+	return ok;
 }
 
 

src/include/fe_utils/psqlscan.h

@@ -48,13 +48,22 @@ typedef enum _promptStatus
 	PROMPT_COPY
 } promptStatus_t;
 
+/* Quoting request types for get_variable() callback */
+typedef enum
+{
+	PQUOTE_PLAIN,				/* just return the actual value */
+	PQUOTE_SQL_LITERAL,			/* add quotes to make a valid SQL literal */
+	PQUOTE_SQL_IDENT,			/* quote if needed to make a SQL identifier */
+	PQUOTE_SHELL_ARG			/* quote if needed to be safe in a shell cmd */
+} PsqlScanQuoteType;
+
 /* Callback functions to be used by the lexer */
 typedef struct PsqlScanCallbacks
 {
-	/* Fetch value of a variable, as a pfree'able string; NULL if unknown */
+	/* Fetch value of a variable, as a free'able string; NULL if unknown */
 	/* This pointer can be NULL if no variable substitution is wanted */
-	char	   *(*get_variable) (const char *varname, bool escape,
+	char	   *(*get_variable) (const char *varname, PsqlScanQuoteType quote,
-										   bool as_ident, void *passthrough);
+											 void *passthrough);
 	/* Print an error message someplace appropriate */
 	/* (very old gcc versions don't support attributes on function pointers) */
 #if defined(__GNUC__) && __GNUC__ < 4

src/include/fe_utils/psqlscan_int.h

@@ -141,6 +141,6 @@ extern char *psqlscan_extract_substring(PsqlScanState state,
 						   const char *txt, int len);
 extern void psqlscan_escape_variable(PsqlScanState state,
 						 const char *txt, int len,
-						 bool as_ident);
+						 PsqlScanQuoteType quote);
 
 #endif   /* PSQLSCAN_INT_H */

src/include/fe_utils/string_utils.h

@@ -42,6 +42,7 @@ extern void appendByteaLiteral(PQExpBuffer buf,
 				   bool std_strings);
 
 extern void appendShellString(PQExpBuffer buf, const char *str);
+extern bool appendShellStringNoError(PQExpBuffer buf, const char *str);
 extern void appendConnStrVal(PQExpBuffer buf, const char *str);
 extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname);