Re: BUG #15827: Unable to connect on Windows using pg_services.conf using Python psycopg2

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Thu, Jun 27, 2019 at 12:20:35PM -0400, Tom Lane wrote:
>> For point (2), I looked through all other fgets() callers in our code.
>> Not all of them have newline-chomping logic, but I made all the ones
>> that do have such do it the same way (except for those that use the
>> isspace() method, which is fine). I'm not sure if this is fixing any
>> live bugs --- most of these places are reading popen() output, and
>> it's unclear to me whether we can rely on that to suppress \r on
>> Windows. The Windows-specific code in pipe_read_line seems to think
>> not (but if its test were dead code we wouldn't know it); yet if this
>> were a hazard you'd think we'd have gotten complaints about at least
>> one of these places. Still, I dislike code that has three or four
>> randomly different ways of doing the exact same thing, especially when
>> some of them are gratuitously inefficient.

> The part for pg_resetwal does not seem necessary to me. The file gets
> written by initdb in binary mode, so there should never be a CR,
> right? Or is it worth worrying about custom tools writing the file,
> which makes no actual sense normally?

Well, as I said, some of these changes may not be fixing live bugs.
The idea was just to make all of these code fragments look alike,
rather than guess about which ones might be exposed to the issue.
If we try to filter \r only where really necessary, we're going to
make mistakes down the line because somebody will copy-and-paste
the wrong version of this logic.

>> Note that I standardized on a loop that chomps trailing \r and \n
>> indiscriminately, not the "if chomp \n then chomp \r" approach we
>> had in some places. I think that approach does have a corner-case
>> bug: if the input is long enough that the \r fits into the buffer
>> but the \n doesn't, it'd fail to chomp the \r.

> That would basically break a bunch of cases where \r is used in
> strings, no, like passwords for single_prompt()?

Huh? This is dealing with input only, not output.

> I really think that
> we should stick with the approach of only removing \r when it is
> followed by \n as we basically want to be able to counter the text
> mode of Windows when something external wrote files read by our code,
> where \n has been magically transformed to \r\n.

As I said, I'm not convinced that filtering \r only where it's actually
adjacent to \n is sufficient, even on Windows. To suppose that it is
sufficient, you'd have to assume that fgets() guarantees not to split
the \r and \n across buffer boundaries, which I doubt that it does.
(If it does do that, it would break some other assumptions we have about
whether the buffer gets filled completely.)

regards, tom lane