classification
Title: getpass.getpass does not respond to ctrl-c or ctrl-z
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: asvetlov, belopolsky, benjamin.peterson, eric.araujo, gregory.p.smith, gvanrossum, loewis, orsenthil, pajs@fodder.org.uk, python-dev, sdaoden, valhallasw
Priority: normal Keywords: needs review, patch

Created on 2011-02-17 20:56 by valhallasw, last changed 2011-04-26 13:20 by orsenthil. This issue is now closed.

Files
File name Uploaded Description Edit
issue11236.patch valhallasw, 2011-02-18 22:43 Patch - removes removal of ISIG flag
11236.depend.patch sdaoden, 2011-03-19 22:46
Messages (25)
msg128754 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-02-17 20:56
In python 2.5, entering ^C in a getpass prompt yields a KeyboardInterrupt. In later versions, nothing happens, and '\x03' is returned after pressing return.
In python 2.5, entering ^Z in a getpass prompt suspends the process. In later versions, nothing happens, and '\x1a' is returned after pressing return.

Pressing ctrl-C or ctrl-Z multiple times returns '\x03'/'\x1a' multiple times.

Tested versions:
Python 2.5.4 (r254:67916, Jan 20 2010, 21:44:03) [GCC 4.3.3] on linux2 (OK)
  getpass.py is http://svn.python.org/view/python/trunk/Lib/getpass.py?revision=43495&view=markup
Python 2.6.5 (r265:79063, Apr 16 2010, 13:09:56) [GCC 4.4.3] on linux2 (BAD)
  getpass.py is r76000 with the following diff:
54c54
<         except (AttributeError, ValueError):
---
>         except:
 (so it doesn't implement r74860)
Python 2.7.1 (r271:86832, Jan  4 2011, 13:57:14) [GCC 4.5.2] on sunos5 (BAD)
Python 3.1.2 (r312:79147, Sep 27 2010, 09:45:41) [GCC 4.4.3] on linux2 (BAD)
Python 3.1.3 (r313:86834, Dec  1 2010, 20:16:39) [GCC 4.5.1] on sunos5 (BAD)

Steps to reproduce:
1) import getpass
2) getpass.getpass('-> ')
3) press ctrl-C or ctrl-Z
- nothing happens
4) press return
- '\x03'/'\x1a' is returned
msg128755 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-02-17 21:09
To allow people to cancel at the password prompt, we added a manual post-check. Although this check runs after return is pressed, it's better than nothing.

Index: branches/rewrite/pywikibot/userinterfaces/terminal_interface.py
===================================================================
--- branches/rewrite/pywikibot/userinterfaces/terminal_interface.py	(revision 8977)
+++ branches/rewrite/pywikibot/userinterfaces/terminal_interface.py	(revision 8978)
@@ -175,6 +175,11 @@
             if password:
                 import getpass
                 text = getpass.getpass('')
+                # See PYWP-13 / http://bugs.python.org/issue11236
+                # getpass does not always raise an KeyboardInterrupt when ^C
+                # is pressed.
+                if '\x03' in text:
+                    raise KeyboardInterrupt()
             else:
                 text = raw_input()
         finally:

http://www.mediawiki.org/wiki/Special:Code/pywikipedia/8978
msg128807 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-18 19:39
Thanks for the report.  Would you like to contribute a patch?  If so, guidelines are found at http://docs.python.org/devguide/
msg128820 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-02-18 22:43
Sorry, I'm not going to run my patch through the entire test suite, I've got better things to do with my time than setting up a working python-development-test-bench. Especially for a one-line-revert.

The result now is:

valhallasw@dorthonion:~/src/pythonpatch$ python -c "import getpass; print getpass.getpass().__repr__()"
Password: Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "getpass.py", line 71, in unix_getpass
    passwd = _raw_input(prompt, stream, input=input)
  File "getpass.py", line 133, in _raw_input
    line = input.readline()
