Re: psql: default base and password reading

Actually, I see problems with both of these:

IIRC, template1 is used as the base whenever you create a new database.
Having psql connect to it by default greatly increases the likelyhood
of unintended modification of template1, which then gets propogated to
every other database subsequently created.

Personally, I'd be happier if psql attempted to create a database in
the name of the logged in user rather than ever give access to template1
to someone who didn't ask for it. However, I think requiring the database
name is a good idea in general.

The password reading patch may be useful, but I suggest that you
reverse the meaning of the option, so that you do not change the
current behavior. This would be a normal practice for backward
compatibility.

-Brad

* Adam Osuchowski (adwol(at)polsl(dot)gliwice(dot)pl) [011012 01:59]:
> Hello,
>
> in attachment I enclose patch on psql which fix two things:
>
> 1. If no database was given, template1 is assumed.
> 2. Password is reading from terminal (/dev/tty) rather than from stdin. It
> was usable when we want to pipe sql script on psql and have set password at
> the same time. It may be turned off, by pass -i option to psql. Then, psql
> behave normally, like it is currently.
>
> Sorry about increasing traffic, if it was already discussed, or it is solved
> in other way.
>
> --
> ## Adam Osuchowski adwol(at)polsl(dot)gliwice(dot)pl, adwol(at)silesia(dot)linux(dot)org(dot)pl
> ## Silesian University of Technology, Computer Centre Gliwice, Poland

> diff -ruN psql/common.c psql_patched/common.c
> --- psql/common.c Sun Apr 15 02:43:37 2001
> +++ psql_patched/common.c Sun Oct 7 14:58:22 2001
> @@ -176,6 +176,7 @@
> {
> int length;
> char *destination;
> + static FILE *term = NULL;
>
> #ifdef HAVE_TERMIOS_H
> struct termios t_orig,
> @@ -190,24 +191,28 @@
> fputs(prompt, stderr);
>
> prompt_state = true;
> + if (!term && pset.stdinPassword == false)
> + term = fopen("/dev/tty", "r");
> + if (term == NULL || pset.stdinPassword == true)
> + term = stdin;
>
> #ifdef HAVE_TERMIOS_H
> if (!echo)
> {
> - tcgetattr(0, &t);
> + tcgetattr(fileno(term), &t);
> t_orig = t;
> t.c_lflag &= ~ECHO;
> - tcsetattr(0, TCSADRAIN, &t);
> + tcsetattr(fileno(term), TCSADRAIN, &t);
> }
> #endif
>
> - if (fgets(destination, maxlen, stdin) == NULL)
> + if (fgets(destination, maxlen, term) == NULL)
> destination[0] = '\0';
>
> #ifdef HAVE_TERMIOS_H
> if (!echo)
> {
> - tcsetattr(0, TCSADRAIN, &t_orig);
> + tcsetattr(fileno(term), TCSADRAIN, &t_orig);
> fputs("\n", stderr);
> }
> #endif
> @@ -223,7 +228,7 @@
>
> do
> {
> - if (fgets(buf, sizeof(buf), stdin) == NULL)
> + if (fgets(buf, sizeof(buf), term) == NULL)
> break;
> buflen = strlen(buf);
> } while (buflen > 0 && buf[buflen - 1] != '\n');
> diff -ruN psql/help.c psql_patched/help.c
> --- psql/help.c Thu Mar 22 05:00:20 2001
> +++ psql_patched/help.c Sun Oct 7 14:57:32 2001
> @@ -103,6 +103,7 @@
> puts(")");
>
> puts(" -H HTML table output mode (-P format=html)");
> + puts(" -i Read password from stdin rather than terminal");
> puts(" -l List available databases, then exit");
> puts(" -n Disable readline");
> puts(" -o <filename> Send query output to filename (or |pipe)");
> diff -ruN psql/settings.h psql_patched/settings.h
> --- psql/settings.h Wed Apr 12 19:16:23 2000
> +++ psql_patched/settings.h Sun Oct 7 14:49:16 2001
> @@ -46,6 +46,7 @@
>
> char *progname; /* in case you renamed psql */
> char *inputfile; /* for error reporting */
> + bool stdinPassword;
> unsigned lineno; /* also for error reporting */
>
> bool issuper; /* is the current user a superuser? (used
> diff -ruN psql/startup.c psql_patched/startup.c
> --- psql/startup.c Fri Mar 23 01:36:38 2001
> +++ psql_patched/startup.c Sun Oct 7 14:55:43 2001
> @@ -331,6 +331,7 @@
> {"field-separator", required_argument, NULL, 'F'},
> {"host", required_argument, NULL, 'h'},
> {"html", no_argument, NULL, 'H'},
> + {"stdin", no_argument, NULL, 'i'},
> {"list", no_argument, NULL, 'l'},
> {"no-readline", no_argument, NULL, 'n'},
> {"output", required_argument, NULL, 'o'},
> @@ -364,14 +365,14 @@
> memset(options, 0, sizeof *options);
>
> #ifdef HAVE_GETOPT_LONG
> - while ((c = getopt_long(argc, argv, "aAc:d:eEf:F:h:Hlno:p:P:qR:sStT:uU:v:VWxX?", long_options, &optindex)) != -1)
> + while ((c = getopt_long(argc, argv, "aAc:d:eEf:F:h:Hilno:p:P:qR:sStT:uU:v:VWxX?", long_options, &optindex)) != -1)
> #else /* not HAVE_GETOPT_LONG */
>
> /*
> * Be sure to leave the '-' in here, so we can catch accidental long
> * options.
> */
> - while ((c = getopt(argc, argv, "aAc:d:eEf:F:h:Hlno:p:P:qR:sStT:uU:v:VWxX?-")) != -1)
> + while ((c = getopt(argc, argv, "aAc:d:eEf:F:h:Hilno:p:P:qR:sStT:uU:v:VWxX?-")) != -1)
> #endif /* not HAVE_GETOPT_LONG */
> {
> switch (c)
> @@ -414,6 +415,9 @@
> case 'H':
> pset.popt.topt.format = PRINT_HTML;
> break;
> + case 'i':
> + pset.stdinPassword = true;
> + break;
> case 'l':
> options->action = ACT_LIST_DB;
> break;
> @@ -571,6 +575,8 @@
> if (used_old_u_option && !QUIET())
> fprintf(stderr, "%s: Warning: The -u option is deprecated. Use -U.\n", pset.progname);
>
> + if (!options->dbname)
> + options->dbname = "template1";
> }
>
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly