Skip to content

bpo-40638: Check for attribute lookup failure in builtin_input_impl #20125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Lib/test/test_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ class BitBucket:
def write(self, line):
pass

class MalformedInputStream:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Malformed" in what way? I think this name could be better.

Suggested change
class MalformedInputStream:
class InputStreamWithReadlineButNoRead:

def readline(self):
return "foobar\n"

def fileno(self):
return 0

class MalformedOutputStream:
def __init__(self):
self.value = ""

def fileno(self):
return 1

def write(self, value):
self.value += value

test_conv_no_sign = [
('0', 0),
('1', 1),
Expand Down Expand Up @@ -1302,6 +1319,13 @@ def test_input(self):
sys.stdin = io.StringIO()
self.assertRaises(EOFError, input)

# input() in tty mode with a malformed input stream should attempt
# to call .readline()
sys.stdin = MalformedInputStream()
sys.stdout = MalformedOutputStream()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using a StringIO object for sys.stdout here?

self.assertEqual(input("baz"), "foobar") # strips \n
self.assertEqual(sys.stdout.value, "baz")

Comment on lines +1322 to +1328
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this here is slightly confusing, as it appears like it may affect the following assertion as well. Perhaps put this at the end of this method or in a separate method.

del sys.stdout
self.assertRaises(RuntimeError, input, 'prompt')
del sys.stdin
Expand Down
18 changes: 12 additions & 6 deletions Python/bltinmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2010,17 +2010,21 @@ builtin_input_impl(PyObject *module, PyObject *prompt)

/* stdin is a text stream, so it must have an encoding. */
stdin_encoding = _PyObject_GetAttrId(fin, &PyId_encoding);
if (!stdin_encoding || !PyUnicode_Check(stdin_encoding)) {
goto _readline_disable_tty_and_fall_back;
}
stdin_errors = _PyObject_GetAttrId(fin, &PyId_errors);
if (!stdin_encoding || !stdin_errors ||
!PyUnicode_Check(stdin_encoding) ||
!PyUnicode_Check(stdin_errors)) {
tty = 0;
goto _readline_errors;
if (!stdin_errors || !PyUnicode_Check(stdin_errors)) {
goto _readline_disable_tty_and_fall_back;
}
stdin_encoding_str = PyUnicode_AsUTF8(stdin_encoding);
if (!stdin_encoding_str) {
goto _readline_errors;
}
stdin_errors_str = PyUnicode_AsUTF8(stdin_errors);
if (!stdin_encoding_str || !stdin_errors_str)
if (!stdin_errors_str) {
goto _readline_errors;
}
tmp = _PyObject_CallMethodIdNoArgs(fout, &PyId_flush);
if (tmp == NULL)
PyErr_Clear();
Expand Down Expand Up @@ -2099,6 +2103,8 @@ builtin_input_impl(PyObject *module, PyObject *prompt)

return result;

_readline_disable_tty_and_fall_back:
tty = 0;
_readline_errors:
Py_XDECREF(stdin_encoding);
Py_XDECREF(stdout_encoding);
Expand Down