From 5058a49e03af322b06356d068174d53f65b00ca4 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 7 Jul 2017 09:10:46 +0300 Subject: [PATCH 1/2] bpo-29854: Fix segfault in call_readline() (GH-728) If history-length is set in .inputrc, and the history file is double the history size (or more), history_get(N) returns NULL, and python segfaults. Fix that by checking for NULL return value. It seems that the root cause is incorrect handling of bigger history in readline, but Python should not segfault even if readline returns unexpected value. This issue affects only GNU readline. When using libedit emulation system history size option does not work. --- Lib/test/test_readline.py | 43 +++++++++++++++++-- .../2017-07-07-02-18-57.bpo-29854.J8wKb_.rst | 2 + Modules/readline.c | 10 +++-- 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-07-07-02-18-57.bpo-29854.J8wKb_.rst diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 2c73df291f0c5a..4667155df206d6 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -9,7 +9,7 @@ import sys import tempfile import unittest -from test.support import import_module, unlink, TESTFN +from test.support import import_module, unlink, temp_dir, TESTFN from test.support.script_helper import assert_python_ok # Skip tests if there is no readline module @@ -195,13 +195,50 @@ def display(substitution, matches, longest_match_length): self.assertIn(b"result " + expected + b"\r\n", output) self.assertIn(b"history " + expected + b"\r\n", output) + @unittest.skipIf(is_editline, + "editline history size configuration is broken") + def test_history_size(self): + history_size = 10 + with temp_dir() as test_dir: + inputrc = os.path.join(test_dir, "inputrc") + with open(inputrc, "wb") as f: + f.write(b"set history-size %d\n" % history_size) + + history_file = os.path.join(test_dir, "history") + with open(history_file, "wb") as f: + # history_size * 2 items crashes readline + data = b"".join(b"item %d\n" % i + for i in range(history_size * 2)) + f.write(data) + + script = """ +import os +import readline + +history_file = os.environ["HISTORY_FILE"] +readline.read_history_file(history_file) +input() +readline.write_history_file(history_file) +""" + + env = dict(os.environ) + env["INPUTRC"] = inputrc + env["HISTORY_FILE"] = history_file + + run_pty(script, input=b"last input\r", env=env) + + with open(history_file, "rb") as f: + lines = f.readlines() + self.assertEqual(len(lines), history_size) + self.assertEqual(lines[-1].strip(), b"last input") + -def run_pty(script, input=b"dummy input\r"): +def run_pty(script, input=b"dummy input\r", env=None): pty = import_module('pty') output = bytearray() [master, slave] = pty.openpty() args = (sys.executable, '-c', script) - proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave) + proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) os.close(slave) with ExitStack() as cleanup: cleanup.enter_context(proc) diff --git a/Misc/NEWS.d/next/Library/2017-07-07-02-18-57.bpo-29854.J8wKb_.rst b/Misc/NEWS.d/next/Library/2017-07-07-02-18-57.bpo-29854.J8wKb_.rst new file mode 100644 index 00000000000000..5c439087dcb344 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-07-07-02-18-57.bpo-29854.J8wKb_.rst @@ -0,0 +1,2 @@ +Fix segfault in readline when using readline's history-size option. Patch +by Nir Soffer. diff --git a/Modules/readline.c b/Modules/readline.c index 54f15bc1c934de..c6035e2f3663ab 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -1330,15 +1330,17 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) if (n > 0) { const char *line; int length = _py_get_history_length(); - if (length > 0) + if (length > 0) { + HIST_ENTRY *hist_ent; #ifdef __APPLE__ if (using_libedit_emulation) { /* handle older 0-based or newer 1-based indexing */ - line = (const char *)history_get(length + libedit_history_start - 1)->line; + hist_ent = history_get(length + libedit_history_start - 1); } else #endif /* __APPLE__ */ - line = (const char *)history_get(length)->line; - else + hist_ent = history_get(length); + line = hist_ent ? hist_ent->line : ""; + } else line = ""; if (strcmp(p, line)) add_history(p); From 0a48572916e6609dda05f80a65c082551802eb2a Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 8 Jul 2017 17:34:27 +0300 Subject: [PATCH 2/2] bpo-29854: Skip history-size test on older readline (GH-2621) Turns out that history-size was added in readline 6.0. This explain why this tests fail on FreeBSD when using readline 5.2. We skip now the history size if readline does not support it. See https://cnswww.cns.cwru.edu/php/chet/readline/CHANGES for details. --- Lib/test/test_readline.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 4667155df206d6..931c855b66ca3c 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -195,6 +195,13 @@ def display(substitution, matches, longest_match_length): self.assertIn(b"result " + expected + b"\r\n", output) self.assertIn(b"history " + expected + b"\r\n", output) + # We have 2 reasons to skip this test: + # - readline: history size was added in 6.0 + # See https://cnswww.cns.cwru.edu/php/chet/readline/CHANGES + # - editline: history size is broken on OS X 10.11.6. + # Newer versions were not tested yet. + @unittest.skipIf(readline._READLINE_VERSION < 0x600, + "this readline version does not support history-size") @unittest.skipIf(is_editline, "editline history size configuration is broken") def test_history_size(self):