Simplify correct use of simple_prompt().

The previous API for this function had it returning a malloc'd string.
That meant that callers had to check for NULL return, which few of them
were doing, and it also meant that callers had to remember to free()
the string later, which required extra logic in most cases.

Instead, make simple_prompt() write into a buffer supplied by the caller.
Anywhere that the maximum required input length is reasonably small,
which is almost all of the callers, we can just use a local or static
array as the buffer instead of dealing with malloc/free.

A fair number of callers used "pointer == NULL" as a proxy for "haven't
requested the password yet".  Maintaining the same behavior requires
adding a separate boolean flag for that, which adds back some of the
complexity we save by removing free()s.  Nonetheless, this nets out
at a small reduction in overall code size, and considerably less code
than we would have had if we'd added the missing NULL-return checks
everywhere they were needed.

In passing, clean up the API comment for simple_prompt() and get rid
of a very-unnecessary malloc/free in its Windows code path.

This is nominally a bug fix, but it does not seem worth back-patching,
because the actual risk of an OOM failure in any of these places seems
pretty tiny, and all of them are client-side not server-side anyway.

This patch is by me, but it owes a great deal to Michael Paquier
who identified the problem and drafted a patch for fixing it the
other way.

Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
This commit is contained in:
Tom Lane 2016-08-30 17:02:02 -04:00
parent 37f6fd1eaa
commit 9daec77e16
15 changed files with 142 additions and 146 deletions

View File

