Skip to content

Commit a692ee5

Browse files
hlinnakahlinnaka
committed
Replace SYSTEMQUOTEs with Windows-specific wrapper functions.
It's easy to forget using SYSTEMQUOTEs when constructing command strings for system() or popen(). Even if we fix all the places missing it now, it is bound to be forgotten again in the future. Introduce wrapper functions that do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the callers. We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and this doesn't change the behavior of those. But user-supplied commands, like archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as pgbench's \shell, will now gain an extra pair of quotes. That is desirable, but if you have existing scripts or config files that include an extra pair of quotes, those might need to be adjusted. Reviewed by Amit Kapila and Tom Lane
1 parent d69ffd6 commit a692ee5

File tree

18 files changed

+196
-93
lines changed

18 files changed

+196
-93
lines changed

‎configure.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,7 @@ if test "$PORTNAME" = "win32"; then
13531353
AC_REPLACE_FUNCS(gettimeofday)
13541354
AC_LIBOBJ(kill)
13551355
AC_LIBOBJ(open)
1356+
AC_LIBOBJ(system)
13561357
AC_LIBOBJ(win32env)
13571358
AC_LIBOBJ(win32error)
13581359
AC_LIBOBJ(win32setlocale)

‎contrib/pg_upgrade/check.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ get_bin_version(ClusterInfo *cluster)
970970
int pre_dot,
971971
post_dot;
972972

973-
snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" --version" SYSTEMQUOTE, cluster->bindir);
973+
snprintf(cmd, sizeof(cmd), "\"%s/pg_ctl\" --version", cluster->bindir);
974974

975975
if ((output = popen(cmd, "r")) == NULL ||
976976
fgets(cmd_output, sizeof(cmd_output), output) == NULL)

‎contrib/pg_upgrade/controldata.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
110110
pg_putenv("LC_ALL", NULL);
111111
pg_putenv("LC_MESSAGES", "C");
112112

113-
snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE,
113+
snprintf(cmd, sizeof(cmd), "\"%s/%s \"%s\"",
114114
cluster->bindir,
115115
live_check ? "pg_controldata\"" : "pg_resetxlog\" -n",
116116
cluster->pgdata);

‎contrib/pg_upgrade/exec.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ static DWORD mainThreadId = 0;
5959
mainThreadId = GetCurrentThreadId();
6060
#endif
6161

62-
written = strlcpy(cmd, SYSTEMQUOTE, sizeof(cmd));
62+
written = 0;
6363
va_start(ap, fmt);
6464
written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
6565
va_end(ap);
6666
if (written >= MAXCMDLEN)
6767
pg_fatal("command too long\n");
6868
written += snprintf(cmd + written, MAXCMDLEN - written,
69-
" >> \"%s\" 2>&1" SYSTEMQUOTE, log_file);
69+
" >> \"%s\" 2>&1", log_file);
7070
if (written >= MAXCMDLEN)
7171
pg_fatal("command too long\n");
7272

‎src/bin/initdb/initdb.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,11 +1130,11 @@ test_config_settings(void)
11301130
test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
11311131

11321132
snprintf(cmd, sizeof(cmd),
1133-
SYSTEMQUOTE "\"%s\" --boot -x0 %s "
1133+
"\"%s\" --boot -x0 %s "
11341134
"-c max_connections=%d "
11351135
"-c shared_buffers=%d "
11361136
"-c dynamic_shared_memory_type=none "
1137-
"< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
1137+
"< \"%s\" > \"%s\" 2>&1",
11381138
backend_exec, boot_options,
11391139
test_conns, test_buffs,
11401140
DEVNULL, DEVNULL);
@@ -1165,11 +1165,11 @@ test_config_settings(void)
11651165
}
11661166

11671167
snprintf(cmd, sizeof(cmd),
1168-
SYSTEMQUOTE "\"%s\" --boot -x0 %s "
1168+
"\"%s\" --boot -x0 %s "
11691169
"-c max_connections=%d "
11701170
"-c shared_buffers=%d "
11711171
"-c dynamic_shared_memory_type=none "
1172-
"< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
1172+
"< \"%s\" > \"%s\" 2>&1",
11731173
backend_exec, boot_options,
11741174
n_connections, test_buffs,
11751175
DEVNULL, DEVNULL);
@@ -1503,7 +1503,7 @@ bootstrap_template1(void)
15031503
unsetenv("PGCLIENTENCODING");
15041504

