diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 99828e5879d..ac3eb48cd6e 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -64,9 +64,15 @@ GetConnection(void) PQconninfoOption *conn_opt; char *err_msg = NULL; + /* pg_recvlogical uses dbname only; others use connection_string only. */ + Assert(dbname == NULL || connection_string == NULL); + /* * Merge the connection info inputs given in form of connection string, * options and default values (dbname=replication, replication=true, etc.) + * Explicitly discard any dbname value in the connection string; + * otherwise, PQconnectdbParams() would interpret that value as being + * itself a connection string. */ i = 0; if (connection_string) @@ -80,7 +86,8 @@ GetConnection(void) for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) { - if (conn_opt->val != NULL && conn_opt->val[0] != '\0') + if (conn_opt->val != NULL && conn_opt->val[0] != '\0' && + strcmp(conn_opt->keyword, "dbname") != 0) argcount++; } @@ -89,7 +96,8 @@ GetConnection(void) for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) { - if (conn_opt->val != NULL && conn_opt->val[0] != '\0') + if (conn_opt->val != NULL && conn_opt->val[0] != '\0' && + strcmp(conn_opt->keyword, "dbname") != 0) { keywords[i] = conn_opt->keyword; values[i] = conn_opt->val; diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index d7506e119e2..8d06e6a5931 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -339,6 +339,210 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix) } +/* + * Append the given string to the shell command being built in the buffer, + * with suitable shell-style quoting to create exactly one argument. + * + * Forbid LF or CR characters, which have scant practical use beyond designing + * security breaches. The Windows command shell is unusable as a conduit for + * 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. + */ +void +appendShellString(PQExpBuffer buf, const char *str) +{ + const char *p; + +#ifndef WIN32 + appendPQExpBufferChar(buf, '\''); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + if (*p == '\'') + appendPQExpBufferStr(buf, "'\"'\"'"); + else + appendPQExpBufferChar(buf, *p); + } + appendPQExpBufferChar(buf, '\''); +#else /* WIN32 */ + int backslash_run_length = 0; + + /* + * A Windows system() argument experiences two layers of interpretation. + * First, cmd.exe interprets the string. Its behavior is undocumented, + * but a caret escapes any byte except LF or CR that would otherwise have + * special meaning. Handling of a caret before LF or CR differs between + * "cmd.exe /c" and other modes, and it is unusable here. + * + * Second, the new process parses its command line to construct argv (see + * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats + * backslash-double quote sequences specially. + */ + appendPQExpBufferStr(buf, "^\""); + for (p = str; *p; p++) + { + if (*p == '\n' || *p == '\r') + { + fprintf(stderr, + _("shell command argument contains a newline or carriage return: \"%s\"\n"), + str); + exit(EXIT_FAILURE); + } + + /* Change N backslashes before a double quote to 2N+1 backslashes. */ + if (*p == '"') + { + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\\"); + } + else if (*p == '\\') + backslash_run_length++; + else + backslash_run_length = 0; + + /* + * Decline to caret-escape the most mundane characters, to ease + * debugging and lest we approach the command length limit. + */ + if (!((*p >= 'a' && *p <= 'z') || + (*p >= 'A' && *p <= 'Z') || + (*p >= '0' && *p <= '9'))) + appendPQExpBufferChar(buf, '^'); + appendPQExpBufferChar(buf, *p); + } + + /* + * Change N backslashes at end of argument to 2N backslashes, because they + * precede the double quote that terminates the argument. + */ + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\""); +#endif /* WIN32 */ +} + + +/* + * Append the given string to the buffer, with suitable quoting for passing + * the string as a value, in a keyword/pair value in a libpq connection + * string + */ +void +appendConnStrVal(PQExpBuffer buf, const char *str) +{ + const char *s; + bool needquotes; + + /* + * If the string is one or more plain ASCII characters, no need to quote + * it. This is quite conservative, but better safe than sorry. + */ + needquotes = true; + for (s = str; *s; s++) + { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + needquotes = true; + break; + } + needquotes = false; + } + + if (needquotes) + { + appendPQExpBufferChar(buf, '\''); + while (*str) + { + /* ' and \ must be escaped by to \' and \\ */ + if (*str == '\'' || *str == '\\') + appendPQExpBufferChar(buf, '\\'); + + appendPQExpBufferChar(buf, *str); + str++; + } + appendPQExpBufferChar(buf, '\''); + } + else + appendPQExpBufferStr(buf, str); +} + + +/* + * Append a psql meta-command that connects to the given database with the + * then-current connection's user, host and port. + */ +void +appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname) +{ + const char *s; + bool complex; + + /* + * If the name is plain ASCII characters, emit a trivial "\connect "foo"". + * For other names, even many not technically requiring it, skip to the + * general case. No database has a zero-length name. + */ + complex = false; + for (s = dbname; *s; s++) + { + if (*s == '\n' || *s == '\r') + { + fprintf(stderr, + _("database name contains a newline or carriage return: \"%s\"\n"), + dbname); + exit(EXIT_FAILURE); + } + + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + complex = true; + } + } + + appendPQExpBufferStr(buf, "\\connect "); + if (complex) + { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + appendPQExpBuffer(buf, "-reuse-previous=on "); + + /* + * As long as the name does not contain a newline, SQL identifier + * quoting satisfies the psql meta-command parser. Prefer not to + * involve psql-interpreted single quotes, which behaved differently + * before PostgreSQL 9.2. + */ + appendPQExpBufferStr(buf, fmtId(connstr.data)); + + termPQExpBuffer(&connstr); + } + else + appendPQExpBufferStr(buf, fmtId(dbname)); + appendPQExpBufferChar(buf, '\n'); +} + + /* * Convert a bytea value (presented as raw bytes) to an SQL string literal * and append it to the given buffer. We assume the specified diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index b1767468c13..b8ebc5444b4 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -81,6 +81,9 @@ extern void appendStringLiteralDQ(PQExpBuffer buf, const char *str, extern void appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length, bool std_strings); +extern void appendShellString(PQExpBuffer buf, const char *str); +extern void appendConnStrVal(PQExpBuffer buf, const char *str); +extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname); extern bool parsePGArray(const char *atext, char ***itemarray, int *nitems); extern bool buildACLCommands(const char *name, const char *subname, const char *type, const char *acls, const char *owner, diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 45098d985a6..7ee560e8cd4 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -102,7 +102,7 @@ typedef struct _restoreOptions SimpleStringList tableNames; int useDB; - char *dbname; + char *dbname; /* subject to expand_dbname */ char *pgport; char *pghost; char *username; @@ -120,7 +120,7 @@ typedef struct _restoreOptions typedef struct _dumpOptions { - const char *dbname; + const char *dbname; /* subject to expand_dbname */ const char *pghost; const char *pgport; const char *username; diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 0be637254c5..6fe96f06623 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -762,9 +762,16 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel) /* If we created a DB, connect to it... */ if (strcmp(te->desc, "DATABASE") == 0) { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBufferStr(&connstr, "dbname="); + appendConnStrVal(&connstr, te->tag); + /* Abandon struct, but keep its buffer until process exit. */ + ahlog(AH, 1, "connecting to new database \"%s\"\n", te->tag); _reconnectToDB(AH, te->tag); - ropt->dbname = pg_strdup(te->tag); + ropt->dbname = connstr.data; } } @@ -2921,12 +2928,17 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname) ReconnectToServer(AH, dbname, NULL); else { - PQExpBuffer qry = createPQExpBuffer(); + if (dbname) + { + PQExpBufferData connectbuf; - appendPQExpBuffer(qry, "\\connect %s\n\n", - dbname ? fmtId(dbname) : "-"); - ahprintf(AH, "%s", qry->data); - destroyPQExpBuffer(qry); + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, dbname); + ahprintf(AH, "%s\n", connectbuf.data); + termPQExpBuffer(&connectbuf); + } + else + ahprintf(AH, "%s\n", "\\connect -\n"); } /* @@ -4400,7 +4412,7 @@ CloneArchive(ArchiveHandle *AH) } else { - char *dbname; + PQExpBufferData connstr; char *pghost; char *pgport; char *username; @@ -4413,14 +4425,18 @@ CloneArchive(ArchiveHandle *AH) * because all just return a pointer and do not actually send/receive * any data to/from the database. */ - dbname = PQdb(AH->connection); + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQdb(AH->connection)); pghost = PQhost(AH->connection); pgport = PQport(AH->connection); username = PQuser(AH->connection); /* this also sets clone->connection */ - ConnectDatabase((Archive *) clone, dbname, pghost, pgport, username, TRI_NO); + ConnectDatabase((Archive *) clone, connstr.data, + pghost, pgport, username, TRI_NO); + termPQExpBuffer(&connstr); /* setupDumpWorker will fix up connection state */ } diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 352595e49fa..b01b174d9db 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -128,6 +128,7 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username) static PGconn * _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) { + PQExpBufferData connstr; PGconn *newConn; const char *newdb; const char *newuser; @@ -156,6 +157,10 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) exit_horribly(modulename, "out of memory\n"); } + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, newdb); + do { const char *keywords[7]; @@ -170,7 +175,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) keywords[3] = "password"; values[3] = password; keywords[4] = "dbname"; - values[4] = newdb; + values[4] = connstr.data; keywords[5] = "fallback_application_name"; values[5] = progname; keywords[6] = NULL; @@ -222,6 +227,8 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser) if (password) free(password); + termPQExpBuffer(&connstr); + /* check for version mismatch */ _check_database_version(AH); diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index da5ba2645ca..6ea6be569cd 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -49,8 +49,6 @@ static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem, const char *name2); static void dumpDatabases(PGconn *conn); static void dumpTimestamp(const char *msg); -static void appendShellString(PQExpBuffer buf, const char *str); -static void appendConnStrVal(PQExpBuffer buf, const char *str); static int runPgDump(const char *dbname); static void buildShSecLabels(PGconn *conn, const char *catalog_name, @@ -1428,7 +1426,7 @@ dumpCreateDB(PGconn *conn) fdbname, fmtId(dbtablespace)); /* connect to original database */ - appendPQExpBuffer(buf, "\\connect %s\n", fdbname); + appendPsqlMetaConnect(buf, dbname); } if (binary_upgrade) @@ -1654,11 +1652,15 @@ dumpDatabases(PGconn *conn) int ret; char *dbname = PQgetvalue(res, i, 0); + PQExpBufferData connectbuf; if (verbose) fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname); - fprintf(OPF, "\\connect %s\n\n", fmtId(dbname)); + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, dbname); + fprintf(OPF, "%s\n", connectbuf.data); + termPQExpBuffer(&connectbuf); /* * Restore will need to write to the target cluster. This connection @@ -1814,7 +1816,9 @@ connectDatabase(const char *dbname, const char *connection_string, /* * Merge the connection info inputs given in form of connection string - * and other options. + * and other options. Explicitly discard any dbname value in the + * connection string; otherwise, PQconnectdbParams() would interpret + * that value as being itself a connection string. */ if (connection_string) { @@ -1827,7 +1831,8 @@ connectDatabase(const char *dbname, const char *connection_string, for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) { - if (conn_opt->val != NULL && conn_opt->val[0] != '\0') + if (conn_opt->val != NULL && conn_opt->val[0] != '\0' && + strcmp(conn_opt->keyword, "dbname") != 0) argcount++; } @@ -1836,7 +1841,8 @@ connectDatabase(const char *dbname, const char *connection_string, for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++) { - if (conn_opt->val != NULL && conn_opt->val[0] != '\0') + if (conn_opt->val != NULL && conn_opt->val[0] != '\0' && + strcmp(conn_opt->keyword, "dbname") != 0) { keywords[i] = conn_opt->keyword; values[i] = conn_opt->val; @@ -2081,145 +2087,3 @@ dumpTimestamp(const char *msg) if (strftime(buf, sizeof(buf), PGDUMP_STRFTIME_FMT, localtime(&now)) != 0) fprintf(OPF, "-- %s %s\n\n", msg, buf); } - - -/* - * Append the given string to the buffer, with suitable quoting for passing - * the string as a value, in a keyword/pair value in a libpq connection - * string - */ -static void -appendConnStrVal(PQExpBuffer buf, const char *str) -{ - const char *s; - bool needquotes; - - /* - * If the string consists entirely of plain ASCII characters, no need to - * quote it. This is quite conservative, but better safe than sorry. - */ - needquotes = false; - for (s = str; *s; s++) - { - if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || - (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) - { - needquotes = true; - break; - } - } - - if (needquotes) - { - appendPQExpBufferChar(buf, '\''); - while (*str) - { - /* ' and \ must be escaped by to \' and \\ */ - if (*str == '\'' || *str == '\\') - appendPQExpBufferChar(buf, '\\'); - - appendPQExpBufferChar(buf, *str); - str++; - } - appendPQExpBufferChar(buf, '\''); - } - else - appendPQExpBufferStr(buf, str); -} - -/* - * Append the given string to the shell command being built in the buffer, - * with suitable shell-style quoting to create exactly one argument. - * - * Forbid LF or CR characters, which have scant practical use beyond designing - * security breaches. The Windows command shell is unusable as a conduit for - * 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. - */ -static void -appendShellString(PQExpBuffer buf, const char *str) -{ - const char *p; - -#ifndef WIN32 - appendPQExpBufferChar(buf, '\''); - for (p = str; *p; p++) - { - if (*p == '\n' || *p == '\r') - { - fprintf(stderr, - _("shell command argument contains a newline or carriage return: \"%s\"\n"), - str); - exit(EXIT_FAILURE); - } - - if (*p == '\'') - appendPQExpBufferStr(buf, "'\"'\"'"); - else - appendPQExpBufferChar(buf, *p); - } - appendPQExpBufferChar(buf, '\''); -#else /* WIN32 */ - int backslash_run_length = 0; - - /* - * A Windows system() argument experiences two layers of interpretation. - * First, cmd.exe interprets the string. Its behavior is undocumented, - * but a caret escapes any byte except LF or CR that would otherwise have - * special meaning. Handling of a caret before LF or CR differs between - * "cmd.exe /c" and other modes, and it is unusable here. - * - * Second, the new process parses its command line to construct argv (see - * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats - * backslash-double quote sequences specially. - */ - appendPQExpBufferStr(buf, "^\""); - for (p = str; *p; p++) - { - if (*p == '\n' || *p == '\r') - { - fprintf(stderr, - _("shell command argument contains a newline or carriage return: \"%s\"\n"), - str); - exit(EXIT_FAILURE); - } - - /* Change N backslashes before a double quote to 2N+1 backslashes. */ - if (*p == '"') - { - while (backslash_run_length) - { - appendPQExpBufferStr(buf, "^\\"); - backslash_run_length--; - } - appendPQExpBufferStr(buf, "^\\"); - } - else if (*p == '\\') - backslash_run_length++; - else - backslash_run_length = 0; - - /* - * Decline to caret-escape the most mundane characters, to ease - * debugging and lest we approach the command length limit. - */ - if (!((*p >= 'a' && *p <= 'z') || - (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9'))) - appendPQExpBufferChar(buf, '^'); - appendPQExpBufferChar(buf, *p); - } - - /* - * Change N backslashes at end of argument to 2N backslashes, because they - * precede the double quote that terminates the argument. - */ - while (backslash_run_length) - { - appendPQExpBufferStr(buf, "^\\"); - backslash_run_length--; - } - appendPQExpBufferStr(buf, "^\""); -#endif /* WIN32 */ -} diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index b079b6de85b..11502a9f0e0 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -404,12 +404,17 @@ void create_script_for_cluster_analyze(char **analyze_script_file_name) { FILE *script = NULL; - char *user_specification = ""; + PQExpBufferData user_specification; prep_status("Creating script to analyze new cluster"); + initPQExpBuffer(&user_specification); if (os_info.user_specified) - user_specification = psprintf("-U \"%s\" ", os_info.user); + { + appendPQExpBufferStr(&user_specification, "-U "); + appendShellString(&user_specification, os_info.user); + appendPQExpBufferChar(&user_specification, ' '); + } *analyze_script_file_name = psprintf("%sanalyze_new_cluster.%s", SCRIPT_PREFIX, SCRIPT_EXT); @@ -449,18 +454,18 @@ create_script_for_cluster_analyze(char **analyze_script_file_name) fprintf(script, "echo %sthis script and run:%s\n", ECHO_QUOTE, ECHO_QUOTE); fprintf(script, "echo %s \"%s/vacuumdb\" %s--all %s%s\n", ECHO_QUOTE, - new_cluster.bindir, user_specification, + new_cluster.bindir, user_specification.data, /* Did we copy the free space files? */ (GET_MAJOR_VERSION(old_cluster.major_version) >= 804) ? "--analyze-only" : "--analyze", ECHO_QUOTE); fprintf(script, "echo%s\n\n", ECHO_BLANK); fprintf(script, "\"%s/vacuumdb\" %s--all --analyze-in-stages\n", - new_cluster.bindir, user_specification); + new_cluster.bindir, user_specification.data); /* Did we copy the free space files? */ if (GET_MAJOR_VERSION(old_cluster.major_version) < 804) fprintf(script, "\"%s/vacuumdb\" %s--all\n", new_cluster.bindir, - user_specification); + user_specification.data); fprintf(script, "echo%s\n\n", ECHO_BLANK); fprintf(script, "echo %sDone%s\n", @@ -474,8 +479,7 @@ create_script_for_cluster_analyze(char **analyze_script_file_name) *analyze_script_file_name, getErrorText()); #endif - if (os_info.user_specified) - pg_free(user_specification); + termPQExpBuffer(&user_specification); check_ok(); } diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c index 19e52e21411..dd97891a082 100644 --- a/src/bin/pg_upgrade/dump.c +++ b/src/bin/pg_upgrade/dump.c @@ -46,6 +46,15 @@ generate_old_dump(void) char sql_file_name[MAXPGPATH], log_file_name[MAXPGPATH]; DbInfo *old_db = &old_cluster.dbarr.dbs[dbnum]; + PQExpBufferData connstr, + escaped_connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, old_db->db_name); + initPQExpBuffer(&escaped_connstr); + appendShellString(&escaped_connstr, connstr.data); + termPQExpBuffer(&connstr); pg_log(PG_STATUS, "%s", old_db->db_name); snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid); @@ -53,10 +62,12 @@ generate_old_dump(void) parallel_exec_prog(log_file_name, NULL, "\"%s/pg_dump\" %s --schema-only --quote-all-identifiers " - "--binary-upgrade --format=custom %s --file=\"%s\" \"%s\"", + "--binary-upgrade --format=custom %s --file=\"%s\" %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), log_opts.verbose ? "--verbose" : "", - sql_file_name, old_db->db_name); + sql_file_name, escaped_connstr.data); + + termPQExpBuffer(&escaped_connstr); } /* reap all children */ diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 566db9d40c1..59ccc67adce 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -307,6 +307,15 @@ create_new_objects(void) char sql_file_name[MAXPGPATH], log_file_name[MAXPGPATH]; DbInfo *old_db = &old_cluster.dbarr.dbs[dbnum]; + PQExpBufferData connstr, + escaped_connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, old_db->db_name); + initPQExpBuffer(&escaped_connstr); + appendShellString(&escaped_connstr, connstr.data); + termPQExpBuffer(&connstr); pg_log(PG_STATUS, "%s", old_db->db_name); snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid); @@ -318,11 +327,13 @@ create_new_objects(void) */ parallel_exec_prog(log_file_name, NULL, - "\"%s/pg_restore\" %s --exit-on-error --verbose --dbname \"%s\" \"%s\"", + "\"%s/pg_restore\" %s --exit-on-error --verbose --dbname %s \"%s\"", new_cluster.bindir, cluster_conn_opts(&new_cluster), - old_db->db_name, + escaped_connstr.data, sql_file_name); + + termPQExpBuffer(&escaped_connstr); } /* reap all children */ diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 50660050fb9..627ef7aabbc 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -11,6 +11,7 @@ #include #include "libpq-fe.h" +#include "pqexpbuffer.h" /* Use port in the private/dynamic port number range */ #define DEF_PGUPORT 50432 @@ -451,6 +452,9 @@ void check_pghost_envvar(void); /* util.c */ char *quote_identifier(const char *s); +extern void appendShellString(PQExpBuffer buf, const char *str); +extern void appendConnStrVal(PQExpBuffer buf, const char *str); +extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname); int get_user_info(char **user_name_p); void check_ok(void); void report_status(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3); diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c index a1a8b098e6c..14373894121 100644 --- a/src/bin/pg_upgrade/server.c +++ b/src/bin/pg_upgrade/server.c @@ -51,18 +51,25 @@ connectToServer(ClusterInfo *cluster, const char *db_name) static PGconn * get_db_conn(ClusterInfo *cluster, const char *db_name) { - char conn_opts[2 * NAMEDATALEN + MAXPGPATH + 100]; + PQExpBufferData conn_opts; + PGconn *conn; + /* Build connection string with proper quoting */ + initPQExpBuffer(&conn_opts); + appendPQExpBufferStr(&conn_opts, "dbname="); + appendConnStrVal(&conn_opts, db_name); + appendPQExpBufferStr(&conn_opts, " user="); + appendConnStrVal(&conn_opts, os_info.user); + appendPQExpBuffer(&conn_opts, " port=%d", cluster->port); if (cluster->sockdir) - snprintf(conn_opts, sizeof(conn_opts), - "dbname = '%s' user = '%s' host = '%s' port = %d", - db_name, os_info.user, cluster->sockdir, cluster->port); - else - snprintf(conn_opts, sizeof(conn_opts), - "dbname = '%s' user = '%s' port = %d", - db_name, os_info.user, cluster->port); + { + appendPQExpBufferStr(&conn_opts, " host="); + appendConnStrVal(&conn_opts, cluster->sockdir); + } - return PQconnectdb(conn_opts); + conn = PQconnectdb(conn_opts.data); + termPQExpBuffer(&conn_opts); + return conn; } @@ -74,23 +81,28 @@ get_db_conn(ClusterInfo *cluster, const char *db_name) * sets, but the utilities we need aren't very consistent about the treatment * of database name options, so we leave that out. * - * Note result is in static storage, so use it right away. + * Result is valid until the next call to this function. */ char * cluster_conn_opts(ClusterInfo *cluster) { - static char conn_opts[MAXPGPATH + NAMEDATALEN + 100]; + static PQExpBuffer buf; + + if (buf == NULL) + buf = createPQExpBuffer(); + else + resetPQExpBuffer(buf); if (cluster->sockdir) - snprintf(conn_opts, sizeof(conn_opts), - "--host \"%s\" --port %d --username \"%s\"", - cluster->sockdir, cluster->port, os_info.user); - else - snprintf(conn_opts, sizeof(conn_opts), - "--port %d --username \"%s\"", - cluster->port, os_info.user); + { + appendPQExpBufferStr(buf, "--host "); + appendShellString(buf, cluster->sockdir); + appendPQExpBufferChar(buf, ' '); + } + appendPQExpBuffer(buf, "--port %d --username ", cluster->port); + appendShellString(buf, os_info.user); - return conn_opts; + return buf->data; } diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh index aa7f3994caa..bb414199241 100644 --- a/src/bin/pg_upgrade/test.sh +++ b/src/bin/pg_upgrade/test.sh @@ -153,6 +153,20 @@ set -x standard_initdb "$oldbindir"/initdb "$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w + +# Create databases with names covering the ASCII bytes other than NUL, BEL, +# LF, or CR. BEL would ring the terminal bell in the course of this test, and +# it is not otherwise a special case. PostgreSQL doesn't support the rest. +dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++) + if (i != 7 && i != 10 && i != 13) printf "%c", i }' = 'a' && *p <= 'z') || + (*p >= 'A' && *p <= 'Z') || + (*p >= '0' && *p <= '9'))) + appendPQExpBufferChar(buf, '^'); + appendPQExpBufferChar(buf, *p); + } + + /* + * Change N backslashes at end of argument to 2N backslashes, because they + * precede the double quote that terminates the argument. + */ + while (backslash_run_length) + { + appendPQExpBufferStr(buf, "^\\"); + backslash_run_length--; + } + appendPQExpBufferStr(buf, "^\""); +#endif /* WIN32 */ +} + + +/* + * Append the given string to the buffer, with suitable quoting for passing + * the string as a value, in a keyword/pair value in a libpq connection + * string + */ +void +appendConnStrVal(PQExpBuffer buf, const char *str) +{ + const char *s; + bool needquotes; + + /* + * If the string is one or more plain ASCII characters, no need to quote + * it. This is quite conservative, but better safe than sorry. + */ + needquotes = true; + for (s = str; *s; s++) + { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + needquotes = true; + break; + } + needquotes = false; + } + + if (needquotes) + { + appendPQExpBufferChar(buf, '\''); + while (*str) + { + /* ' and \ must be escaped by to \' and \\ */ + if (*str == '\'' || *str == '\\') + appendPQExpBufferChar(buf, '\\'); + + appendPQExpBufferChar(buf, *str); + str++; + } + appendPQExpBufferChar(buf, '\''); + } + else + appendPQExpBufferStr(buf, str); +} + + +/* + * Append a psql meta-command that connects to the given database with the + * then-current connection's user, host and port. + */ +void +appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname) +{ + const char *s; + bool complex; + + /* + * If the name is plain ASCII characters, emit a trivial "\connect "foo"". + * For other names, even many not technically requiring it, skip to the + * general case. No database has a zero-length name. + */ + complex = false; + for (s = dbname; *s; s++) + { + if (*s == '\n' || *s == '\r') + { + fprintf(stderr, + _("database name contains a newline or carriage return: \"%s\"\n"), + dbname); + exit(EXIT_FAILURE); + } + + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || + (*s >= '0' && *s <= '9') || *s == '_' || *s == '.')) + { + complex = true; + } + } + + appendPQExpBufferStr(buf, "\\connect "); + if (complex) + { + PQExpBufferData connstr; + + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + appendPQExpBuffer(buf, "-reuse-previous=on "); + + /* + * As long as the name does not contain a newline, SQL identifier + * quoting satisfies the psql meta-command parser. Prefer not to + * involve psql-interpreted single quotes, which behaved differently + * before PostgreSQL 9.2. + */ + appendPQExpBufferStr(buf, quote_identifier(connstr.data)); + + termPQExpBuffer(&connstr); + } + else + appendPQExpBufferStr(buf, quote_identifier(dbname)); + appendPQExpBufferChar(buf, '\n'); +} + + /* * get_user_info() */ diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c index b93c414d7a3..c215974ea99 100644 --- a/src/bin/pg_upgrade/version.c +++ b/src/bin/pg_upgrade/version.c @@ -48,10 +48,16 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster, bool check_mode) found = true; if (!check_mode) { + PQExpBufferData connectbuf; + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s\n", output_path, getErrorText()); - fprintf(script, "\\connect %s\n", - quote_identifier(active_db->db_name)); + + initPQExpBuffer(&connectbuf); + appendPsqlMetaConnect(&connectbuf, active_db->db_name); + fputs(connectbuf.data, script); + termPQExpBuffer(&connectbuf); + fprintf(script, "SELECT pg_catalog.lo_create(t.loid)\n" "FROM (SELECT DISTINCT loid FROM pg_catalog.pg_largeobject) AS t;\n"); diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 6a885674d06..4537a7220a8 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1630,6 +1630,7 @@ do_connect(enum trivalue reuse_previous_specification, bool keep_password; bool has_connection_string; bool reuse_previous; + PQExpBufferData connstr; if (!o_conn && (!dbname || !user || !host || !port)) { @@ -1693,7 +1694,15 @@ do_connect(enum trivalue reuse_previous_specification, * changes: passwords aren't (usually) database-specific. */ if (!dbname && reuse_previous) - dbname = PQdb(o_conn); + { + initPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQdb(o_conn)); + dbname = connstr.data; + /* has_connection_string=true would be a dead store */ + } + else + connstr.data = NULL; /* * If the user asked to be prompted for a password, ask for one now. If @@ -1802,8 +1811,12 @@ do_connect(enum trivalue reuse_previous_specification, } PQfinish(n_conn); + if (connstr.data) + termPQExpBuffer(&connstr); return false; } + if (connstr.data) + termPQExpBuffer(&connstr); /* * Replace the old connection with the new one, and update diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c index bf888bd4e97..75dc051ef13 100644 --- a/src/bin/scripts/clusterdb.c +++ b/src/bin/scripts/clusterdb.c @@ -229,6 +229,7 @@ cluster_all_databases(bool verbose, const char *maintenance_db, { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int i; conn = connectMaintenanceDatabase(maintenance_db, host, port, username, @@ -236,6 +237,7 @@ cluster_all_databases(bool verbose, const char *maintenance_db, result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); for (i = 0; i < PQntuples(result); i++) { char *dbname = PQgetvalue(result, i, 0); @@ -246,10 +248,15 @@ cluster_all_databases(bool verbose, const char *maintenance_db, fflush(stdout); } - cluster_one_database(dbname, verbose, NULL, + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + cluster_one_database(connstr.data, verbose, NULL, host, port, username, prompt_password, progname, echo); } + termPQExpBuffer(&connstr); PQclear(result); } diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index e579c2a75af..a2aaed1faf8 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -330,6 +330,7 @@ reindex_all_databases(const char *maintenance_db, { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int i; conn = connectMaintenanceDatabase(maintenance_db, host, port, username, @@ -337,6 +338,7 @@ reindex_all_databases(const char *maintenance_db, result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); for (i = 0; i < PQntuples(result); i++) { char *dbname = PQgetvalue(result, i, 0); @@ -347,9 +349,15 @@ reindex_all_databases(const char *maintenance_db, fflush(stdout); } - reindex_one_database(dbname, dbname, "DATABASE", host, port, username, - prompt_password, progname, echo, verbose); + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, dbname); + + reindex_one_database(NULL, connstr.data, "DATABASE", host, + port, username, prompt_password, + progname, echo, verbose); } + termPQExpBuffer(&connstr); PQclear(result); } @@ -372,7 +380,7 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port, if (verbose) appendPQExpBuffer(&sql, " (VERBOSE)"); - appendPQExpBuffer(&sql, " SYSTEM %s;", PQdb(conn)); + appendPQExpBuffer(&sql, " SYSTEM %s;", fmtId(PQdb(conn))); if (!executeMaintenanceCommand(conn, sql.data, echo)) { diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index 056e24ddab8..f99be3bf7a1 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -539,6 +539,7 @@ vacuum_all_databases(vacuumingOptions *vacopts, { PGconn *conn; PGresult *result; + PQExpBufferData connstr; int stage; int i; @@ -549,6 +550,7 @@ vacuum_all_databases(vacuumingOptions *vacopts, progname, echo); PQfinish(conn); + initPQExpBuffer(&connstr); if (analyze_in_stages) { /* @@ -563,10 +565,11 @@ vacuum_all_databases(vacuumingOptions *vacopts, { for (i = 0; i < PQntuples(result); i++) { - const char *dbname; + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQgetvalue(result, i, 0)); - dbname = PQgetvalue(result, i, 0); - vacuum_one_database(dbname, vacopts, + vacuum_one_database(connstr.data, vacopts, stage, NULL, host, port, username, prompt_password, @@ -579,10 +582,11 @@ vacuum_all_databases(vacuumingOptions *vacopts, { for (i = 0; i < PQntuples(result); i++) { - const char *dbname; + resetPQExpBuffer(&connstr); + appendPQExpBuffer(&connstr, "dbname="); + appendConnStrVal(&connstr, PQgetvalue(result, i, 0)); - dbname = PQgetvalue(result, i, 0); - vacuum_one_database(dbname, vacopts, + vacuum_one_database(connstr.data, vacopts, ANALYZE_NO_STAGE, NULL, host, port, username, prompt_password, @@ -590,6 +594,7 @@ vacuum_all_databases(vacuumingOptions *vacopts, progname, echo, quiet); } } + termPQExpBuffer(&connstr); PQclear(result); } diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index a570b81265d..a1c7a9672a6 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -4422,7 +4422,11 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage, * of "dbname" keyword is a connection string (as indicated by * recognized_connection_string) then parse and process it, overriding any * previously processed conflicting keywords. Subsequent keywords will take - * precedence, however. + * precedence, however. In-tree programs generally specify expand_dbname=true, + * so command-line arguments naming a database can use a connection string. + * Some code acquires arbitrary database names from known-literal sources like + * PQdb(), PQconninfoParse() and pg_database.datname. When connecting to such + * a database, in-tree code first wraps the name in a connection string. */ static PQconninfoOption * conninfo_array_parse(const char *const * keywords, const char *const * values, diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index 44080ed471d..7d6135b0b02 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -369,6 +369,41 @@ sub standard_initdb $ENV{PGDATA}) == 0); } +# This is similar to appendShellString(). Perl system(@args) bypasses +# cmd.exe, so omit the caret escape layer. +sub quote_system_arg +{ + my $arg = shift; + + # Change N >= 0 backslashes before a double quote to 2N+1 backslashes. + $arg =~ s/(\\*)"/${\($1 . $1)}\\"/gs; + + # Change N >= 1 backslashes at end of argument to 2N backslashes. + $arg =~ s/(\\+)$/${\($1 . $1)}/gs; + + # Wrap the whole thing in unescaped double quotes. + return "\"$arg\""; +} + +# Generate a database with a name made of a range of ASCII characters, useful +# for testing pg_upgrade. +sub generate_db +{ + my ($prefix, $from_char, $to_char, $suffix) = @_; + + my $dbname = $prefix; + for my $i ($from_char .. $to_char) + { + next if $i == 7 || $i == 10 || $i == 13; # skip BEL, LF, and CR + $dbname = $dbname . sprintf('%c', $i); + } + $dbname .= $suffix; + + system('createdb', quote_system_arg($dbname)); + my $status = $? >> 8; + exit $status if $status; +} + sub upgradecheck { my $status; @@ -402,6 +437,12 @@ sub upgradecheck print "\nStarting old cluster\n\n"; my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log", '-w'); system(@args) == 0 or exit 1; + + print "\nCreating databases with names covering most ASCII bytes\n\n"; + generate_db("\\\"\\", 1, 45, "\\\\\"\\\\\\"); + generate_db('', 46, 90, ''); + generate_db('', 91, 127, ''); + print "\nSetting up data for upgrading\n\n"; installcheck();