@ -261,7 +261,8 @@ PGconn *
sql_conn(struct options * my_opts)
{
PGconn *conn;
char *password = NULL;
bool have_password = false;
char password[100];
bool new_pass;
/*
@ -282,7 +283,7 @@ sql_conn(struct options * my_opts)
keywords[2] = "user";
values[2] = my_opts->username;
keywords[3] = "password";
values[3] = password;
values[3] = have_password ? password : NULL;
keywords[4] = "dbname";
values[4] = my_opts->dbname;
keywords[5] = "fallback_application_name";
@ -302,17 +303,15 @@ sql_conn(struct options * my_opts)
if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
password == NULL)
!have_password)
{
PQfinish(conn);
password = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);
if (password)
free(password);
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{

View File

@ -65,13 +65,17 @@ vacuumlo(const char *database, const struct _param * param)
long matched;
long deleted;
int i;
static char *password = NULL;
bool new_pass;
bool success = true;
static bool have_password = false;
static char password[100];
/* Note: password can be carried over from a previous call */
if (param->pg_prompt == TRI_YES && password == NULL)
password = simple_prompt("Password: ", 100, false);
if (param->pg_prompt == TRI_YES && !have_password)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
/*
* Start the connection. Loop until we have a password if requested by
@ -91,7 +95,7 @@ vacuumlo(const char *database, const struct _param * param)
keywords[2] = "user";
values[2] = param->pg_user;
keywords[3] = "password";
values[3] = password;
values[3] = have_password ? password : NULL;
keywords[4] = "dbname";
values[4] = database;
keywords[5] = "fallback_application_name";
@ -110,11 +114,12 @@ vacuumlo(const char *database, const struct _param * param)
if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
password == NULL &&
!have_password &&
param->pg_prompt != TRI_NO)
{
PQfinish(conn);
password = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);

View File

@ -1557,8 +1557,8 @@ setup_auth(FILE *cmdfd)
static void
get_su_pwd(void)
{
char *pwd1,
*pwd2;
char pwd1[100];
char pwd2[100];
if (pwprompt)
{
@ -1567,14 +1567,13 @@ get_su_pwd(void)
*/
printf("\n");
fflush(stdout);
pwd1 = simple_prompt("Enter new superuser password: ", 100, false);
pwd2 = simple_prompt("Enter it again: ", 100, false);
simple_prompt("Enter new superuser password: ", pwd1, sizeof(pwd1), false);
simple_prompt("Enter it again: ", pwd2, sizeof(pwd2), false);
if (strcmp(pwd1, pwd2) != 0)
{
fprintf(stderr, _("Passwords didn't match.\n"));
exit_nicely();
}
free(pwd2);
}
else
{
@ -1587,7 +1586,6 @@ get_su_pwd(void)
* for now.
*/
FILE *pwf = fopen(pwfilename, "r");
char pwdbuf[MAXPGPATH];
int i;
if (!pwf)
@ -1596,7 +1594,7 @@ get_su_pwd(void)
progname, pwfilename, strerror(errno));
exit_nicely();
}
if (!fgets(pwdbuf, sizeof(pwdbuf), pwf))
if (!fgets(pwd1, sizeof(pwd1), pwf))
{
if (ferror(pwf))
fprintf(stderr, _("%s: could not read password from file \"%s\": %s\n"),
@ -1608,15 +1606,12 @@ get_su_pwd(void)
}
fclose(pwf);
i = strlen(pwdbuf);
while (i > 0 && (pwdbuf[i - 1] == '\r' || pwdbuf[i - 1] == '\n'))
pwdbuf[--i] = '\0';
pwd1 = pg_strdup(pwdbuf);
i = strlen(pwd1);
while (i > 0 && (pwd1[i - 1] == '\r' || pwd1[i - 1] == '\n'))
pwd1[--i] = '\0';
}
superuser_password = pwd1;
superuser_password = pg_strdup(pwd1);
}
/*

View File

@ -2,3 +2,4 @@
CATALOG_NAME = pg_basebackup
AVAIL_LANGUAGES = de es fr it ko pl pt_BR ru zh_CN
GETTEXT_FILES = pg_basebackup.c pg_receivexlog.c pg_recvlogical.c receivelog.c streamutil.c ../../common/fe_memutils.c
GETTEXT_TRIGGERS = simple_prompt

View File

@ -41,7 +41,8 @@ char *dbport = NULL;
char *replication_slot = NULL;
char *dbname = NULL;
int dbgetpassword = 0; /* 0=auto, -1=never, 1=always */
static char *dbpassword = NULL;
static bool have_password = false;
static char password[100];
PGconn *conn = NULL;
/*
@ -141,24 +142,23 @@ GetConnection(void)
}
/* If -W was given, force prompt for password, but only the first time */
need_password = (dbgetpassword == 1 && dbpassword == NULL);
need_password = (dbgetpassword == 1 && !have_password);
do
{
/* Get a new password if appropriate */
if (need_password)
{
if (dbpassword)
free(dbpassword);
dbpassword = simple_prompt(_("Password: "), 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
need_password = false;
}
/* Use (or reuse, on a subsequent connection) password if we have it */
if (dbpassword)
if (have_password)
{
keywords[i] = "password";
values[i] = dbpassword;
values[i] = password;
}
else
{

View File

@ -134,6 +134,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
const char *newdb;
const char *newuser;
char *password;
char passbuf[100];
bool new_pass;
if (!reqdb)
@ -149,13 +150,12 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
ahlog(AH, 1, "connecting to database \"%s\" as user \"%s\"\n",
newdb, newuser);
password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL;
password = AH->savedPassword;
if (AH->promptPassword == TRI_YES && password == NULL)
{
password = simple_prompt("Password: ", 100, false);
if (password == NULL)
exit_horribly(modulename, "out of memory\n");
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
initPQExpBuffer(&connstr);
@ -201,16 +201,14 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
fprintf(stderr, "Connecting to %s as %s\n",
newdb, newuser);
if (password)
free(password);
if (AH->promptPassword != TRI_NO)
password = simple_prompt("Password: ", 100, false);
{
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
else
exit_horribly(modulename, "connection needs password\n");
if (password == NULL)
exit_horribly(modulename, "out of memory\n");
new_pass = true;
}
} while (new_pass);
@ -225,8 +223,6 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
free(AH->savedPassword);
AH->savedPassword = pg_strdup(PQpass(newConn));
}
if (password)
free(password);
termPQExpBuffer(&connstr);
@ -258,18 +254,18 @@ ConnectDatabase(Archive *AHX,
{
ArchiveHandle *AH = (ArchiveHandle *) AHX;
char *password;
char passbuf[100];
bool new_pass;
if (AH->connection)
exit_horribly(modulename, "already connected to a database\n");
password = AH->savedPassword ? pg_strdup(AH->savedPassword) : NULL;
password = AH->savedPassword;
if (prompt_password == TRI_YES && password == NULL)
{
password = simple_prompt("Password: ", 100, false);
if (password == NULL)
exit_horribly(modulename, "out of memory\n");
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
}
AH->promptPassword = prompt_password;
@ -309,9 +305,8 @@ ConnectDatabase(Archive *AHX,
prompt_password != TRI_NO)
{
PQfinish(AH->connection);
password = simple_prompt("Password: ", 100, false);
if (password == NULL)
exit_horribly(modulename, "out of memory\n");
simple_prompt("Password: ", passbuf, sizeof(passbuf), false);
password = passbuf;
new_pass = true;
}
} while (new_pass);
@ -332,8 +327,6 @@ ConnectDatabase(Archive *AHX,
free(AH->savedPassword);
AH->savedPassword = pg_strdup(PQpass(AH->connection));
}
if (password)
free(password);
/* check for version mismatch */
_check_database_version(AH);

View File

@ -1884,13 +1884,17 @@ connectDatabase(const char *dbname, const char *connection_string,
bool new_pass;
const char *remoteversion_str;
int my_version;
static char *password = NULL;
const char **keywords = NULL;
const char **values = NULL;
PQconninfoOption *conn_opts = NULL;
static bool have_password = false;
static char password[100];
if (prompt_password == TRI_YES && !password)
password = simple_prompt("Password: ", 100, false);
if (prompt_password == TRI_YES && !have_password)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
/*
* Start the connection. Loop until we have a password if requested by
@ -1970,7 +1974,7 @@ connectDatabase(const char *dbname, const char *connection_string,
values[i] = pguser;
i++;
}
if (password)
if (have_password)
{
keywords[i] = "password";
values[i] = password;
@ -1998,11 +2002,12 @@ connectDatabase(const char *dbname, const char *connection_string,
if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
password == NULL &&
!have_password &&
prompt_password != TRI_NO)
{
PQfinish(conn);
password = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);

View File

@ -773,8 +773,9 @@ static PGconn *
doConnect(void)
{
PGconn *conn;
static char *password = NULL;
bool new_pass;
static bool have_password = false;
static char password[100];
/*
* Start the connection. Loop until we have a password if requested by
@ -794,7 +795,7 @@ doConnect(void)
keywords[2] = "user";
values[2] = login;
keywords[3] = "password";
values[3] = password;
values[3] = have_password ? password : NULL;
keywords[4] = "dbname";
values[4] = dbName;
keywords[5] = "fallback_application_name";
@ -815,10 +816,11 @@ doConnect(void)
if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
password == NULL)
!have_password)
{
PQfinish(conn);
password = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);

View File

@ -1089,11 +1089,11 @@ exec_command(const char *cmd,
/* \password -- set user password */
else if (strcmp(cmd, "password") == 0)
{
char *pw1;
char *pw2;
char pw1[100];
char pw2[100];
pw1 = simple_prompt("Enter new password: ", 100, false);
pw2 = simple_prompt("Enter it again: ", 100, false);
simple_prompt("Enter new password: ", pw1, sizeof(pw1), false);
simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
if (strcmp(pw1, pw2) != 0)
{
@ -1139,9 +1139,6 @@ exec_command(const char *cmd,
if (opt0)
free(opt0);
}
free(pw1);
free(pw2);
}
/* \prompt -- prompt and set variable */
@ -1173,7 +1170,10 @@ exec_command(const char *cmd,
opt = arg1;
if (!pset.inputfile)
result = simple_prompt(prompt_text, 4096, true);
{
result = (char *) pg_malloc(4096);
simple_prompt(prompt_text, result, 4096, true);
}
else
{
if (prompt_text)
@ -1747,20 +1747,19 @@ exec_command(const char *cmd,
static char *
prompt_for_password(const char *username)
{
char *result;
char buf[100];
if (username == NULL)
result = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", buf, sizeof(buf), false);
else
{
char *prompt_text;
prompt_text = psprintf(_("Password for user %s: "), username);
result = simple_prompt(prompt_text, 100, false);
simple_prompt(prompt_text, buf, sizeof(buf), false);
free(prompt_text);
}
return result;
return pg_strdup(buf);
}
static bool

View File

@ -103,7 +103,8 @@ main(int argc, char *argv[])
{
struct adhoc_opts options;
int successResult;
char *password = NULL;
bool have_password = false;
char password[100];
char *password_prompt = NULL;
bool new_pass;
@ -210,7 +211,10 @@ main(int argc, char *argv[])
options.username);
if (pset.getPassword == TRI_YES)
password = simple_prompt(password_prompt, 100, false);
{
simple_prompt(password_prompt, password, sizeof(password), false);
have_password = true;
}
/* loop until we have a password if requested by backend */
do
@ -226,7 +230,7 @@ main(int argc, char *argv[])
keywords[2] = "user";
values[2] = options.username;
keywords[3] = "password";
values[3] = password;
values[3] = have_password ? password : NULL;
keywords[4] = "dbname"; /* see do_connect() */
values[4] = (options.list_dbs && options.dbname == NULL) ?
"postgres" : options.dbname;
@ -244,16 +248,16 @@ main(int argc, char *argv[])
if (PQstatus(pset.db) == CONNECTION_BAD &&
PQconnectionNeedsPassword(pset.db) &&
password == NULL &&
!have_password &&
pset.getPassword != TRI_NO)
{
PQfinish(pset.db);
password = simple_prompt(password_prompt, 100, false);
simple_prompt(password_prompt, password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);
free(password);
free(password_prompt);
if (PQstatus(pset.db) == CONNECTION_BAD)

View File

@ -68,18 +68,18 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
const char *progname, bool fail_ok, bool allow_password_reuse)
{
PGconn *conn;
static char *password = NULL;
bool new_pass;
static bool have_password = false;
static char password[100];
if (!allow_password_reuse)
{
if (password)
free(password);
password = NULL;
}
have_password = false;
if (password == NULL && prompt_password == TRI_YES)
password = simple_prompt("Password: ", 100, false);
if (!have_password && prompt_password == TRI_YES)
{
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
/*
* Start the connection. Loop until we have a password if requested by
@ -97,7 +97,7 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
keywords[2] = "user";
values[2] = pguser;
keywords[3] = "password";
values[3] = password;
values[3] = have_password ? password : NULL;
keywords[4] = "dbname";
values[4] = dbname;
keywords[5] = "fallback_application_name";
@ -123,9 +123,8 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
prompt_password != TRI_NO)
{
PQfinish(conn);
if (password)
free(password);
password = simple_prompt("Password: ", 100, false);
simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
new_pass = true;
}
} while (new_pass);
@ -275,22 +274,15 @@ yesno_prompt(const char *question)
for (;;)
{
char *resp;
char resp[10];
resp = simple_prompt(prompt, 1, true);
simple_prompt(prompt, resp, sizeof(resp), true);
if (strcmp(resp, _(PG_YESLETTER)) == 0)
{
free(resp);
return true;
}
else if (strcmp(resp, _(PG_NOLETTER)) == 0)
{
free(resp);
if (strcmp(resp, _(PG_NOLETTER)) == 0)
return false;
}
free(resp);
printf(_("Please answer \"%s\" or \"%s\".\n"),
_(PG_YESLETTER), _(PG_NOLETTER));
}

View File

@ -66,6 +66,8 @@ main(int argc, char *argv[])
char *conn_limit = NULL;
bool pwprompt = false;
char *newpassword = NULL;
char newuser_buf[128];
char newpassword_buf[100];
/* Tri-valued variables. */
enum trivalue createdb = TRI_DEFAULT,
@ -188,7 +190,11 @@ main(int argc, char *argv[])
if (newuser == NULL)
{
if (interactive)
newuser = simple_prompt("Enter name of role to add: ", 128, true);
{
simple_prompt("Enter name of role to add: ",
newuser_buf, sizeof(newuser_buf), true);
newuser = newuser_buf;
}
else
{
if (getenv("PGUSER"))
@ -200,18 +206,17 @@ main(int argc, char *argv[])
if (pwprompt)
{
char *pw1,
*pw2;
char pw2[100];
pw1 = simple_prompt("Enter password for new role: ", 100, false);
pw2 = simple_prompt("Enter it again: ", 100, false);
if (strcmp(pw1, pw2) != 0)
simple_prompt("Enter password for new role: ",
newpassword_buf, sizeof(newpassword_buf), false);
simple_prompt("Enter it again: ", pw2, sizeof(pw2), false);
if (strcmp(newpassword_buf, pw2) != 0)
{
fprintf(stderr, _("Passwords didn't match.\n"));
exit(1);
}
newpassword = pw1;
free(pw2);
newpassword = newpassword_buf;
}
if (superuser == 0)

View File

@ -46,6 +46,7 @@ main(int argc, char *argv[])
enum trivalue prompt_password = TRI_DEFAULT;
bool echo = false;
bool interactive = false;
char dropuser_buf[128];
PQExpBufferData sql;
@ -108,7 +109,11 @@ main(int argc, char *argv[])
if (dropuser == NULL)
{
if (interactive)
dropuser = simple_prompt("Enter name of role to drop: ", 128, true);
{
simple_prompt("Enter name of role to drop: ",
dropuser_buf, sizeof(dropuser_buf), true);
dropuser = dropuser_buf;
}
else
{
fprintf(stderr, _("%s: missing required argument role name\n"), progname);

View File

@ -203,7 +203,8 @@ extern char *pgwin32_setlocale(int category, const char *locale);
#endif /* WIN32 */
/* Portable prompt handling */
extern char *simple_prompt(const char *prompt, int maxlen, bool echo);
extern void simple_prompt(const char *prompt, char *destination, size_t destlen,
bool echo);
#ifdef WIN32
#define PG_SIGNAL_COUNT 32

View File

@ -12,33 +12,31 @@
*
*-------------------------------------------------------------------------
*/
/*
* simple_prompt
*
* Generalized function especially intended for reading in usernames and
* password interactively. Reads from /dev/tty or stdin/stderr.
*
* prompt: The prompt to print
* maxlen: How many characters to accept
* echo: Set to false if you want to hide what is entered (for passwords)
*
* Returns a malloc()'ed string with the input (w/o trailing newline).
*/
#include "c.h"
#ifdef HAVE_TERMIOS_H
#include <termios.h>
#endif
extern char *simple_prompt(const char *prompt, int maxlen, bool echo);
char *
simple_prompt(const char *prompt, int maxlen, bool echo)
/*
* simple_prompt
*
* Generalized function especially intended for reading in usernames and
* passwords interactively. Reads from /dev/tty or stdin/stderr.
*
* prompt: The prompt to print, or NULL if none (automatically localized)
* destination: buffer in which to store result
* destlen: allocated length of destination
* echo: Set to false if you want to hide what is entered (for passwords)
*
* The input (without trailing newline) is returned in the destination buffer,
* with a '\0' appended.
*/
void
simple_prompt(const char *prompt, char *destination, size_t destlen, bool echo)
{
int length;
char *destination;
FILE *termin,
*termout;
@ -48,14 +46,10 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
#else
#ifdef WIN32
HANDLE t = NULL;
LPDWORD t_orig = NULL;
DWORD t_orig = 0;
#endif
#endif
destination = (char *) malloc(maxlen + 1);
if (!destination)
return NULL;
#ifdef WIN32
/*
@ -118,11 +112,10 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
if (!echo)
{
/* get a new handle to turn echo off */
t_orig = (LPDWORD) malloc(sizeof(DWORD));
t = GetStdHandle(STD_INPUT_HANDLE);
/* save the old configuration first */
GetConsoleMode(t, t_orig);
GetConsoleMode(t, &t_orig);
/* set to the new mode */
SetConsoleMode(t, ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
@ -136,7 +129,7 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
fflush(termout);
}
if (fgets(destination, maxlen + 1, termin) == NULL)
if (fgets(destination, destlen, termin) == NULL)
destination[0] = '\0';
length = strlen(destination);
@ -170,10 +163,9 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
if (!echo)
{
/* reset to the original console mode */
SetConsoleMode(t, *t_orig);
SetConsoleMode(t, t_orig);
fputs("\n", termout);
fflush(termout);
free(t_orig);
}
#endif
#endif
@ -183,6 +175,4 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
fclose(termin);
fclose(termout);
}
return destination;
}