Re: BUG #15827: Unable to connect on Windows using pg_services.conf using Python psycopg2
От | Tom Lane |
---|---|
Тема | Re: BUG #15827: Unable to connect on Windows using pg_services.conf using Python psycopg2 |
Дата | |
Msg-id | 25605.1561652435@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | Re: BUG #15827: Unable to connect on Windows using pg_services.conf using Python psycopg2 (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: BUG #15827: Unable to connect on Windows using pg_services.confusing Python psycopg2
(Michael Paquier <michael@paquier.xyz>)
|
Список | pgsql-bugs |
I wrote: > I'm still of the opinion that > (1) it's very weird that this code allows for leading space on a line > but not trailing space; > (2) we need to look for other places where we have the same issue. > Possibly libpq is the only chunk of our code that's at serious risk, > since we don't change the default binary mode in the backend. But > even if you assume that that's true, this isn't the only config file > that libpq examines. Patch 0001 attached responds to point (1), ie it uses isspace() tests to get rid of \r and \n and any trailing whitespace in parseServiceFile(). I think we should do this in HEAD, but there's perhaps an argument to be made that this is a behavior change and it'd be better to use Michael's patch in the back branches. For point (2), I looked through all other fgets() callers in our code. Not all of them have newline-chomping logic, but I made all the ones that do have such do it the same way (except for those that use the isspace() method, which is fine). I'm not sure if this is fixing any live bugs --- most of these places are reading popen() output, and it's unclear to me whether we can rely on that to suppress \r on Windows. The Windows-specific code in pipe_read_line seems to think not (but if its test were dead code we wouldn't know it); yet if this were a hazard you'd think we'd have gotten complaints about at least one of these places. Still, I dislike code that has three or four randomly different ways of doing the exact same thing, especially when some of them are gratuitously inefficient. Note that I standardized on a loop that chomps trailing \r and \n indiscriminately, not the "if chomp \n then chomp \r" approach we had in some places. I think that approach does have a corner-case bug: if the input is long enough that the \r fits into the buffer but the \n doesn't, it'd fail to chomp the \r. Thoughts? regards, tom lane diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index c800d79..f2b166b 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 */ diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index 877226d..4abbef5 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 dfb6c19..79c4627 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 2734f87..256363c 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 73f395f..202da23 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 4514cf8..59afbc7 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 c800d79..f2b166b 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -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 146fb00..02164d4 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) {
В списке pgsql-bugs по дате отправления:
Следующее
От: Alvaro HerreraДата:
Сообщение: Re: BUG #15724: Can't create foreign table as partition