Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3958abd

Browse files
committed
Fix recently-introduced breakage in psql's \connect command.
Through my misreading of what the existing code actually did, commits 85c5428 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 85c5428 we ignored it (and instead, prompted for a password). Commit 94929f1 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 94929f1 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 85c5428 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 db83c04 commit 3958abd

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
@@ -921,6 +921,8 @@ testdb=>
921921
is changed from its previous value using the positional syntax,
922922
any <replaceable>hostaddr</replaceable> setting present in the
923923
existing connection's parameters is dropped.
924+
Also, any password used for the existing connection will be re-used
925+
only if the user, host, and port settings are not changed.
924926
When the command neither specifies nor reuses a particular parameter,
925927
the <application>libpq</application> default is used.
926928
</para>

src/bin/psql/command.c

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2892,6 +2892,7 @@ do_connect(enum trivalue reuse_previous_specification,
28922892
int nconnopts = 0;
28932893
bool same_host = false;
28942894
char *password = NULL;
2895+
char *client_encoding;
28952896
bool success = true;
28962897
bool keep_password = true;
28972898
bool has_connection_string;
@@ -2962,6 +2963,7 @@ do_connect(enum trivalue reuse_previous_specification,
29622963
{
29632964
PQconninfoOption *ci;
29642965
PQconninfoOption *replci;
2966+
bool have_password = false;
29652967

29662968
for (ci = cinfo, replci = replcinfo;
29672969
ci->keyword && replci->keyword;
@@ -2980,6 +2982,26 @@ do_connect(enum trivalue reuse_previous_specification,
29802982

29812983
replci->val = ci->val;
29822984
ci->val = swap;
2985+
2986+
/*
2987+
* Check whether connstring provides options affecting
2988+
* password re-use. While any change in user, host,
2989+
* hostaddr, or port causes us to ignore the old
2990+
* connection's password, we don't force that for
2991+
* dbname, since passwords aren't database-specific.
2992+
*/
2993+
if (replci->val == NULL ||
2994+
strcmp(ci->val, replci->val) != 0)
2995+
{
2996+
if (strcmp(replci->keyword, "user") == 0 ||
2997+
strcmp(replci->keyword, "host") == 0 ||
2998+
strcmp(replci->keyword, "hostaddr") == 0 ||
2999+
strcmp(replci->keyword, "port") == 0)
3000+
keep_password = false;
3001+
}
3002+
/* Also note whether connstring contains a password. */
3003+
if (strcmp(replci->keyword, "password") == 0)
3004+
have_password = true;
29833005
}
29843006
}
29853007
Assert(ci->keyword == NULL && replci->keyword == NULL);
@@ -2989,8 +3011,13 @@ do_connect(enum trivalue reuse_previous_specification,
29893011

29903012
PQconninfoFree(replcinfo);
29913013

2992-
/* We never re-use a password with a conninfo string. */
2993-
keep_password = false;
3014+
/*
3015+
* If the connstring contains a password, tell the loop below
3016+
* that we may use it, regardless of other settings (i.e.,
3017+
* cinfo's password is no longer an "old" password).
3018+
*/
3019+
if (have_password)
3020+
keep_password = true;
29943021

29953022
/* Don't let code below try to inject dbname into params. */
29963023
dbname = NULL;
@@ -3078,14 +3105,16 @@ do_connect(enum trivalue reuse_previous_specification,
30783105
*/
30793106
password = prompt_for_password(has_connection_string ? NULL : user);
30803107
}
3081-
else if (o_conn && keep_password)
3082-
{
3083-
password = PQpass(o_conn);
3084-
if (password && *password)
3085-
password = pg_strdup(password);
3086-
else
3087-
password = NULL;
3088-
}
3108+
3109+
/*
3110+
* Consider whether to force client_encoding to "auto" (overriding
3111+
* anything in the connection string). We do so if we have a terminal
3112+
* connection and there is no PGCLIENTENCODING environment setting.
3113+
*/
3114+
if (pset.notty || getenv("PGCLIENTENCODING"))
3115+
client_encoding = NULL;
3116+
else
3117+
client_encoding = "auto";
30893118

30903119
/* Loop till we have a connection or fail, which we might've already */
30913120
while (success)
@@ -3097,12 +3126,12 @@ do_connect(enum trivalue reuse_previous_specification,
30973126

30983127
/*
30993128
* Copy non-default settings into the PQconnectdbParams parameter
3100-
* arrays; but override any values specified old-style, as well as the
3101-
* password and a couple of fields we want to set forcibly.
3129+
* arrays; but inject any values specified old-style, as well as any
3130+
* interactively-obtained password, and a couple of fields we want to
3131+
* set forcibly.
31023132
*
31033133
* If you change this code, see also the initial-connection code in
3104-
* main(). For no good reason, a connection string password= takes
3105-
* precedence in main() but not here.
3134+
* main().
31063135
*/
31073136
for (ci = cinfo; ci->keyword; ci++)
31083137
{
@@ -3121,12 +3150,15 @@ do_connect(enum trivalue reuse_previous_specification,
31213150
}
31223151
else if (port && strcmp(ci->keyword, "port") == 0)
31233152
values[paramnum++] = port;
3124-
else if (strcmp(ci->keyword, "password") == 0)
3153+
/* If !keep_password, we unconditionally drop old password */
3154+
else if ((password || !keep_password) &&
3155+
strcmp(ci->keyword, "password") == 0)
31253156
values[paramnum++] = password;
31263157
else if (strcmp(ci->keyword, "fallback_application_name") == 0)
31273158
values[paramnum++] = pset.progname;
3128-
else if (strcmp(ci->keyword, "client_encoding") == 0)
3129-
values[paramnum++] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
3159+
else if (client_encoding &&
3160+
strcmp(ci->keyword, "client_encoding") == 0)
3161+
values[paramnum++] = client_encoding;
31303162
else if (ci->val)
31313163
values[paramnum++] = ci->val;
31323164
/* else, don't bother making libpq parse this keyword */

0 commit comments

Comments
 (0)