From b654714f9bcfb4443bcc531c32f059fd85f798ec Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 25 Jul 2019 12:10:54 -0400 Subject: [PATCH] Fix failures to ignore \r when reading Windows-style newlines. libpq failed to ignore Windows-style newlines in connection service files. This normally wasn't a problem on Windows itself, because fgets() would convert \r\n to just \n. But if libpq were running inside a program that changes the default fopen mode to binary, it would see the \r's and think they were data. In any case, it's project policy to ignore \r in text files unconditionally, because people sometimes try to use files with DOS-style newlines on Unix machines, where the C library won't hide that from us. Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of the line. In HEAD, go a little further and make it ignore all trailing whitespace, to match what it's always done with leading whitespace. In HEAD, also run around and fix up everyplace where we have newline-chomping code to make all those places look consistent and uniformly drop \r. It is not clear whether any of those changes are fixing live bugs. Most of the non-cosmetic changes are in places that are reading popen output, and the jury is still out as to whether popen on Windows can return \r\n. (The Windows-specific code in pipe_read_line seems to think so, but our lack of support for this elsewhere suggests maybe it's not a problem in practice.) Hence, I desisted from applying those changes to back branches, except in run_ssl_passphrase_command() which is new enough and little-tested enough that we'd probably not have heard about any problems there. Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha. Back-patch the parseServiceFile() change to all supported branches, and the run_ssl_passphrase_command() change to v11 where that was added. Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org --- src/backend/libpq/be-secure-common.c | 5 +++-- src/bin/pg_ctl/pg_ctl.c | 10 +++++++--- src/bin/pg_resetwal/pg_resetwal.c | 8 +++----- src/bin/pg_upgrade/option.c | 21 +++++++++++++++------ src/bin/psql/prompt.c | 7 +++++-- src/interfaces/libpq/fe-connect.c | 23 +++++++++++------------ src/port/sprompt.c | 8 +++++--- 7 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index 877226d377..4abbef5bf1 100644 --- a/src/backend/libpq/be-secure-common.c +++ b/src/backend/libpq/be-secure-common.c @@ -112,9 +112,10 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, goto error; } - /* strip trailing newline */ + /* strip trailing newline, including \r in case we're on Windows */ len = strlen(buf); - if (len > 0 && buf[len - 1] == '\n') + while (len > 0 && (buf[len - 1] == '\n' || + buf[len - 1] == '\r')) buf[--len] = '\0'; error: diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index a10bc8d545..3a9a65d31d 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -2176,6 +2176,7 @@ adjust_data_dir(void) filename[MAXPGPATH], *my_exec_path; FILE *fd; + int len; /* do nothing if we're working without knowledge of data dir */ if (pg_config == NULL) @@ -2218,9 +2219,12 @@ adjust_data_dir(void) pclose(fd); free(my_exec_path); - /* Remove trailing newline */ - if (strchr(filename, '\n') != NULL) - *strchr(filename, '\n') = '\0'; + /* Remove trailing newline, handling Windows newlines as well */ + len = strlen(filename); + while (len > 0 && + (filename[len - 1] == '\n' || + filename[len - 1] == '\r')) + filename[--len] = '\0'; free(pg_data); pg_data = pg_strdup(filename); diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index ff0f8ea5e7..9d9c33d78c 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -559,12 +559,10 @@ CheckDataVersion(void) /* remove trailing newline, handling Windows newlines as well */ len = strlen(rawline); - if (len > 0 && rawline[len - 1] == '\n') - { + while (len > 0 && + (rawline[len - 1] == '\n' || + rawline[len - 1] == '\r')) rawline[--len] = '\0'; - if (len > 0 && rawline[len - 1] == '\r') - rawline[--len] = '\0'; - } if (strcmp(rawline, PG_MAJORVERSION) != 0) { diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index d76f27c9e8..b62cbbba43 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -405,6 +405,7 @@ adjust_data_dir(ClusterInfo *cluster) cmd_output[MAX_STRING]; FILE *fp, *output; + int len; /* Initially assume config dir and data dir are the same */ cluster->pgconfig = pg_strdup(cluster->pgdata); @@ -445,9 +446,12 @@ adjust_data_dir(ClusterInfo *cluster) pclose(output); - /* Remove trailing newline */ - if (strchr(cmd_output, '\n') != NULL) - *strchr(cmd_output, '\n') = '\0'; + /* Remove trailing newline, handling Windows newlines as well */ + len = strlen(cmd_output); + while (len > 0 && + (cmd_output[len - 1] == '\n' || + cmd_output[len - 1] == '\r')) + cmd_output[--len] = '\0'; cluster->pgdata = pg_strdup(cmd_output); @@ -508,10 +512,15 @@ get_sock_dir(ClusterInfo *cluster, bool live_check) sscanf(line, "%hu", &old_cluster.port); if (lineno == LOCK_FILE_LINE_SOCKET_DIR) { + int len; + cluster->sockdir = pg_strdup(line); - /* strip off newline */ - if (strchr(cluster->sockdir, '\n') != NULL) - *strchr(cluster->sockdir, '\n') = '\0'; + /* strip off newline, handling Windows newlines as well */ + len = strlen(cluster->sockdir); + while (len > 0 && + (cluster->sockdir[len - 1] == '\n' || + cluster->sockdir[len - 1] == '\r')) + cluster->sockdir[--len] = '\0'; } } fclose(fp); diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c index 4514cf8551..59afbc793a 100644 --- a/src/bin/psql/prompt.c +++ b/src/bin/psql/prompt.c @@ -264,6 +264,7 @@ get_prompt(promptStatus_t status, ConditionalStack cstack) FILE *fd; char *file = pg_strdup(p + 1); int cmdend; + int buflen; cmdend = strcspn(file, "`"); file[cmdend] = '\0'; @@ -274,8 +275,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack) buf[0] = '\0'; pclose(fd); } - if (strlen(buf) > 0 && buf[strlen(buf) - 1] == '\n') - buf[strlen(buf) - 1] = '\0'; + buflen = strlen(buf); + while (buflen > 0 && (buf[buflen - 1] == '\n' || + buf[buflen - 1] == '\r')) + buf[--buflen] = '\0'; free(file); p += cmdend + 1; break; diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index d70cf1f948..a329ebbf93 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -5020,6 +5020,8 @@ parseServiceFile(const char *serviceFile, while ((line = fgets(buf, sizeof(buf), f)) != NULL) { + int len; + linenr++; if (strlen(line) >= sizeof(buf) - 1) @@ -5032,16 +5034,17 @@ parseServiceFile(const char *serviceFile, return 2; } - /* ignore EOL at end of line */ - if (strlen(line) && line[strlen(line) - 1] == '\n') - line[strlen(line) - 1] = 0; + /* ignore whitespace at end of line, especially the newline */ + len = strlen(line); + while (len > 0 && isspace((unsigned char) line[len - 1])) + line[--len] = '\0'; - /* ignore leading blanks */ + /* ignore leading whitespace too */ while (*line && isspace((unsigned char) line[0])) line++; /* ignore comments and empty lines */ - if (strlen(line) == 0 || line[0] == '#') + if (line[0] == '\0' || line[0] == '#') continue; /* Check for right groupname */ @@ -6910,14 +6913,10 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname, len = strlen(buf); - /* Remove trailing newline */ - if (len > 0 && buf[len - 1] == '\n') - { + /* Remove trailing newline, including \r in case we're on Windows */ + while (len > 0 && (buf[len - 1] == '\n' || + buf[len - 1] == '\r')) buf[--len] = '\0'; - /* Handle DOS-style line endings, too, even when not on Windows */ - if (len > 0 && buf[len - 1] == '\r') - buf[--len] = '\0'; - } if (len == 0) continue; diff --git a/src/port/sprompt.c b/src/port/sprompt.c index 146fb0004e..02164d497a 100644 --- a/src/port/sprompt.c +++ b/src/port/sprompt.c @@ -144,9 +144,11 @@ simple_prompt(const char *prompt, char *destination, size_t destlen, bool echo) } while (buflen > 0 && buf[buflen - 1] != '\n'); } - if (length > 0 && destination[length - 1] == '\n') - /* remove trailing newline */ - destination[length - 1] = '\0'; + /* strip trailing newline, including \r in case we're on Windows */ + while (length > 0 && + (destination[length - 1] == '\n' || + destination[length - 1] == '\r')) + destination[--length] = '\0'; if (!echo) {