15051505
snprintf(cmd, sizeof(cmd),
1506-
SYSTEMQUOTE "\"%s\" --boot -x1 %s %s %s" SYSTEMQUOTE,
1506+
"\"%s\" --boot -x1 %s %s %s",
15071507
backend_exec,
15081508
data_checksums ? "-k" : "",
15091509
boot_options, talkargs);
@@ -1544,7 +1544,7 @@ setup_auth(void)
15441544
fflush(stdout);
15451545

15461546
snprintf(cmd, sizeof(cmd),
1547-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
1547+
"\"%s\" %s template1 >%s",
15481548
backend_exec, backend_options,
15491549
DEVNULL);
15501550

@@ -1622,7 +1622,7 @@ get_set_pwd(void)
16221622
fflush(stdout);
16231623

16241624
snprintf(cmd, sizeof(cmd),
1625-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
1625+
"\"%s\" %s template1 >%s",
16261626
backend_exec, backend_options,
16271627
DEVNULL);
16281628

@@ -1722,7 +1722,7 @@ setup_depend(void)
17221722
fflush(stdout);
17231723

17241724
snprintf(cmd, sizeof(cmd),
1725-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
1725+
"\"%s\" %s template1 >%s",
17261726
backend_exec, backend_options,
17271727
DEVNULL);
17281728

@@ -1755,7 +1755,7 @@ setup_sysviews(void)
17551755
* We use -j here to avoid backslashing stuff in system_views.sql
17561756
*/
17571757
snprintf(cmd, sizeof(cmd),
1758-
SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
1758+
"\"%s\" %s -j template1 >%s",
17591759
backend_exec, backend_options,
17601760
DEVNULL);
17611761

@@ -1786,7 +1786,7 @@ setup_description(void)
17861786
fflush(stdout);
17871787

17881788
snprintf(cmd, sizeof(cmd),
1789-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
1789+
"\"%s\" %s template1 >%s",
17901790
backend_exec, backend_options,
17911791
DEVNULL);
17921792

@@ -1893,7 +1893,7 @@ setup_collation(void)
18931893

18941894
#if defined(HAVE_LOCALE_T) && !defined(WIN32)
18951895
snprintf(cmd, sizeof(cmd),
1896-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
1896+
"\"%s\" %s template1 >%s",
18971897
backend_exec, backend_options,
18981898
DEVNULL);
18991899

@@ -2038,7 +2038,7 @@ setup_conversion(void)
20382038
fflush(stdout);
20392039

20402040
snprintf(cmd, sizeof(cmd),
2041-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2041+
"\"%s\" %s template1 >%s",
20422042
backend_exec, backend_options,
20432043
DEVNULL);
20442044

@@ -2076,7 +2076,7 @@ setup_dictionary(void)
20762076
* We use -j here to avoid backslashing stuff
20772077
*/
20782078
snprintf(cmd, sizeof(cmd),
2079-
SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
2079+
"\"%s\" %s -j template1 >%s",
20802080
backend_exec, backend_options,
20812081
DEVNULL);
20822082

@@ -2127,7 +2127,7 @@ setup_privileges(void)
21272127
fflush(stdout);
21282128

21292129
snprintf(cmd, sizeof(cmd),
2130-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2130+
"\"%s\" %s template1 >%s",
21312131
backend_exec, backend_options,
21322132
DEVNULL);
21332133

@@ -2190,7 +2190,7 @@ setup_schema(void)
21902190
* We use -j here to avoid backslashing stuff in information_schema.sql
21912191
*/
21922192
snprintf(cmd, sizeof(cmd),
2193-
SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
2193+
"\"%s\" %s -j template1 >%s",
21942194
backend_exec, backend_options,
21952195
DEVNULL);
21962196

@@ -2207,7 +2207,7 @@ setup_schema(void)
22072207
PG_CMD_CLOSE;
22082208

22092209
snprintf(cmd, sizeof(cmd),
2210-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2210+
"\"%s\" %s template1 >%s",
22112211
backend_exec, backend_options,
22122212
DEVNULL);
22132213

@@ -2241,7 +2241,7 @@ load_plpgsql(void)
22412241
fflush(stdout);
22422242

22432243
snprintf(cmd, sizeof(cmd),
2244-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2244+
"\"%s\" %s template1 >%s",
22452245
backend_exec, backend_options,
22462246
DEVNULL);
22472247

@@ -2266,7 +2266,7 @@ vacuum_db(void)
22662266
fflush(stdout);
22672267

22682268
snprintf(cmd, sizeof(cmd),
2269-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2269+
"\"%s\" %s template1 >%s",
22702270
backend_exec, backend_options,
22712271
DEVNULL);
22722272

