From 16dbbae2981c96c7c9b1ae81e1708d54b08c10ac Mon Sep 17 00:00:00 2001 From: R David Murray Date: Wed, 10 Jul 2013 17:02:24 -0400 Subject: [PATCH] #18116: getpass no longer always falls back to stdin. Also fixes a resource warning that occurred when the fallback is taken. Patch by Serhiy Storchaka. (We couldn't figure out how to write tests for this.) --- Lib/getpass.py | 96 ++++++++++++++++++++++------------------ Lib/test/test_getpass.py | 20 ++++++--- Misc/NEWS | 4 ++ 3 files changed, 69 insertions(+), 51 deletions(-) diff --git a/Lib/getpass.py b/Lib/getpass.py index 6ec6c78824b..53c38b88975 100644 --- a/Lib/getpass.py +++ b/Lib/getpass.py @@ -15,7 +15,11 @@ On the Mac EasyDialogs.AskPassword is used, if available. # Guido van Rossum (Windows support and cleanup) # Gregory P. Smith (tty support & GetPassWarning) -import os, sys, warnings +import contextlib +import io +import os +import sys +import warnings __all__ = ["getpass","getuser","GetPassWarning"] @@ -38,53 +42,57 @@ def unix_getpass(prompt='Password: ', stream=None): Always restores terminal settings before returning. """ - fd = None - tty = None passwd = None - try: - # Always try reading and writing directly on the tty first. - fd = os.open('/dev/tty', os.O_RDWR|os.O_NOCTTY) - tty = os.fdopen(fd, 'w+', 1) - input = tty - if not stream: - stream = tty - except OSError as e: - # If that fails, see if stdin can be controlled. + with contextlib.ExitStack() as stack: try: - fd = sys.stdin.fileno() - except (AttributeError, ValueError): - passwd = fallback_getpass(prompt, stream) - input = sys.stdin - if not stream: - stream = sys.stderr - - if fd is not None: - passwd = None - try: - old = termios.tcgetattr(fd) # a copy to save - new = old[:] - new[3] &= ~termios.ECHO # 3 == 'lflags' - tcsetattr_flags = termios.TCSAFLUSH - if hasattr(termios, 'TCSASOFT'): - tcsetattr_flags |= termios.TCSASOFT + # Always try reading and writing directly on the tty first. + fd = os.open('/dev/tty', os.O_RDWR|os.O_NOCTTY) + tty = io.FileIO(fd, 'w+') + stack.enter_context(tty) + input = io.TextIOWrapper(tty) + stack.enter_context(input) + if not stream: + stream = input + except OSError as e: + # If that fails, see if stdin can be controlled. + stack.close() try: - termios.tcsetattr(fd, tcsetattr_flags, new) - passwd = _raw_input(prompt, stream, input=input) - finally: - termios.tcsetattr(fd, tcsetattr_flags, old) - stream.flush() # issue7208 - except termios.error: - if passwd is not None: - # _raw_input succeeded. The final tcsetattr failed. Reraise - # instead of leaving the terminal in an unknown state. - raise - # We can't control the tty or stdin. Give up and use normal IO. - # fallback_getpass() raises an appropriate warning. - del input, tty # clean up unused file objects before blocking - passwd = fallback_getpass(prompt, stream) + fd = sys.stdin.fileno() + except (AttributeError, ValueError): + fd = None + passwd = fallback_getpass(prompt, stream) + input = sys.stdin + if not stream: + stream = sys.stderr - stream.write('\n') - return passwd + if fd is not None: + try: + old = termios.tcgetattr(fd) # a copy to save + new = old[:] + new[3] &= ~termios.ECHO # 3 == 'lflags' + tcsetattr_flags = termios.TCSAFLUSH + if hasattr(termios, 'TCSASOFT'): + tcsetattr_flags |= termios.TCSASOFT + try: + termios.tcsetattr(fd, tcsetattr_flags, new) + passwd = _raw_input(prompt, stream, input=input) + finally: + termios.tcsetattr(fd, tcsetattr_flags, old) + stream.flush() # issue7208 + except termios.error: + if passwd is not None: + # _raw_input succeeded. The final tcsetattr failed. Reraise + # instead of leaving the terminal in an unknown state. + raise + # We can't control the tty or stdin. Give up and use normal IO. + # fallback_getpass() raises an appropriate warning. + if stream is not input: + # clean up unused file objects before blocking + stack.close() + passwd = fallback_getpass(prompt, stream) + + stream.write('\n') + return passwd def win_getpass(prompt='Password: ', stream=None): diff --git a/Lib/test/test_getpass.py b/Lib/test/test_getpass.py index 15537b90b33..1731bd44a65 100644 --- a/Lib/test/test_getpass.py +++ b/Lib/test/test_getpass.py @@ -1,7 +1,7 @@ import getpass import os import unittest -from io import StringIO +from io import BytesIO, StringIO from unittest import mock from test import support @@ -88,7 +88,8 @@ class UnixGetpassTest(unittest.TestCase): def test_uses_tty_directly(self): with mock.patch('os.open') as open, \ - mock.patch('os.fdopen'): + mock.patch('io.FileIO') as fileio, \ + mock.patch('io.TextIOWrapper') as textio: # By setting open's return value to None the implementation will # skip code we don't care about in this test. We can mock this out # fully if an alternate implementation works differently. @@ -96,10 +97,13 @@ class UnixGetpassTest(unittest.TestCase): getpass.unix_getpass() open.assert_called_once_with('/dev/tty', os.O_RDWR | os.O_NOCTTY) + fileio.assert_called_once_with(open.return_value, 'w+') + textio.assert_called_once_with(fileio.return_value) def test_resets_termios(self): with mock.patch('os.open') as open, \ - mock.patch('os.fdopen'), \ + mock.patch('io.FileIO'), \ + mock.patch('io.TextIOWrapper'), \ mock.patch('termios.tcgetattr') as tcgetattr, \ mock.patch('termios.tcsetattr') as tcsetattr: open.return_value = 3 @@ -110,21 +114,23 @@ class UnixGetpassTest(unittest.TestCase): def test_falls_back_to_fallback_if_termios_raises(self): with mock.patch('os.open') as open, \ - mock.patch('os.fdopen') as fdopen, \ + mock.patch('io.FileIO') as fileio, \ + mock.patch('io.TextIOWrapper') as textio, \ mock.patch('termios.tcgetattr'), \ mock.patch('termios.tcsetattr') as tcsetattr, \ mock.patch('getpass.fallback_getpass') as fallback: open.return_value = 3 - fdopen.return_value = StringIO() + fileio.return_value = BytesIO() tcsetattr.side_effect = termios.error getpass.unix_getpass() fallback.assert_called_once_with('Password: ', - fdopen.return_value) + textio.return_value) def test_flushes_stream_after_input(self): # issue 7208 with mock.patch('os.open') as open, \ - mock.patch('os.fdopen'), \ + mock.patch('io.FileIO'), \ + mock.patch('io.TextIOWrapper'), \ mock.patch('termios.tcgetattr'), \ mock.patch('termios.tcsetattr'): open.return_value = 3 diff --git a/Misc/NEWS b/Misc/NEWS index f457fddc2a1..ed579ef1f7a 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -142,6 +142,10 @@ Core and Builtins Library ------- +- Issue #18116: getpass was always getting an error when testing /dev/tty, + and thus was always falling back to stdin. It also leaked an open file + when it did so. Both of these issues are now fixed. + - Issue #17198: Fix a NameError in the dbm module. Patch by Valentina Mukhamedzhanova.