Fix limitations on what SQL commands can be issued to a walsender.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 24 Jan 2022 20:33:34 +0000 (15:33 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 24 Jan 2022 20:33:38 +0000 (15:33 -0500)
In logical replication mode, a WalSender is supposed to be able
to execute any regular SQL command, as well as the special
replication commands.  Poor design of the replication-command
parser caused it to fail in various cases, notably:

* semicolons embedded in a command, or multiple SQL commands
sent in a single message;

* dollar-quoted literals containing odd numbers of single
or double quote marks;

* commands starting with a comment.

The basic problem here is that we're trying to run repl_scanner.l
across the entire input string even when it's not a replication
command.  Since repl_scanner.l does not understand all of the
token types known to the core lexer, this is doomed to have
failure modes.

We certainly don't want to make repl_scanner.l as big as scan.l,
so instead rejigger stuff so that we only lex the first token of
a non-replication command.  That will usually look like an IDENT
to repl_scanner.l, though a comment would end up getting reported
as a '-' or '/' single-character token.  If the token is a replication
command keyword, we push it back and proceed normally with repl_gram.y
parsing.  Otherwise, we can drop out of exec_replication_command()
without examining the rest of the string.

(It's still theoretically possible for repl_scanner.l to fail on
the first token; but that could only happen if it's an unterminated
single- or double-quoted string, in which case you'd have gotten
largely the same error from the core lexer too.)

In this way, repl_gram.y isn't involved at all in handling general
SQL commands, so we can get rid of the SQLCmd node type.  (In
the back branches, we can't remove it because renumbering enum
NodeTag would be an ABI break; so just leave it sit there unused.)

I failed to resist the temptation to clean up some other sloppy
coding in repl_scanner.l while at it.  The only externally-visible
behavior change from that is it now accepts \r and \f as whitespace,
same as the core lexer.

Per bug #17379 from Greg Rychlewski.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17379-6a5c6cfb3f1f5e77@postgresql.org

src/backend/replication/repl_gram.y
src/backend/replication/repl_scanner.l
src/backend/replication/walsender.c
src/include/nodes/nodes.h
src/include/nodes/replnodes.h
src/include/replication/walsender_private.h

index 6d1dbd72e911a898248ea0a78aec31f871724c87..3b92180e8cbf3f576a44f70029efd7e53ef88dee 100644 (file)
@@ -25,8 +25,6 @@
 /* Result of the parsing is returned here */
 Node *replication_parse_result;
 
-static SQLCmd *make_sqlcmd(void);
-
 
 /*
  * Bison doesn't allocate anything that needs to live across parser calls,
@@ -59,7 +57,6 @@ static SQLCmd *make_sqlcmd(void);
 %token <str> SCONST IDENT
 %token <uintval> UCONST
 %token <recptr> RECPTR
-%token T_WORD
 
 /* Keyword tokens. */
 %token K_BASE_BACKUP
@@ -95,7 +92,7 @@ static SQLCmd *make_sqlcmd(void);
 %type <node>   command
 %type <node>   base_backup start_replication start_logical_replication
                create_replication_slot drop_replication_slot identify_system
-               read_replication_slot timeline_history show sql_cmd
+               read_replication_slot timeline_history show
 %type <list>   base_backup_legacy_opt_list generic_option_list
 %type <defelt> base_backup_legacy_opt generic_option
 %type <uintval>    opt_timeline
@@ -129,7 +126,6 @@ command:
            | read_replication_slot
            | timeline_history
            | show
-           | sql_cmd
            ;
 
 /*
@@ -450,10 +446,6 @@ plugin_opt_arg:
            | /* EMPTY */                   { $$ = NULL; }
        ;
 
-sql_cmd:
-           IDENT                           { $$ = (Node *) make_sqlcmd(); }
-       ;
-
 generic_option_list:
            generic_option_list ',' generic_option
                { $$ = lappend($1, $3); }
@@ -514,20 +506,4 @@ ident_or_keyword:
 
 %%
 
-static SQLCmd *
-make_sqlcmd(void)
-{
-   SQLCmd *cmd = makeNode(SQLCmd);
-   int tok;
-
-   /* Just move lexer to the end of command. */
-   for (;;)
-   {
-       tok = yylex();
-       if (tok == ';' || tok == 0)
-           break;
-   }
-   return cmd;
-}
-
 #include "repl_scanner.c"
index 81ac41e9d451f71909fb9710b1fa7d1aac5ad944..d992bcc2e3c04dea755cb12239e2d4790f0c70cd 100644 (file)
@@ -31,6 +31,10 @@ fprintf_to_ereport(const char *fmt, const char *msg)
 /* Handle to the buffer that the lexer uses internally */
 static YY_BUFFER_STATE scanbufhandle;
 
+/* Pushed-back token (we only handle one) */
+static int repl_pushed_back_token;
+
+/* Work area for collecting literals */
 static StringInfoData litbuf;
 
 static void startlit(void);
@@ -51,7 +55,18 @@ static void addlitchar(unsigned char ychar);
 %option warn
 %option prefix="replication_yy"
 
-%x xq xd
+/*
+ * Exclusive states:
+ *  <xd> delimited identifiers (double-quoted identifiers)
+ *  <xq> standard single-quoted strings
+ */
+%x xd
+%x xq
+
+space          [ \t\n\r\f]
+
+quote          '
+quotestop      {quote}
 
 /* Extended quote
  * xqdouble implements embedded quote, ''''
@@ -69,11 +84,8 @@ xdstop           {dquote}
 xddouble       {dquote}{dquote}
 xdinside       [^"]+
 
-digit          [0-9]+
-hexdigit       [0-9A-Za-z]+
-
-quote          '
-quotestop      {quote}
+digit          [0-9]
+hexdigit       [0-9A-Fa-f]
 
 ident_start        [A-Za-z\200-\377_]
 ident_cont     [A-Za-z\200-\377_0-9\$]
@@ -82,6 +94,19 @@ identifier       {ident_start}{ident_cont}*
 
 %%
 
+%{
+   /* This code is inserted at the start of replication_yylex() */
+
+   /* If we have a pushed-back token, return that. */
+   if (repl_pushed_back_token)
+   {
+       int         result = repl_pushed_back_token;
+
+       repl_pushed_back_token = 0;
+       return result;
+   }
+%}
+
 BASE_BACKUP            { return K_BASE_BACKUP; }
 FAST           { return K_FAST; }
 IDENTIFY_SYSTEM        { return K_IDENTIFY_SYSTEM; }
@@ -112,14 +137,7 @@ WAIT               { return K_WAIT; }
 MANIFEST           { return K_MANIFEST; }
 MANIFEST_CHECKSUMS { return K_MANIFEST_CHECKSUMS; }
 
-","                { return ','; }
-";"                { return ';'; }
-"("                { return '('; }
-")"                { return ')'; }
-
-[\n]           ;
-[\t]           ;
-" "                ;
+{space}+       { /* do nothing */ }
 
 {digit}+       {
                    yylval.uintval = strtoul(yytext, NULL, 10);
@@ -181,6 +199,11 @@ MANIFEST_CHECKSUMS { return K_MANIFEST_CHECKSUMS; }
                    return IDENT;
                }
 
+.              {
+                   /* Any char not recognized above is returned as itself */
+                   return yytext[0];
+               }
+
 <xq,xd><<EOF>> { yyerror("unterminated quoted string"); }
 
 
@@ -188,9 +211,6 @@ MANIFEST_CHECKSUMS  { return K_MANIFEST_CHECKSUMS; }
                    yyterminate();
                }
 
-.              {
-                   return T_WORD;
-               }
 %%
 
 /* LCOV_EXCL_STOP */
@@ -250,6 +270,7 @@ replication_scanner_init(const char *str)
 
    /* Make sure we start in proper state */
    BEGIN(INITIAL);
+   repl_pushed_back_token = 0;
 }
 
 void
@@ -258,3 +279,35 @@ replication_scanner_finish(void)
    yy_delete_buffer(scanbufhandle);
    scanbufhandle = NULL;
 }
