At Fri, 31 Jul 2020 09:42:42 +0800 (CST), ZHAOWANCHENG <zhaowch...@163.com>
wrote in
> At 2014-01-28 21:11:54, "Bruce Momjian" <br...@momjian.us> wrote:
> >On Mon, Jul 1, 2013 at 08:10:14PM -0400, Josh Kupershmidt wrote:
> >> On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao <masao.fu...@gmail.com>
> >> wrote:
> >> > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmi...@gmail.com>
> >> > wrote:
> >> >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fu...@gmail.com>
> >> >> wrote:
> >> >>> Though this is a corner case, the patch doesn't seem to handle
> >> >>> properly the case
> >> >>> where "-D" appears as other option value, e.g., -k option value, in
> >> >>> postmaster.opts
> >> >>> file.
> >> >>
> >> >> Could I see a command-line example of what you mean?
> >> >
> >> > postmaster -k "-D", for example. Of course, it's really a corner case :)
> >>
> >> Oh, I see. I was able to trip up strip_datadirs() with something like
> >>
> >> $ PGDATA="/my/data/" postmaster -k "-D" -S 100 &
> >> $ pg_ctl -D /my/data/ restart
> >>
> >> that example causes pg_ctl to fail to start the server after stopping
> >> it, although perhaps you could even trick the server into starting
> >> with the wrong options. Of course, similar problems exists today in
> >> other cases, such as with the relative paths issue this patch is
> >> trying to address, or a datadir containing embedded quotes.
> >>
> >> I am eager to see the relative paths issue fixed, but maybe we need to
> >> bite the bullet and sort out the escaping of command-line options in
> >> the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \"
> >> quote" can consistently be used by pg_ctl {start|stop|restart} before
> >> we can fix this wart.
> >
> >Where are we on this patch?
> >
> >--
> > Bruce Momjian <br...@momjian.us> http://momjian.us
> > EnterpriseDB http://enterprisedb.com
> >
> > + Everyone has their own god. +
> >
> >
> >--
> >Sent via pgsql-hackers mailing list (pgsql-hack...@postgresql.org)
> >To make changes to your subscription:
> >http://www.postgresql.org/mailpref/pgsql-hackers
>
> >
>
>
> Hi, I encountered the same problem.
> I want to know is there a final conclusion?
> thank you very much!
It seems to me agrouding on parsing issue. We haven't find a way to
parse the content of postmaster.opt properly.
1. For escaped option arguments, we can't find where directory name ends.
"-D" "/tmp/here's a \" quote"
2. We need to distinguish option names and arguments.
"-k" "-D" # "-D" is an arguemnt, not a option name.
3. This is not mentioned here, but getopt accepts "merged" (I'm not
sure what to call it.) short options.
"-iD" "/hoge" # equivalent to "-i" "-D" "hoge"
We need to either let pg_ctl reparse the commandline the same way with
postmaster or let postmaster normalize and/or markup the content of
postmaster.opts so that pg_ctl can read it desired way. That can be as
attached, but the change seems a bit too big..
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5b5fc97c72..0650cc10e8 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -560,6 +560,45 @@ int postmaster_alive_fds[2] = {-1, -1};
HANDLE PostmasterHandle;
#endif
+char *norm_args = NULL; /* normalized arguments */
+char *norm_args_tail = NULL;
+int norm_args_len = 0;
+
+static void
+add_norm_argument(int opt, char *value)
+{
+ int valuelen = 0;
+
+ if (norm_args_len == 0)
+ {
+ norm_args_len = 128;
+ norm_args = malloc(norm_args_len);
+ norm_args_tail = norm_args;
+ }
+
+ if (opt == 0)
+ {
+ *norm_args_tail++ = '\0'; /* terminator */
+ return;
+ }
+
+ if (value)
+ valuelen = strlen(value) + 3; /* _\"val\"*/
+
+ /* expand buffer as needed */
+ while (norm_args_tail - norm_args + 4 /* \"-x\" */ + valuelen + 1
+ > norm_args_len)
+ norm_args_len *= 2;
+ norm_args = realloc(norm_args, norm_args_len);
+
+ *norm_args_tail++ = '\1'; /* delimiter */
+
+ if (value != NULL)
+ norm_args_tail += sprintf(norm_args_tail, "\"-%c\" \"%s\"", opt, value);
+ else
+ norm_args_tail += sprintf(norm_args_tail, "\"-%c\"", opt);
+}
+
/*
* Postmaster main entry point
*/
@@ -680,6 +719,8 @@ PostmasterMain(int argc, char *argv[])
*/
while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
{
+ add_norm_argument(opt, optarg);
+
switch (opt)
{
case 'B':
@@ -850,6 +891,9 @@ PostmasterMain(int argc, char *argv[])
}
}
+ /* terminate normalized arguemnt list */
+ add_norm_argument(0, NULL);
+
/*
* Postmaster accepts no non-option switch arguments.
*/
@@ -5666,7 +5710,6 @@ static bool
CreateOptsFile(int argc, char *argv[], char *fullprogname)
{
FILE *fp;
- int i;
#define OPTS_FILE "postmaster.opts"
@@ -5677,8 +5720,8 @@ CreateOptsFile(int argc, char *argv[], char *fullprogname)
}
fprintf(fp, "%s", fullprogname);
- for (i = 1; i < argc; i++)
- fprintf(fp, " \"%s\"", argv[i]);
+ if (norm_args)
+ fprintf(fp, "%s", norm_args);
fputs("\n", fp);
if (fclose(fp))
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 1cdc3ebaa3..b4ccaf224f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -755,10 +755,37 @@ read_post_opts(void)
* Are we at the first option, as defined by space and
* double-quote?
*/
- if ((arg1 = strstr(optline, " \"")) != NULL)
+ if ((arg1 = strstr(optline, "\1\"")) != NULL)
{
+ char *pto;
+ char *pfrom;
+
*arg1 = '\0'; /* terminate so we get only program name */
post_opts = pg_strdup(arg1 + 1); /* point past whitespace */
+
+ pto = pfrom = post_opts;
+ while (*pfrom)
+ {
+ if (*pfrom != '\1')
+ {
+ *pto++ = *pfrom++;
+ continue;
+ }
+
+ pfrom++;
+
+ /* Remove -D optsion if we have a replacment */
+ if (pgdata_opt && strncmp(pfrom, "\"-D\"", 4) == 0)
+ {
+ /* Skip -D option */
+ while (*pfrom && *pfrom != '\1') pfrom++;
+ continue;
+ }
+
+ /* replace '\1' with a space */
+ *pto++ = ' ';
+ }
+ *pto = 0;
}
if (exec_path == NULL)
exec_path = pg_strdup(optline);
@@ -870,8 +897,8 @@ do_start(void)
read_post_opts();
- /* No -D or -D already added during server start */
- if (ctl_command == RESTART_COMMAND || pgdata_opt == NULL)
+ /* Use "" for printf safety */
+ if (pgdata_opt == NULL)
pgdata_opt = "";
if (exec_path == NULL)
