From 61cd9a12522a51afc47c67e44468cbe66394cbb3 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 19 May 2017 21:29:47 +0300 Subject: [PATCH 1/3] Allow setting environment for readline tests This enable testing custom readline configuration using the INPUTRC environment variable, or passing arguments to the child process in a clean way. --- Lib/test/test_readline.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 06a9149374aeab..c1a017c0d8a089 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -211,12 +211,12 @@ def display(substitution, matches, longest_match_length): self.assertIn(b"history " + expected + b"\r\n", output) -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) From e6ea33038ba3ab2f5ba568393fa38315513372fa Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 19 May 2017 22:40:31 +0300 Subject: [PATCH 2/3] bpo-29854: Add failing test that crash readline readline segfaults on input() if the number of items in the history file is equal or more to history size * 2. This issue affects only GNU readline. When using libedit emulation system history size option does not work. --- Lib/test/test_readline.py | 40 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index c1a017c0d8a089..54ca4bf600598c 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 @@ -210,6 +210,44 @@ 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") + @unittest.expectedFailure + 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", env=None): pty = import_module('pty') From 9fcda6152b42f0969ad13555e554a4daa5896aea Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Mon, 20 Mar 2017 00:21:34 +0200 Subject: [PATCH 3/3] bpo-29854: Fix segfault in call_readline() 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. --- Lib/test/test_readline.py | 1 - .../Library/2017-07-07-02-18-57.bpo-29854.J8wKb_.rst | 2 ++ Modules/readline.c | 10 ++++++---- 3 files changed, 8 insertions(+), 5 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 54ca4bf600598c..cc3001a3796e2f 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -212,7 +212,6 @@ def display(substitution, matches, longest_match_length): @unittest.skipIf(is_editline, "editline history size configuration is broken") - @unittest.expectedFailure def test_history_size(self): history_size = 10 with temp_dir() as test_dir: 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 dfbbb292f668f5..6ba124742a3450 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -1347,15 +1347,17 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) if (should_auto_add_history && 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);