^D to credentials prompt results in "fatal: ... Success"

A bit of an amusing edge case.

I'm not exactly sure the correct approach to fix this but here's my reproduction, triage, and a few potential options I see.

Note that after the username prompt, I pressed ^D

$./prefix/bin/git --version git version 2.18.0 $ PATH=$PWD/prefix/bin:$PATH git clone https://github.com/asottile/this-does-not-exist-i-promise Cloning into 'this-does-not-exist-i-promise'... Username for 'https://github.com': fatal: could not read Username for 'https://github.com': Success

Looking at `prompt.c`, it's hitting this bit of code:

        if (git_env_bool("GIT_TERMINAL_PROMPT", 1)) {
            r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
            err = strerror(errno);
        } else {
            err = "terminal prompts disabled";

        }
        if (!r) {
            /* prompts already contain ": " at the end */
            die("could not read %s%s", prompt, err);
        }

From `git_terminal_prompt` (compat/terminal.c):

    r = strbuf_getline_lf(&buf, input_fh);
    if (!echo) {
        putc('\n', output_fh);
        fflush(output_fh);
    }

    restore_term();
    fclose(input_fh);
    fclose(output_fh);

    if (r == EOF)
        return NULL;
    return buf.buf;

in the `EOF` case, this is returning `NULL`, but `errno = 0` at this point causing the error string to be "Success"

I see a couple of options here:

1. special case EOF in `git_terminal_prompt` / `git_prompt` and
produce an error message such as:

fatal: could not read Username for 'https://github.com': EOF

(I tried returing `EOF` directly from `git_terminal_prompt` and was able to get this messaging to work, however `r == EOF` is a pointer-int comparison so this approach didn't really seem like a good idea -- changing the signature of `git_terminal_prompt` to set a special flag for EOF is another option, but seems a lot of work for a case that probably doesn't happen all too often)

I also couldn't find an appropriate errno to set in the EOF case either

2. treat EOF less specially

The function this is replacing, `getpass` simply returns an empty string on `EOF`. This patch would implement that:

On Fri, Jun 22, 2018 at 07:42:38PM -0700, Anthony Sottile wrote:

Yeah, agreed that is not ideal.

Either of those would be fine with me. We do not have to adhere to getpass() behavior exactly (the whole point of having our custom wrapper is that getpass() behaves badly in a few situations). But I doubt anybody really cares that much either way, so we might as well have consistent behavior.

I think this goes too far, though. We get EOF from strbuf_getline on actual EOF _or_ on a real error. And we'd want to keep reporting a real error, since it may tell the user something useful.

I suspect you probably need to check ferror(input_fh) before closing, and rewrite "r" to 0 in that case.

-Peff