On Tue, May 22, 2018 at 6:14 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Well, the question that ought to be answered first is whether to do > anything at all, beyond not-crashing. It doesn't seem to me that > refusing to accept a password if we can't disable echo is a net win, > so I'm inclined to think it's okay to silently ignore failure to > turn off echo.
As a middle ground, I suppose you could print a warning message but not bail out of simple_prompt; that doesn't mean much for users, since echo only happens when you're actually typing something at a console, where echoing is obvious. The chief benefit is for developers: once somebody notices echoing hasn't been disabled, somebody's going to have to figure out why, and a good error message might make that easier. > The other aspect of this code that maybe needs consideration is the > silent fallback to use stdin/stderr if we can't open the console. > It seems like that could be a really bad idea if stdin is a pipe. > But on the other hand, maybe somebody is depending on it somewhere? > I'm not sure I'd want to back-patch a change in that behavior, anyway. In terms of the echoing, this isn't an issue: if there's no pipe, stdin is a wrapper around CONIN$ already, and if it's a pipe (or other redirect) echoing isn't an issue. It seems to be possible to trigger the fallback by doing hijinks with the console in a parent process and having psql inherit the now-restricted console. The MSDN docs say parent processes /should/ use sharing modes that would avoid this problem, but it's not actually enforced. Based on the current code, the fallback is necessary for things to work with terminal emulators like mintty on windows, because they operate by hooking child processes up to a pipe, since the terminal emulator isn't an actual console (windows doesn't have ptys). I don't think echo disabling works there, but psql doesn't seem to run in that environment at all.
