Patchwork ui: check EOF of getpass() response read from command-server channel

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 14, 2017, 1:56 p.m.
Message ID <7a50d3fb633a1ef3b804.1484402204@mimosa>
Download mbox | patch
Permalink /patch/18218/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 14, 2017, 1:56 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1484393495 -32400
#      Sat Jan 14 20:31:35 2017 +0900
# Node ID 7a50d3fb633a1ef3b80485a1947047898f1a24b7
# Parent  98bfce9bd5e53e88d1c94af5f24b0e91795e18a3
ui: check EOF of getpass() response read from command-server channel

readline() returns '' only when EOF is encountered, in which case, Python's
getpass() raises EOFError. We should do the same to abort the session as
"response expected."

This bug was reported to
https://bitbucket.org/tortoisehg/thg/issues/4659/
Augie Fackler - Jan. 14, 2017, 10:33 p.m.
> On Jan 14, 2017, at 8:56 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1484393495 -32400
> #      Sat Jan 14 20:31:35 2017 +0900
> # Node ID 7a50d3fb633a1ef3b80485a1947047898f1a24b7
> # Parent  98bfce9bd5e53e88d1c94af5f24b0e91795e18a3
> ui: check EOF of getpass() response read from command-server channel

Queued this, thanks.

> 
> readline() returns '' only when EOF is encountered, in which case, Python's
> getpass() raises EOFError. We should do the same to abort the session as
> "response expected."
> 
> This bug was reported to
> https://bitbucket.org/tortoisehg/thg/issues/4659/
> 
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -972,7 +972,10 @@ class ui(object):
>             # disable getpass() only if explicitly specified. it's still valid
>             # to interact with tty even if fin is not a tty.
>             if self.configbool('ui', 'nontty'):
> -                return self.fin.readline().rstrip('\n')
> +                l = self.fin.readline()
> +                if not l:
> +                    raise EOFError
> +                return l.rstrip('\n')
>             else:
>                 return getpass.getpass('')
>         except EOFError:
> diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t
> --- a/tests/test-commandserver.t
> +++ b/tests/test-commandserver.t
> @@ -607,6 +607,12 @@ changelog and manifest would have invali
>   ...     runcommand(server, ['debuggetpass', '--config',
>   ...                         'ui.interactive=True'],
>   ...                input=stringio('1234\n'))
> +  ...     runcommand(server, ['debuggetpass', '--config',
> +  ...                         'ui.interactive=True'],
> +  ...                input=stringio('\n'))
> +  ...     runcommand(server, ['debuggetpass', '--config',
> +  ...                         'ui.interactive=True'],
> +  ...                input=stringio(''))
>   ...     runcommand(server, ['debugprompt', '--config',
>   ...                         'ui.interactive=True'],
>   ...                input=stringio('5678\n'))
> @@ -614,6 +620,11 @@ changelog and manifest would have invali
>   ...     runcommand(server, ['debugwritestdout'])
>   *** runcommand debuggetpass --config ui.interactive=True
>   password: 1234
> +  *** runcommand debuggetpass --config ui.interactive=True
> +  password: 
> +  *** runcommand debuggetpass --config ui.interactive=True
> +  password: abort: response expected
> +   [255]
>   *** runcommand debugprompt --config ui.interactive=True
>   prompt: 5678
>   *** runcommand debugreadstdin
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -972,7 +972,10 @@  class ui(object):
             # disable getpass() only if explicitly specified. it's still valid
             # to interact with tty even if fin is not a tty.
             if self.configbool('ui', 'nontty'):
-                return self.fin.readline().rstrip('\n')
+                l = self.fin.readline()
+                if not l:
+                    raise EOFError
+                return l.rstrip('\n')
             else:
                 return getpass.getpass('')
         except EOFError:
diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t
--- a/tests/test-commandserver.t
+++ b/tests/test-commandserver.t
@@ -607,6 +607,12 @@  changelog and manifest would have invali
   ...     runcommand(server, ['debuggetpass', '--config',
   ...                         'ui.interactive=True'],
   ...                input=stringio('1234\n'))
+  ...     runcommand(server, ['debuggetpass', '--config',
+  ...                         'ui.interactive=True'],
+  ...                input=stringio('\n'))
+  ...     runcommand(server, ['debuggetpass', '--config',
+  ...                         'ui.interactive=True'],
+  ...                input=stringio(''))
   ...     runcommand(server, ['debugprompt', '--config',
   ...                         'ui.interactive=True'],
   ...                input=stringio('5678\n'))
@@ -614,6 +620,11 @@  changelog and manifest would have invali
   ...     runcommand(server, ['debugwritestdout'])
   *** runcommand debuggetpass --config ui.interactive=True
   password: 1234
+  *** runcommand debuggetpass --config ui.interactive=True
+  password: 
+  *** runcommand debuggetpass --config ui.interactive=True
+  password: abort: response expected
+   [255]
   *** runcommand debugprompt --config ui.interactive=True
   prompt: 5678
   *** runcommand debugreadstdin