Hi Tom, On 2014-02-04 12:02:45 -0500, Tom Lane wrote: > Andres Freund <and...@2ndquadrant.com> writes: > > On 2014-02-04 11:36:22 -0500, Tom Lane wrote: > >> -1. This is not a general solution to the problem. There are other > >> GUCs for which people might want spaces in the value. > > > Sure, I didn't say it was. But I don't see any oother values that are > > likely being passed via PGOPTIONS that frequently contain spaces. > > application_name --- weren't we just reading about people passing entire > command lines there? (They must be using some other way of setting it > currently, but PGOPTIONS doesn't seem like an implausible source.)
You can't easily use PGOPTIONS to set application_name in many cases anyway, libpq's support for it gets in the way since it takes effect later. And I think libpq is much more likely way to set it. Also you can simply circumvent the problem by using a different naming convention, that's not problem with repeatable read. So I still think we should add read_committed, repeatable_read as aliases. > >> Yeah. See pg_split_opts(), which explicitly acknowledges that it'll fall > >> down for space-containing options. Not sure what the most appropriate > >> quoting convention would be there, but I'm sure we can think of something. > > > No argument against introducing it. What about simply allowing escaping > > of the next character using \? > > The same thought had occurred to me. Since it'll typically already be > inside some levels of quoting, any quoted-string convention seems like > it'd be a pain to use. But a straight backslash-escapes-the-next-char > thing wouldn't be too awful, I think. Ok, here's a patch implementing that. There's a slight backward concern in that a \ would earlier have been passed through unmodified, but now would be taken as a escape. I think that's not too much of a problem though. I thought about simply outputting the escape unless it's been used as an escape before a speace, but that seems like a bad idea, barring future uses to me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
>From 8baed020afeda57d2e292a7f37e3c9a97ceaf524 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Fri, 7 Feb 2014 10:59:23 +0100 Subject: [PATCH] Allow escaping of option values for options passed at connection start. This is primarily useful because it allows to set a per-connection default_transaction_isolation value of 'repeatable read' which previously was not possible, but other usecases like seach_path do also exist. --- src/backend/postmaster/postmaster.c | 3 +-- src/backend/utils/init/postinit.c | 51 +++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 5bc5213..7619fb5 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -4065,8 +4065,7 @@ BackendRun(Port *port) /* * Pass any backend switches specified with -o on the postmaster's own - * command line. We assume these are secure. (It's OK to mangle - * ExtraOptions now, since we're safely inside a subprocess.) + * command line. We assume these are secure. */ pg_split_opts(av, &ac, ExtraOptions); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 2a57ed3..6f25777 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -407,31 +407,55 @@ InitCommunication(void) /* * pg_split_opts -- split a string of options and append it to an argv array * - * NB: the input string is destructively modified! Also, caller is responsible - * for ensuring the argv array is large enough. The maximum possible number - * of arguments added by this routine is (strlen(optstr) + 1) / 2. + * The caller is responsible for ensuring the argv array is large enough. The + * maximum possible number of arguments added by this routine is + * (strlen(optstr) + 1) / 2. * - * Since no current POSTGRES arguments require any quoting characters, - * we can use the simple-minded tactic of assuming each set of space- - * delimited characters is a separate argv element. - * - * If you don't like that, well, we *used* to pass the whole option string - * as ONE argument to execl(), which was even less intelligent... + * Because some option values can contain spaces we allow escaping using + * backslashes, with a \\ representing a literal backslash. */ void pg_split_opts(char **argv, int *argcp, char *optstr) { + StringInfoData s; + + initStringInfo(&s); + while (*optstr) { + bool last_was_escape = false; + + resetStringInfo(&s); + + /* skip over leading space */ while (isspace((unsigned char) *optstr)) optstr++; + if (*optstr == '\0') break; - argv[(*argcp)++] = optstr; - while (*optstr && !isspace((unsigned char) *optstr)) + + /* + * Parse a single option + value, stopping at the first space, unless + * it's escaped. + */ + while (*optstr) + { + if (isspace(*optstr) && !last_was_escape) + break; + + if (!last_was_escape && *optstr == '\\') + last_was_escape = true; + else + { + last_was_escape = false; + appendStringInfoChar(&s, *optstr); + } + optstr++; - if (*optstr) - *optstr++ = '\0'; + } + + /* now store the option */ + argv[(*argcp)++] = pstrdup(s.data); } } @@ -979,7 +1003,6 @@ process_startup_options(Port *port, bool am_superuser) av[ac++] = "postgres"; - /* Note this mangles port->cmdline_options */ pg_split_opts(av, &ac, port->cmdline_options); av[ac] = NULL; -- 1.8.3.251.g1462b67
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers