8000 Fix recently-introduced breakage in psql's \connect command. · postgrespro/postgres_cluster@72b930f · GitHub
[go: up one dir, main page]
Skip to content

Commit 72b930f

Browse files
tglsfdctglsfdc
committed
Fix recently-introduced breakage in psql's \connect command.
Through my misreading of what the existing code actually did, commits 85c54287a et al. broke psql's behavior for the case where "\c connstring" provides a password in the connstring. We should use that password in such a case, but as of 85c54287a we ignored it (and instead, prompted for a password). Commit 94929f1cf fixed that in HEAD, but since I thought it was cleaning up a longstanding misbehavior and not one I'd just created, I didn't back-patch it. Hence, back-patch the portions of 94929f1cf having to do with password management. In addition to fixing the introduced bug, this means that "\c -reuse-previous=on connstring" will allow re-use of an existing connection's password if the connstring doesn't change user/host/port. That didn't happen before, but it seems like a bug fix, and anyway I'm loath to have significant differences in this code across versions. Also fix an error with the same root cause about whether or not to override a connstring's setting of client_encoding. As of 85c54287a we always did so; restore the previous behavior of overriding only when stdin/stdout are a terminal and there's no environment setting of PGCLIENTENCODING. (I find that definition a bit surprising, but right now doesn't seem like the time to revisit it.) Per bug #16746 from Krzysztof Gradek. As with the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org
1 parent 1eb499a commit 72b930f

File tree

2 files changed

+51
-17
lines changed

2 files changed

+51
-17
lines changed

‎doc/src/sgml/ref/psql-ref.sgml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,8 @@ testdb=>
920920
is changed from its previous value using the positional syntax,
921921
any <replaceable>hostaddr</replaceable> setting present in the
922922
existing connection's parameters is dropped.
923+
Also, any password used for the existing connection will be re-used
924+
only if the user, host, and port settings are not changed.
923925
When the command neither specifies nor reuses a particular parameter,
924926
the <application>libpq</application> default is used.
925927
</para>

‎src/bin/psql/command.c‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3031,6 +3031,7 @@ do_connect(enum trivalue reuse_previous_specification,
30313031
int nconnopts = 0;
30323032
bool same_host = false;
30333033
char *password = NULL;
3034+
char *client_encoding;
30343035
bool success = true;
30353036
bool keep_password = true;
30363037
bool has_connection_string;
@@ -3101,6 +3102,7 @@ do_connect(enum trivalue reuse_previous_specification,
31013102
{
31023103
PQconninfoOption *ci;
31033104
PQconninfoOption *replci;
3105+
bool have_password = false;
31043106

31053107
for (ci = cinfo, replci = replcinfo;
31063108
ci->keyword && replci->keyword;
@@ -3119,6 +3121,26 @@ do_connect(enum trivalue reuse_previous_specification,
31193121

31203122
replci->val = ci->val;
31213123
ci->val = swap;
3124+
3125+
/*
3126+
* Check whether connstring provides options affecting
3127+
* password re-use. While any change in user, host,
3128+
* hostaddr, or port causes us to ignore the old
3129+
* connection's password, we don't force that for
3130+
* dbname, since passwords aren't database-specific.
3131+
*/
3132+
if (replci->val == NULL ||
3133+
strcmp(ci->val, replci->val) != 0)
3134+
{
3135+
if (strcmp(replci->keyword, "user") == 0 ||
3136+
strcmp(replci->keyword, "host") == 0 ||
3137+
strcmp(replci->keyword, "hostaddr") == 0 ||
3138+
strcmp(replci->keyword, "port") == 0)
3139+
keep_password = false;
3140+
}
3141+
/* Also note whether connstring contains a password. */
3142+
if (strcmp(replci->keyword, "password") == 0)
3143+
have_password = true;
31223144
}
31233145
}
31243146
Assert(ci->keyword == NULL && replci->keyword == NULL);
@@ -3128,8 +3150,13 @@ do_connect(enum trivalue reuse_previous_specification,
31283150

31293151
PQconninfoFree(replcinfo);
31303152

3131-
/* We never re-use a password with a conninfo string. */
3132-
keep_password = false;
3153+
/*
3154+
* If the connstring contains a password, tell the loop below
3155+
* that we may use it, regardless of other settings (i.e.,
3156+
* cinfo's password is no longer an "old" password).
3157+
*/
3158+
if (have_password)
3159+
keep_password = true;
31333160

31343161
/* Don't let code below try to inject dbname into params. */
31353162
dbname = NULL;
@@ -3217,14 +3244,16 @@ do_connect(enum trivalue reuse_previous_specification,
32173244
*/
32183245
password = prompt_for_password(has_connection_string ? NULL : user);
32193246
}
3220-
else if (o_conn && keep_password)
3221-
{
3222-
password = PQpass(o_conn);
3223-
if (password && *password)
3224-
password = pg_strdup(password);
3225-
else
3226-
password = NULL;
3227-
}
3247+
3248+
/*
3249+
* Consider whether to force client_encoding to "auto" (overriding
3250+
* anything in the connection string). We do so if we have a terminal
3251+
* connection and there is no PGCLIENTENCODING environment setting.
3252+
*/
3253+
if (pset.notty || getenv("PGCLIENTENCODING"))
3254+
client_encoding = NULL;
3255+
else
3256+
client_encoding = "auto";
32283257

32293258
/* Loop till we have a connection or fail, which we might've already */
32303259
while (success)
@@ -3236,12 +3265,12 @@ do_connect(enum trivalue reuse_previous_specification,
32363265

32373266
/*
32383267
* Copy non-default settings into the PQconnectdbParams parameter
3239-
* arrays; but override any values specified old-style, as well as the
3240-
* password and a couple of fields we want to set forcibly.
3268+
* arrays; but inject any values specified old-style, as well as any
3269+
* interactively-obtained password, and a couple of fields we want to
3270+
* set forcibly.
32413271
*
32423272
* If you change this code, see also the initial-connection code in
3243-
* main(). For no good reason, a connection string password= takes
3244-
* precedence in main() but not here.
3273+
* main().
32453274
*/
32463275
for (ci = cinfo; ci->keyword; ci++)
32473276
{
@@ -3260,12 +3289,15 @@ do_connect(enum trivalue reuse_previous_specification,
32603289
}
32613290
else if (port && strcmp(ci->keyword, "port") == 0)
32623291
values[paramnum++] = port;
3263-
else if (strcmp(ci->keyword, "password") == 0)
3292+
/* If !keep_password, we unconditionally drop old password */
3293+
else if ((password || !keep_password) &&
3294+
strcmp(ci->keyword, "password") == 0)
32643295
values[paramnum++] = password;
32653296
else if (strcmp(ci->keyword, "fallback_application_name") == 0)
32663297
values[paramnum++] = pset.progname;
3267-
else if (strcmp(ci->keyword, "client_encoding") == 0)
3268-
values[paramnum++] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
3298+
else if (client_encoding &&
3299+
strcmp(ci->keyword, "client_encoding") == 0)
3300+
values[paramnum++] = client_encoding;
32693301
else if (ci->val)
32703302
values[paramnum++] = ci->val;
32713303
/* else, don't bother making libpq parse this keyword */

0 commit comments

Comments
 (0)
0