+
+/*
+ * Check to see if the first token of a command is a WalSender keyword.
+ *
+ * To keep repl_scanner.l minimal, we don't ask it to know every construct
+ * that the core lexer knows.  Therefore, we daren't lex more than the
+ * first token of a general SQL command.  That will usually look like an
+ * IDENT token here, although some other cases are possible.
+ */
+bool
+replication_scanner_is_replication_command(void)
+{
+   int         first_token = replication_yylex();
+
+   switch (first_token)
+   {
+       case K_IDENTIFY_SYSTEM:
+       case K_BASE_BACKUP:
+       case K_START_REPLICATION:
+       case K_CREATE_REPLICATION_SLOT:
+       case K_DROP_REPLICATION_SLOT:
+       case K_READ_REPLICATION_SLOT:
+       case K_TIMELINE_HISTORY:
+       case K_SHOW:
+           /* Yes; push back the first token so we can parse later. */
+           repl_pushed_back_token = first_token;
+           return true;
+       default:
+           /* Nope; we don't bother to push back the token. */
+           return false;
+   }
+}
index 4cf95ce4392c01d64b6321d65053a98a999e7a2c..655760fee3e608289fda82b09e51203c9a5e056b 100644 (file)
@@ -1630,7 +1630,8 @@ exec_replication_command(const char *cmd_string)
     */
    if (MyWalSnd->state == WALSNDSTATE_STOPPING)
        ereport(ERROR,
-               (errmsg("cannot execute new commands while WAL sender is in stopping mode")));
+               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                errmsg("cannot execute new commands while WAL sender is in stopping mode")));
 
    /*
     * CREATE_REPLICATION_SLOT ... LOGICAL exports a snapshot until the next
@@ -1641,7 +1642,7 @@ exec_replication_command(const char *cmd_string)
    CHECK_FOR_INTERRUPTS();
 
    /*
-    * Parse the command.
+    * Prepare to parse and execute the command.
     */
    cmd_context = AllocSetContextCreate(CurrentMemoryContext,
                                        "Replication command context",
@@ -1649,33 +1650,41 @@ exec_replication_command(const char *cmd_string)
    old_context = MemoryContextSwitchTo(cmd_context);
 
    replication_scanner_init(cmd_string);
-   parse_rc = replication_yyparse();
-   if (parse_rc != 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_SYNTAX_ERROR),
-                errmsg_internal("replication command parser returned %d",
-                                parse_rc)));
-   replication_scanner_finish();
-
-   cmd_node = replication_parse_result;
 
    /*
-    * If it's a SQL command, just clean up our mess and return false; the
-    * caller will take care of executing it.
+    * Is it a WalSender command?
     */
-   if (IsA(cmd_node, SQLCmd))
+   if (!replication_scanner_is_replication_command())
    {
-       if (MyDatabaseId == InvalidOid)
-           ereport(ERROR,
-                   (errmsg("cannot execute SQL commands in WAL sender for physical replication")));
+       /* Nope; clean up and get out. */
+       replication_scanner_finish();
 
        MemoryContextSwitchTo(old_context);
        MemoryContextDelete(cmd_context);
 
+       /* XXX this is a pretty random place to make this check */
+       if (MyDatabaseId == InvalidOid)
+           ereport(ERROR,
+                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                    errmsg("cannot execute SQL commands in WAL sender for physical replication")));
+
        /* Tell the caller that this wasn't a WalSender command. */
        return false;
    }
 
+   /*
+    * Looks like a WalSender command, so parse it.
+    */
+   parse_rc = replication_yyparse();
+   if (parse_rc != 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_SYNTAX_ERROR),
+                errmsg_internal("replication command parser returned %d",
+                                parse_rc)));
+   replication_scanner_finish();
+
+   cmd_node = replication_parse_result;
+
    /*
     * Report query to various monitoring facilities.  For this purpose, we
     * report replication commands just like SQL commands.
index f9ddafd3459458bafcc9e767e2c9c7c2f537e53e..da35f2c27228a1d07986dce8951209d9bf7a73fb 100644 (file)
@@ -501,7 +501,6 @@ typedef enum NodeTag
    T_ReadReplicationSlotCmd,
    T_StartReplicationCmd,
    T_TimeLineHistoryCmd,
-   T_SQLCmd,
 
    /*
     * TAGS FOR RANDOM OTHER STUFF
index a77234474009f8b5a5c8a47cf34d687528893fcb..8ae9c517ff210f1591da170578ecd5f69f77e450 100644 (file)
@@ -108,13 +108,4 @@ typedef struct TimeLineHistoryCmd
    TimeLineID  timeline;
 } TimeLineHistoryCmd;
 
-/* ----------------------
- *     SQL commands
- * ----------------------
- */
-typedef struct SQLCmd
-{
-   NodeTag     type;
-} SQLCmd;
-
 #endif                         /* REPLNODES_H */
index e62ef965f70ab562d20dd838111be0e7329fee50..9631047c6c1bdce3e3d0661fbbbaaf100f598166 100644 (file)
@@ -121,6 +121,7 @@ extern int  replication_yylex(void);
 extern void replication_yyerror(const char *str) pg_attribute_noreturn();
 extern void replication_scanner_init(const char *query_string);
 extern void replication_scanner_finish(void);
+extern bool replication_scanner_is_replication_command(void);
 
 extern Node *replication_parse_result;