KeyboardInterrupt


As for a patch - see attachment. It reverts one change from r76000
http://svn.python.org/view/python/trunk/Lib/getpass.py?r1=74860&r2=76000&pathrev=76000 line 65

The ISIG flag is copied from getpass.c (http://www.koders.com/c/fid3C9D79A0C31256E7875CB8930CF8B9E49BDA8C12.aspx line 122). According to the r76000 commit message:

"This change also incorporates some additional getpass implementation
suggestions for security based on an analysis of getpass.c linked to from the issue."
'The issue' should probably be issue7208, but I cannot find any reference to getpass.c there.
msg128821 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-18 22:49
Thanks for the patch.  Someone will write a test and we’ll check that it works correctly on all supported platforms.
msg128822 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-18 22:50
BTW, on which platforms did you notice the bug?  (python -m platform)
msg128823 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-02-18 22:52
Linux-2.6.22.18-co-0.7.4-i686-with-Ubuntu-10.04-lucid (the 'on linux2' versions) and Solaris-2.10-i86pc-i386-32bit-ELF (the 'on sunos5').
msg128824 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-18 22:55
Patch fixes the bug for 3.2 on my Debian (Linux-2.6.32-5-amd64-x86_64-with-debian-wheezy-sid).  getpass currently has no unit tests.
msg129561 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-02-26 15:34
I fail to see the bug here. Python's getpass.py is mimicing the                                                                                                        
behavior of getpass.c                                                                                                                                                  
                                                                                                                                                                       
Your patch tries to remove, termios.ISIG , but it was specifically                                                                                                     
added so that getpass.py is closer to the POSIX's getpass                                                                                                              
                                                                                                                                                                       
http://mail.python.org/pipermail/python-dev/2003-December/040579.html                                                                                                  
                                                                                                                                                                       
Also, try an example program which uses getpass.c                                                                                                                      
http://www.koders.com/c/fidFB8B9443522DCC3CC53ECCC0FFED861FE898F3A1.aspx?s=%22unknown%22                                                                               
                                                                                                                                                                       
And try ctrl-c and ctrl-z for those, the behavior is same what you                                                                                                     
decribe in this bug-report.                                                                                                                                            
                                                                                                                                                                       
So, having the function behavior sames as POSIX one is really not a                                                                                                    
bug and just a change from the previous behavior. Would you agree to                                                                                                   
this stance and shall we close the bug?
msg129564 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-02-26 15:59
No, I do not, for several reasons.

First of all, this is not a change *from* previous behaviour, but a change *back to* previous behaviour. And sensible behaviour, too.

Secondly, I have tested what getpass does on windows (Python 2.7.1 (r271:86832, Nov 27 2010, 18:30:46) [MSC v.1500 32 bit (Intel)] on
win32). There, I get a KeyboardInterrupt when pressing ctrl-c. So for the simple reason of having consistend cross-platform behaviour, either the linux or the windows implementation should change.

Thirdly, the reason for adding the ISIG flag was that it is 'a possible security hole' - but no-one actually mentions *what* that hole would be. As Guido notes (in that same thread!): "Sorry, but this just doesn't make sense.  There are so many differences between C and Python that you can't just compare a C and a Python version of a function and pointing at the differences as possible security holes or bugs.".

Lastly, I see no reason to mimic POSIX behaviour per se. Why not use the POSIX getpass function in the first place, then? 

By the way - even with ISIG on, it should be possible to support KeyboardInterrupts - just read per-character instead of per-line (cf. the windows getpass).

[1] http://mail.python.org/pipermail/python-dev/2003-December/040583.html
msg130567 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-03-11 13:20
I do consider this as a bug, too (you hit ^C - and it doesn't help!)!
I've opened #11466 due to a different problem, but since raising
(the expected) KeyboardInterrupt would trigger that problem, too,
i've included stripping termios.ISIG in file21081.
msg131414 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-19 13:39
If this bugs needs to be solved before #11466 can be fixed, let’s mark it as a dependency.
msg131454 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-03-19 22:46
The attached 11236.depend.patch updates Merlijn's patch so that it 
applies cleanly just in case the #11466.5.patch is used in the end.
msg131962 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-03-24 10:30
By the way, in another thread i've seen a link to issue960406, 
where Guido van Rossum states (in msg46074):

    Ideally, ^C should always cause the signal handler for
    SIGINT to be called, and the KeyboardInterrupt should be
    generated by the default SIGINT handler

Thus removing ISIG contradicts approaches Python has chosen to 
use a long time ago (tracks get lost, as always). 
Just in case anybody is not convinced yet that ISIG has to go.
msg131977 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-03-24 14:19
Agree to removing of termios.ISIG so that we get a KeyBoardInterrupt exception raised when CNTL-C is pressed. Looking at discussion more carefully, it does not present any security risk.

Should this be fixed in 3.3 only with NEWS detailing the change in behavior (back to old 2.5 behavior) or should be this be backported?

Close similarity with getpass.c 's behavior had lent some to support to this change in 2.6. Changing now in older codeline has some chances of breaking others code.

Someone who has been affected by this change in behavior should provide some insights if back-porting would make sense.
msg131978 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-24 14:27
New changeset c177faafec51 by Senthil Kumaran in branch 'default':
issue11236 getpass.getpass to respond ctrl-c or ctrl-z
http://hg.python.org/cpython/rev/c177faafec51
msg131997 - (view) Author: Merlijn van Deen (valhallasw) * Date: 2011-03-24 16:12
@orsenthil
> Close similarity with getpass.c 's behavior had lent some to support to this change in 2.6. Changing now in older codeline has some chances of breaking others code.
> Someone who has been affected by this change in behavior should provide some insights if back-porting would make sense.

Most code will probably have been updated their getpass code with a line like

if '\x03' in text:
  raise KeyboardInterrupt()

( http://www.mediawiki.org/wiki/Special:Code/pywikipedia/8978 )

However, people might have changed their code from
try:
  pass = getpass.getpass()
except KeyboardInterrupt:
  print "Ctrl-C!"

to:
pass = getpass.getpass()
if "\x03' in pass:
  print "Ctrl-C!"

which will break with this change. The first workaround makes more sense, though, so I suspect very little code will be broken by reverting the ISIG flag.

Overall, I think most people are not aware of the removal, either because they still use python 2.5 or because they don't press ctrl-c. They are still in for a surprise if the ISIG flag is not removed (although it will probably stay in the 2.6 branch, anyway?).
msg132214 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2011-03-26 03:46
Is there any reason to assume this is not a bug? I think it should be fixed in all versions.
msg132269 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-03-26 19:49
Greg: Can you please comment what the original motivation was for removing ISIG from the flags? This is your change, from issue 7208, AFAICT.
msg134458 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-26 13:01
New changeset 154b323e0e7f by Senthil Kumaran in branch '3.1':
Fix for issue11236 getpass.getpass to respond ctrl-c or ctrl-z
http://hg.python.org/cpython/rev/154b323e0e7f
msg134460 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-26 13:03
New changeset a3b4887edba4 by Senthil Kumaran in branch '2.7':
issue11236 getpass.getpass to respond ctrl-c or ctrl-z
http://hg.python.org/cpython/rev/a3b4887edba4
msg134461 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-04-26 13:08
I have made the changes in 3.3,3.2,3.1 and 2.7 codeline. The behavior is aligned with the 2.5 (and earlier) behaviors. I cannot change this 2.6 because this is not a security issue to be back-ported. (rather it was misinterpreted problem and resulted in changed behavior in 2.6). 

I shall update this in README file just that people note this change
msg134462 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-26 13:14
New changeset f799530dbde7 by Senthil Kumaran in branch '3.1':
Update News entry for Issue11236
http://hg.python.org/cpython/rev/f799530dbde7
msg134463 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-26 13:18
New changeset 1b261f3bef09 by Senthil Kumaran in branch '2.7':
Update NEWS for Issue11236.
http://hg.python.org/cpython/rev/1b261f3bef09
msg134464 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-04-26 13:20
Updated the NEWS entry.
History
Date User Action Args
2011-04-26 13:20:00orsenthilsetstatus: open -> closed

messages: + msg134464
2011-04-26 13:18:33python-devsetmessages: + msg134463
2011-04-26 13:14:45python-devsetmessages: + msg134462
2011-04-26 13:08:24orsenthilsettype: behavior
resolution: fixed
messages: + msg134461
stage: test needed -> resolved
2011-04-26 13:03:05python-devsetmessages: + msg134460
2011-04-26 13:01:32python-devsetmessages: + msg134458
2011-03-26 19:58:02pitrousetnosy: - pitrou
2011-03-26 19:49:14loewissetnosy: + loewis
messages: + msg132269
2011-03-26 03:46:03gvanrossumsetnosy: + gvanrossum
messages: + msg132214
2011-03-24 16:12:16valhallaswsetmessages: + msg131997
2011-03-24 14:27:17python-devsetnosy: + python-dev
messages: + msg131978
2011-03-24 14:19:15orsenthilsetassignee: orsenthil
messages: + msg131977
2011-03-24 10:30:17sdaodensetmessages: + msg131962
2011-03-24 04:32:06asvetlovsetnosy: + asvetlov
2011-03-19 22:46:33sdaodensetfiles: + 11236.depend.patch

messages: + msg131454
nosy: gregory.p.smith, belopolsky, orsenthil, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw, sdaoden
2011-03-19 13:39:00eric.araujosetnosy: gregory.p.smith, belopolsky, orsenthil, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw, sdaoden
messages: + msg131414
2011-03-19 13:37:32eric.araujosetnosy: gregory.p.smith, belopolsky, orsenthil, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw, sdaoden
messages: - msg130613
2011-03-11 21:13:24eric.araujosetnosy: gregory.p.smith, belopolsky, orsenthil, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw, sdaoden
messages: + msg130613
2011-03-11 13:20:46sdaodensetnosy: + sdaoden
messages: + msg130567
2011-02-26 15:59:07valhallaswsetnosy: gregory.p.smith, belopolsky, orsenthil, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw
messages: + msg129564
2011-02-26 15:34:34orsenthilsetnosy: + orsenthil
messages: + msg129561
2011-02-18 22:55:08eric.araujosetnosy: gregory.p.smith, belopolsky, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw
messages: + msg128824
2011-02-18 22:52:30valhallaswsetnosy: gregory.p.smith, belopolsky, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw
messages: + msg128823
2011-02-18 22:50:18eric.araujosetnosy: gregory.p.smith, belopolsky, pitrou, benjamin.peterson, pajs@fodder.org.uk, eric.araujo, valhallasw
messages: + msg128822
2011-02-18 22:49:18eric.araujosetnosy: + gregory.p.smith, pitrou, belopolsky, pajs@fodder.org.uk
messages: + msg128821

keywords: + needs review
stage: needs patch -> test needed
2011-02-18 22:43:42valhallaswsetfiles: + issue11236.patch

messages: + msg128820
keywords: + patch
nosy: benjamin.peterson, eric.araujo, valhallasw
2011-02-18 19:39:23eric.araujosetnosy: + eric.araujo
messages: + msg128807
2011-02-18 02:01:58r.david.murraysetnosy: + benjamin.peterson
2011-02-17 22:49:13eric.araujosetstage: needs patch
versions: + Python 3.2, Python 3.3, - Python 2.6
2011-02-17 21:09:12valhallaswsetmessages: + msg128755
2011-02-17 20:56:28valhallaswcreate