@@ -2322,7 +2322,7 @@ make_template0(void)
23222322
fflush(stdout);
23232323

23242324
snprintf(cmd, sizeof(cmd),
2325-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2325+
"\"%s\" %s template1 >%s",
23262326
backend_exec, backend_options,
23272327
DEVNULL);
23282328

@@ -2354,7 +2354,7 @@ make_postgres(void)
23542354
fflush(stdout);
23552355

23562356
snprintf(cmd, sizeof(cmd),
2357-
SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
2357+
"\"%s\" %s template1 >%s",
23582358
backend_exec, backend_options,
23592359
DEVNULL);
23602360

‎src/bin/pg_ctl/pg_ctl.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -435,11 +435,11 @@ start_postmaster(void)
435435
* the PID without having to rely on reading it back from the pidfile.
436436
*/
437437
if (log_file != NULL)
438-
snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &" SYSTEMQUOTE,
438+
snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &",
439439
exec_path, pgdata_opt, post_opts,
440440
DEVNULL, log_file);
441441
else
442-
snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s < \"%s\" 2>&1 &" SYSTEMQUOTE,
442+
snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" 2>&1 &",
443443
exec_path, pgdata_opt, post_opts, DEVNULL);
444444

445445
return system(cmd);
@@ -453,10 +453,10 @@ start_postmaster(void)
453453
PROCESS_INFORMATION pi;
454454

455455
if (log_file != NULL)
456-
snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE,
456+
snprintf(cmd, MAXPGPATH, "CMD /C \"\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1\"",
457457
exec_path, pgdata_opt, post_opts, DEVNULL, log_file);
458458
else
459-
snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"%s\" %s%s < \"%s\" 2>&1" SYSTEMQUOTE,
459+
snprintf(cmd, MAXPGPATH, "CMD /C \"\"%s\" %s%s < \"%s\" 2>&1\"",
460460
exec_path, pgdata_opt, post_opts, DEVNULL);
461461

462462
if (!CreateRestrictedProcess(cmd, &pi, false))
@@ -814,10 +814,10 @@ do_init(void)
814814
post_opts = "";
815815

816816
if (!silent_mode)
817-
snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s" SYSTEMQUOTE,
817+
snprintf(cmd, MAXPGPATH, "\"%s\" %s%s",
818818
exec_path, pgdata_opt, post_opts);
819819
else
820-
snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s > \"%s\"" SYSTEMQUOTE,
820+
snprintf(cmd, MAXPGPATH, "\"%s\" %s%s > \"%s\"",
821821
exec_path, pgdata_opt, post_opts, DEVNULL);
822822

823823
if (system(cmd) != 0)
@@ -2035,7 +2035,7 @@ adjust_data_dir(void)
20352035
my_exec_path = pg_strdup(exec_path);
20362036

20372037
/* it's important for -C to be the first option, see main.c */
2038-
snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" -C data_directory %s%s" SYSTEMQUOTE,
2038+
snprintf(cmd, MAXPGPATH, "\"%s\" -C data_directory %s%s",
20392039
my_exec_path,
20402040
pgdata_opt ? pgdata_opt : "",
20412041
post_opts ? post_opts : "");

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,7 @@ runPgDump(const char *dbname)
16661666
PQExpBuffer cmd = createPQExpBuffer();
16671667
int ret;
16681668

1669-
appendPQExpBuffer(cmd, SYSTEMQUOTE "\"%s\" %s", pg_dump_bin,
1669+
appendPQExpBuffer(cmd, "\"%s\" %s", pg_dump_bin,
16701670
pgdumpopts->data);
16711671

16721672
/*
@@ -1687,8 +1687,6 @@ runPgDump(const char *dbname)
16871687

16881688
doShellQuoting(cmd, connstrbuf->data);
16891689

1690-
appendPQExpBufferStr(cmd, SYSTEMQUOTE);
1691-
16921690
if (verbose)
16931691
fprintf(stderr, _("%s: running \"%s\"\n"), progname, cmd->data);
16941692

‎src/bin/psql/command.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,10 +1936,10 @@ editFile(const char *fname, int lineno)
19361936
editorName, fname);
19371937
#else
19381938
if (lineno > 0)
1939-
sys = psprintf(SYSTEMQUOTE "\"%s\" %s%d \"%s\"" SYSTEMQUOTE,
1939+
sys = psprintf("\"%s\" %s%d \"%s\"",
19401940
editorName, editor_lineno_arg, lineno, fname);
19411941
else
1942-
sys = psprintf(SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE,
1942+
sys = psprintf("\"%s\" \"%s\"",
19431943
editorName, fname);
19441944
#endif
19451945
result = system(sys);
@@ -2643,7 +2643,7 @@ do_shell(const char *command)
26432643
#ifndef WIN32
26442644
sys = psprintf("exec %s", shellName);
26452645
#else
2646-
sys = psprintf(SYSTEMQUOTE "\"%s\"" SYSTEMQUOTE, shellName);
2646+
sys = psprintf("\"%s\"", shellName);
26472647
#endif
26482648
result = system(sys);
26492649
free(sys);

‎src/include/port.h

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -115,37 +115,6 @@ extern BOOL AddUserToTokenDacl(HANDLE hToken);
115115
#define DEVNULL "/dev/null"
116116
#endif
117117

118-
/*
119-
* Win32 needs double quotes at the beginning and end of system()
120-
* strings. If not, it gets confused with multiple quoted strings.
121-
* It also requires double-quotes around the executable name and
122-
* any files used for redirection. Other args can use single-quotes.
123-
*
124-
* Generated using Win32 "CMD /?":
125-
*
126-
* 1. If all of the following conditions are met, then quote characters
127-
* on the command line are preserved:
128-
*
129-
* - no /S switch
130-
* - exactly two quote characters
131-
* - no special characters between the two quote characters, where special
132-
* is one of: &<>()@^|
133-
* - there are one or more whitespace characters between the two quote
134-
* characters
135-
* - the string between the two quote characters is the name of an
136-
* executable file.
137-
*
138-
* 2. Otherwise, old behavior is to see if the first character is a quote
139-
* character and if so, strip the leading character and remove the last
140-
* quote character on the command line, preserving any text after the last
141-
* quote character.
142-
*/
143-
#if defined(WIN32) && !defined(__CYGWIN__)
144-
#define SYSTEMQUOTE "\""
145-
#else
146-
#define SYSTEMQUOTE ""
147-
#endif
148-
149118
/* Portable delay handling */
150119
extern void pg_usleep(long microsec);
151120

@@ -332,12 +301,16 @@ extern FILE *pgwin32_fopen(const char *, const char *);
332301
#define fopen(a,b) pgwin32_fopen(a,b)
333302
#endif
334303

335-
#ifndef popen
336-
#define popen(a,b) _popen(a,b)
337-
#endif
338-
#ifndef pclose
304+
/*
305+
* system() and popen() replacements to enclose the command in an extra
306+
* pair of quotes.
307+
*/
308+
extern int pgwin32_system(const char *command);
309+
extern FILE *pgwin32_popen(const char *command, const char *type);
310+
311+
#define system(a) pgwin32_system(a)
312+
#define popen(a,b) pgwin32_popen(a,b)
339313
#define pclose(a) _pclose(a)
340-
#endif
341314

342315
/* New versions of MingW have gettimeofday, old mingw and msvc don't */
343316
#ifndef HAVE_GETTIMEOFDAY

‎src/interfaces/ecpg/test/pg_regress_ecpg.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ ecpg_start_test(const char *testname,
137137
snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname);
138138

139139
snprintf(cmd, sizeof(cmd),
140-
SYSTEMQUOTE "\"%s\" >\"%s\" 2>\"%s\"" SYSTEMQUOTE,
140+
"\"%s\" >\"%s\" 2>\"%s\"",
141141
inprg,
142142
outfile_stdout,
143143
outfile_stderr);

‎src/interfaces/libpq/Makefile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
3838
OBJS += chklocale.o inet_net_ntop.o noblock.o pgstrcasecmp.o pqsignal.o \
3939
thread.o
4040
# libpgport C files that are needed if identified by configure
41-
OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
41+
OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
4242
# backend/libpq
4343
OBJS += ip.o md5.o
4444
# utils/mb
@@ -89,7 +89,7 @@ backend_src = $(top_srcdir)/src/backend
8989
# For some libpgport modules, this only happens if configure decides
9090
# the module is needed (see filter hack in OBJS, above).
9191

92-
chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
92+
chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
9393
rm -f $@ && $(LN_S) $< .
9494

9595
ip.c md5.c: % : $(backend_src)/libpq/%
@@ -150,7 +150,7 @@ clean distclean: clean-lib
150150
# Might be left over from a Win32 client-only build
151151
rm -f pg_config_paths.h
152152
rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
153-
rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
153+
rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
154154
rm -f pgsleep.c
155155
rm -f md5.c ip.c
156156
rm -f encnames.c wchar.c

0 commit comments

Comments
 (0)