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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:

diff --git a/compat/terminal.c b/compat/terminal.c
index fa13ee672..8bd08108e 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -122,7 +122,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
        fputs(prompt, output_fh);
        fflush(output_fh);

-       r = strbuf_getline_lf(&buf, input_fh);
+       strbuf_getline_lf(&buf, input_fh);
        if (!echo) {
                putc('\n', output_fh);
                fflush(output_fh);
@@ -132,8 +132,6 @@ char *git_terminal_prompt(const char *prompt, int echo)
        fclose(input_fh);
        fclose(output_fh);

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


however then the output is a bit strange for ^D (note I pressed ^D twice):

$ 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': Password for 'https://github.com':
remote: Repository not found.
fatal: Authentication failed for
'https://github.com/asottile/this-does-not-exist-i-promise/'

Anthony



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux