From 9d3c61c86a20678d604c96a68bbf4a966877f0b9 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 5 Sep 2015 21:05:05 +1000 Subject: [PATCH 1/7] Close #24748: Restore imp.load_dynamic compatibility To resolve a compatibility problem found with py2exe and pywin32, imp.load_dynamic() once again ignores previously loaded modules to support Python modules replacing themselves with extension modules. Patch by Petr Viktorin. --- Lib/imp.py | 8 +++++++- Lib/test/imp_dummy.py | 3 +++ Lib/test/test_imp.py | 24 ++++++++++++++++++++++++ Misc/NEWS | 5 +++++ Modules/_testmultiphase.c | 10 ++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 Lib/test/imp_dummy.py diff --git a/Lib/imp.py b/Lib/imp.py index 2cd64407e86..f6fff442013 100644 --- a/Lib/imp.py +++ b/Lib/imp.py @@ -334,6 +334,12 @@ if create_dynamic: """ import importlib.machinery loader = importlib.machinery.ExtensionFileLoader(name, path) - return loader.load_module() + + # Issue #24748: Skip the sys.modules check in _load_module_shim; + # always load new extension + spec = importlib.machinery.ModuleSpec( + name=name, loader=loader, origin=path) + return _load(spec) + else: load_dynamic = None diff --git a/Lib/test/imp_dummy.py b/Lib/test/imp_dummy.py new file mode 100644 index 00000000000..2a4deb49547 --- /dev/null +++ b/Lib/test/imp_dummy.py @@ -0,0 +1,3 @@ +# Fodder for test of issue24748 in test_imp + +dummy_name = True diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index 47bf1de92a4..ee9ee1ad8c3 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -3,6 +3,7 @@ try: except ImportError: _thread = None import importlib +import importlib.util import os import os.path import shutil @@ -275,6 +276,29 @@ class ImportTests(unittest.TestCase): self.skipTest("found module doesn't appear to be a C extension") imp.load_module(name, None, *found[1:]) + @requires_load_dynamic + def test_issue24748_load_module_skips_sys_modules_check(self): + name = 'test.imp_dummy' + try: + del sys.modules[name] + except KeyError: + pass + try: + module = importlib.import_module(name) + spec = importlib.util.find_spec('_testmultiphase') + module = imp.load_dynamic(name, spec.origin) + self.assertEqual(module.__name__, name) + self.assertEqual(module.__spec__.name, name) + self.assertEqual(module.__spec__.origin, spec.origin) + self.assertRaises(AttributeError, getattr, module, 'dummy_name') + self.assertEqual(module.int_const, 1969) + self.assertIs(sys.modules[name], module) + finally: + try: + del sys.modules[name] + except KeyError: + pass + @unittest.skipIf(sys.dont_write_bytecode, "test meaningful only when writing bytecode") def test_bug7732(self): diff --git a/Misc/NEWS b/Misc/NEWS index 288fa62cfa3..7ea23875310 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -15,6 +15,11 @@ Core and Builtins Library ------- +- Issue #24748: To resolve a compatibility problem found with py2exe and + pywin32, imp.load_dynamic() once again ignores previously loaded modules + to support Python modules replacing themselves with extension modules. + Patch by Petr Viktorin. + - Issue #24635: Fixed a bug in typing.py where isinstance([], typing.Iterable) would return True once, then False on subsequent calls. diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 2919687e112..2005205d335 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -582,3 +582,13 @@ PyInit__testmultiphase_exec_unreported_exception(PyObject *spec) { return PyModuleDef_Init(&def_exec_unreported_exception); } + +/*** Helper for imp test ***/ + +static PyModuleDef imp_dummy_def = TEST_MODULE_DEF("imp_dummy", main_slots, testexport_methods); + +PyMODINIT_FUNC +PyInit_imp_dummy(PyObject *spec) +{ + return PyModuleDef_Init(&imp_dummy_def); +} From 2ebd8f5194257d91b076e6866573b8f20f8f40a0 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sat, 5 Sep 2015 11:57:47 -0700 Subject: [PATCH 2/7] Issue #25005: Backout fix for #8232 because of use of unsafe subprocess.call(shell=True) --- Lib/webbrowser.py | 120 ++++------------------------------------------ Misc/NEWS | 3 -- 2 files changed, 9 insertions(+), 114 deletions(-) diff --git a/Lib/webbrowser.py b/Lib/webbrowser.py index 6deed922306..845f1d004c0 100755 --- a/Lib/webbrowser.py +++ b/Lib/webbrowser.py @@ -495,23 +495,10 @@ if os.environ.get("TERM"): # if sys.platform[:3] == "win": - class WindowsDefault(BaseBrowser): - # Windows Default opening arguments. - - cmd = "start" - newwindow = "" - newtab = "" - def open(self, url, new=0, autoraise=True): - # Format the command for optional arguments and add the url. - if new == 1: - self.cmd += " " + self.newwindow - elif new == 2: - self.cmd += " " + self.newtab - self.cmd += " " + url try: - subprocess.call(self.cmd, shell=True) + os.startfile(url) except OSError: # [Error 22] No application is associated with the specified # file for this operation: '' @@ -519,108 +506,19 @@ if sys.platform[:3] == "win": else: return True - - # Windows Sub-Classes for commonly used browsers. - - class InternetExplorer(WindowsDefault): - """Launcher class for Internet Explorer browser""" - - cmd = "start iexplore.exe" - newwindow = "" - newtab = "" - - - class WinChrome(WindowsDefault): - """Launcher class for windows specific Google Chrome browser""" - - cmd = "start chrome.exe" - newwindow = "-new-window" - newtab = "-new-tab" - - - class WinFirefox(WindowsDefault): - """Launcher class for windows specific Firefox browser""" - - cmd = "start firefox.exe" - newwindow = "-new-window" - newtab = "-new-tab" - - - class WinOpera(WindowsDefault): - """Launcher class for windows specific Opera browser""" - - cmd = "start opera" - newwindow = "" - newtab = "" - - - class WinSeaMonkey(WindowsDefault): - """Launcher class for windows specific SeaMonkey browser""" - - cmd = "start seamonkey" - newwinow = "" - newtab = "" - - _tryorder = [] _browsers = {} - # First try to use the default Windows browser. + # First try to use the default Windows browser register("windows-default", WindowsDefault) - def find_windows_browsers(): - """ Access the windows registry to determine - what browsers are on the system. - """ - - import winreg - HKLM = winreg.HKEY_LOCAL_MACHINE - subkey = r'Software\Clients\StartMenuInternet' - read32 = winreg.KEY_READ | winreg.KEY_WOW64_32KEY - read64 = winreg.KEY_READ | winreg.KEY_WOW64_64KEY - key32 = winreg.OpenKey(HKLM, subkey, access=read32) - key64 = winreg.OpenKey(HKLM, subkey, access=read64) - - # Return a list of browsers found in the registry - # Check if there are any different browsers in the - # 32 bit location instead of the 64 bit location. - browsers = [] - i = 0 - while True: - try: - browsers.append(winreg.EnumKey(key32, i)) - except EnvironmentError: - break - i += 1 - - i = 0 - while True: - try: - browsers.append(winreg.EnumKey(key64, i)) - except EnvironmentError: - break - i += 1 - - winreg.CloseKey(key32) - winreg.CloseKey(key64) - - return browsers - - # Detect some common windows browsers - for browser in find_windows_browsers(): - browser = browser.lower() - if "iexplore" in browser: - register("iexplore", None, InternetExplorer("iexplore")) - elif "chrome" in browser: - register("chrome", None, WinChrome("chrome")) - elif "firefox" in browser: - register("firefox", None, WinFirefox("firefox")) - elif "opera" in browser: - register("opera", None, WinOpera("opera")) - elif "seamonkey" in browser: - register("seamonkey", None, WinSeaMonkey("seamonkey")) - else: - register(browser, None, WindowsDefault(browser)) + # Detect some common Windows browsers, fallback to IE + iexplore = os.path.join(os.environ.get("PROGRAMFILES", "C:\\Program Files"), + "Internet Explorer\\IEXPLORE.EXE") + for browser in ("firefox", "firebird", "seamonkey", "mozilla", + "netscape", "opera", iexplore): + if shutil.which(browser): + register(browser, None, BackgroundBrowser(browser)) # # Platform support for MacOS diff --git a/Misc/NEWS b/Misc/NEWS index b28aa197848..d67ec38dfb8 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -312,9 +312,6 @@ Library - Issue #14373: C implementation of functools.lru_cache() now can be used with methods. -- Issue #8232: webbrowser support incomplete on Windows. Patch by Brandon - Milam - - Issue #24347: Set KeyError if PyDict_GetItemWithError returns NULL. - Issue #24348: Drop superfluous incref/decref. From 62b24624dd3eec75869372e08d5ecbc4b8976101 Mon Sep 17 00:00:00 2001 From: Larry Hastings Date: Sun, 6 Sep 2015 00:31:02 -0700 Subject: [PATCH 3/7] Backing out 09b62202d9b7; the tests fail on Linux, and it needs a re-think. --- Lib/test/test_time.py | 6 ------ Misc/NEWS | 2 -- Modules/timemodule.c | 12 ------------ 3 files changed, 20 deletions(-) diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index 3f571a0e6f6..6334e022e00 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -174,12 +174,6 @@ class TimeTestCase(unittest.TestCase): def test_strftime_bounding_check(self): self._bounds_checking(lambda tup: time.strftime('', tup)) - def test_strftime_format_check(self): - for x in [ '', 'A', '%A', '%AA' ]: - for y in range(0x0, 0x10): - for z in [ '%', 'A%', 'AA%', '%A%', 'A%A%', '%#' ]: - self.assertRaises(ValueError, time.strftime, x * y + z) - def test_default_values_for_zero(self): # Make sure that using all zeros uses the proper default # values. No test for daylight savings since strftime() does diff --git a/Misc/NEWS b/Misc/NEWS index ba24851f11e..5ac6df9f29c 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -22,8 +22,6 @@ Library to support Python modules replacing themselves with extension modules. Patch by Petr Viktorin. -- Issue #24917: time_strftime() Buffer Over-read. Patch by John Leitch. - - Issue #24635: Fixed a bug in typing.py where isinstance([], typing.Iterable) would return True once, then False on subsequent calls. diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 55e26fa8a25..197d2c0b8dd 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -623,12 +623,6 @@ time_strftime(PyObject *self, PyObject *args) Py_DECREF(format); return NULL; } - else if (outbuf[1] == '\0') - { - PyErr_SetString(PyExc_ValueError, "Incomplete format string"); - Py_DECREF(format); - return NULL; - } } #elif (defined(_AIX) || defined(sun)) && defined(HAVE_WCSFTIME) for(outbuf = wcschr(fmt, '%'); @@ -642,12 +636,6 @@ time_strftime(PyObject *self, PyObject *args) "format %y requires year >= 1900 on AIX"); return NULL; } - else if (outbuf[1] == '\0') - { - PyErr_SetString(PyExc_ValueError, "Incomplete format string"); - Py_DECREF(format); - return NULL; - } } #endif From 714e49371b8d73059cf19f92a8566dcd20c6089a Mon Sep 17 00:00:00 2001 From: Larry Hastings Date: Sun, 6 Sep 2015 00:39:37 -0700 Subject: [PATCH 4/7] Issue #24305: Prevent import subsystem stack frames from being counted by the warnings.warn(stacklevel=) parameter. --- .../__init__.py} | 31 +++++--- Lib/test/test_warnings/__main__.py | 3 + Lib/test/test_warnings/data/import_warning.py | 3 + .../data/stacklevel.py} | 0 Lib/warnings.py | 31 ++++++-- Misc/NEWS | 3 + Python/_warnings.c | 72 ++++++++++++++++++- 7 files changed, 127 insertions(+), 16 deletions(-) rename Lib/test/{test_warnings.py => test_warnings/__init__.py} (97%) create mode 100644 Lib/test/test_warnings/__main__.py create mode 100644 Lib/test/test_warnings/data/import_warning.py rename Lib/test/{warning_tests.py => test_warnings/data/stacklevel.py} (100%) diff --git a/Lib/test/test_warnings.py b/Lib/test/test_warnings/__init__.py similarity index 97% rename from Lib/test/test_warnings.py rename to Lib/test/test_warnings/__init__.py index c7d2e5cfbba..991a249f4a4 100644 --- a/Lib/test/test_warnings.py +++ b/Lib/test/test_warnings/__init__.py @@ -7,7 +7,7 @@ import unittest from test import support from test.support.script_helper import assert_python_ok, assert_python_failure -from test import warning_tests +from test.test_warnings.data import stacklevel as warning_tests import warnings as original_warnings @@ -188,11 +188,11 @@ class FilterTests(BaseTest): self.module.resetwarnings() self.module.filterwarnings("once", category=UserWarning) message = UserWarning("FilterTests.test_once") - self.module.warn_explicit(message, UserWarning, "test_warnings.py", + self.module.warn_explicit(message, UserWarning, "__init__.py", 42) self.assertEqual(w[-1].message, message) del w[:] - self.module.warn_explicit(message, UserWarning, "test_warnings.py", + self.module.warn_explicit(message, UserWarning, "__init__.py", 13) self.assertEqual(len(w), 0) self.module.warn_explicit(message, UserWarning, "test_warnings2.py", @@ -298,10 +298,10 @@ class WarnTests(BaseTest): module=self.module) as w: warning_tests.inner("spam1") self.assertEqual(os.path.basename(w[-1].filename), - "warning_tests.py") + "stacklevel.py") warning_tests.outer("spam2") self.assertEqual(os.path.basename(w[-1].filename), - "warning_tests.py") + "stacklevel.py") def test_stacklevel(self): # Test stacklevel argument @@ -311,25 +311,36 @@ class WarnTests(BaseTest): module=self.module) as w: warning_tests.inner("spam3", stacklevel=1) self.assertEqual(os.path.basename(w[-1].filename), - "warning_tests.py") + "stacklevel.py") warning_tests.outer("spam4", stacklevel=1) self.assertEqual(os.path.basename(w[-1].filename), - "warning_tests.py") + "stacklevel.py") warning_tests.inner("spam5", stacklevel=2) self.assertEqual(os.path.basename(w[-1].filename), - "test_warnings.py") + "__init__.py") warning_tests.outer("spam6", stacklevel=2) self.assertEqual(os.path.basename(w[-1].filename), - "warning_tests.py") + "stacklevel.py") warning_tests.outer("spam6.5", stacklevel=3) self.assertEqual(os.path.basename(w[-1].filename), - "test_warnings.py") + "__init__.py") warning_tests.inner("spam7", stacklevel=9999) self.assertEqual(os.path.basename(w[-1].filename), "sys") + def test_stacklevel_import(self): + # Issue #24305: With stacklevel=2, module-level warnings should work. + support.unload('test.test_warnings.data.import_warning') + with warnings_state(self.module): + with original_warnings.catch_warnings(record=True, + module=self.module) as w: + self.module.simplefilter('always') + import test.test_warnings.data.import_warning + self.assertEqual(len(w), 1) + self.assertEqual(w[0].filename, __file__) + def test_missing_filename_not_main(self): # If __file__ is not specified and __main__ is not the module name, # then __file__ should be set to the module name. diff --git a/Lib/test/test_warnings/__main__.py b/Lib/test/test_warnings/__main__.py new file mode 100644 index 00000000000..44e52ec0704 --- /dev/null +++ b/Lib/test/test_warnings/__main__.py @@ -0,0 +1,3 @@ +import unittest + +unittest.main('test.test_warnings') diff --git a/Lib/test/test_warnings/data/import_warning.py b/Lib/test/test_warnings/data/import_warning.py new file mode 100644 index 00000000000..d6ea2ce1046 --- /dev/null +++ b/Lib/test/test_warnings/data/import_warning.py @@ -0,0 +1,3 @@ +import warnings + +warnings.warn('module-level warning', DeprecationWarning, stacklevel=2) \ No newline at end of file diff --git a/Lib/test/warning_tests.py b/Lib/test/test_warnings/data/stacklevel.py similarity index 100% rename from Lib/test/warning_tests.py rename to Lib/test/test_warnings/data/stacklevel.py diff --git a/Lib/warnings.py b/Lib/warnings.py index 16246b43658..1d4fb208f83 100644 --- a/Lib/warnings.py +++ b/Lib/warnings.py @@ -160,6 +160,20 @@ def _getcategory(category): return cat +def _is_internal_frame(frame): + """Signal whether the frame is an internal CPython implementation detail.""" + filename = frame.f_code.co_filename + return 'importlib' in filename and '_bootstrap' in filename + + +def _next_external_frame(frame): + """Find the next frame that doesn't involve CPython internals.""" + frame = frame.f_back + while frame is not None and _is_internal_frame(frame): + frame = frame.f_back + return frame + + # Code typically replaced by _warnings def warn(message, category=None, stacklevel=1): """Issue a warning, or maybe ignore it or raise an exception.""" @@ -174,13 +188,23 @@ def warn(message, category=None, stacklevel=1): "not '{:s}'".format(type(category).__name__)) # Get context information try: - caller = sys._getframe(stacklevel) + if stacklevel <= 1 or _is_internal_frame(sys._getframe(1)): + # If frame is too small to care or if the warning originated in + # internal code, then do not try to hide any frames. + frame = sys._getframe(stacklevel) + else: + frame = sys._getframe(1) + # Look for one frame less since the above line starts us off. + for x in range(stacklevel-1): + frame = _next_external_frame(frame) + if frame is None: + raise ValueError except ValueError: globals = sys.__dict__ lineno = 1 else: - globals = caller.f_globals - lineno = caller.f_lineno + globals = frame.f_globals + lineno = frame.f_lineno if '__name__' in globals: module = globals['__name__'] else: @@ -374,7 +398,6 @@ try: defaultaction = _defaultaction onceregistry = _onceregistry _warnings_defaults = True - except ImportError: filters = [] defaultaction = "default" diff --git a/Misc/NEWS b/Misc/NEWS index 5ac6df9f29c..b28aa197848 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Release date: 2015-09-06 Core and Builtins ----------------- +- Issue #24305: Prevent import subsystem stack frames from being counted + by the warnings.warn(stacklevel=) parameter. + - Issue #24912: Prevent __class__ assignment to immutable built-in objects. - Issue #24975: Fix AST compilation for PEP 448 syntax. diff --git a/Python/_warnings.c b/Python/_warnings.c index 22f617a9ffb..9ca83145c94 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -513,6 +513,64 @@ warn_explicit(PyObject *category, PyObject *message, return result; /* Py_None or NULL. */ } +static int +is_internal_frame(PyFrameObject *frame) +{ + static PyObject *importlib_string = NULL; + static PyObject *bootstrap_string = NULL; + PyObject *filename; + int contains; + + if (importlib_string == NULL) { + importlib_string = PyUnicode_FromString("importlib"); + if (importlib_string == NULL) { + return 0; + } + + bootstrap_string = PyUnicode_FromString("_bootstrap"); + if (bootstrap_string == NULL) { + Py_DECREF(importlib_string); + return 0; + } + Py_INCREF(importlib_string); + Py_INCREF(bootstrap_string); + } + + if (frame == NULL || frame->f_code == NULL || + frame->f_code->co_filename == NULL) { + return 0; + } + filename = frame->f_code->co_filename; + if (!PyUnicode_Check(filename)) { + return 0; + } + contains = PyUnicode_Contains(filename, importlib_string); + if (contains < 0) { + return 0; + } + else if (contains > 0) { + contains = PyUnicode_Contains(filename, bootstrap_string); + if (contains < 0) { + return 0; + } + else if (contains > 0) { + return 1; + } + } + + return 0; +} + +static PyFrameObject * +next_external_frame(PyFrameObject *frame) +{ + do { + frame = frame->f_back; + } while (frame != NULL && is_internal_frame(frame)); + + return frame; +} + /* filename, module, and registry are new refs, globals is borrowed */ /* Returns 0 on error (no new refs), 1 on success */ static int @@ -523,8 +581,18 @@ setup_context(Py_ssize_t stack_level, PyObject **filename, int *lineno, /* Setup globals and lineno. */ PyFrameObject *f = PyThreadState_GET()->frame; - while (--stack_level > 0 && f != NULL) - f = f->f_back; + // Stack level comparisons to Python code is off by one as there is no + // warnings-related stack level to avoid. + if (stack_level <= 0 || is_internal_frame(f)) { + while (--stack_level > 0 && f != NULL) { + f = f->f_back; + } + } + else { + while (--stack_level > 0 && f != NULL) { + f = next_external_frame(f); + } + } if (f == NULL) { globals = PyThreadState_Get()->interp->sysdict; From e5b5895b5b52ff14093eaababd04ede69e394959 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sun, 6 Sep 2015 19:20:51 -0700 Subject: [PATCH 5/7] Issue #24917: time_strftime() buffer over-read. --- Lib/test/test_time.py | 13 +++++++++++++ Misc/NEWS | 2 ++ Modules/timemodule.c | 16 ++++++++++------ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index 6334e022e00..6bcd2124fa4 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -174,6 +174,19 @@ class TimeTestCase(unittest.TestCase): def test_strftime_bounding_check(self): self._bounds_checking(lambda tup: time.strftime('', tup)) + def test_strftime_format_check(self): + # Test that strftime does not crash on invalid format strings + # that may trigger a buffer overread. When not triggered, + # strftime may succeed or raise ValueError depending on + # the platform. + for x in [ '', 'A', '%A', '%AA' ]: + for y in range(0x0, 0x10): + for z in [ '%', 'A%', 'AA%', '%A%', 'A%A%', '%#' ]: + try: + time.strftime(x * y + z) + except ValueError: + pass + def test_default_values_for_zero(self): # Make sure that using all zeros uses the proper default # values. No test for daylight savings since strftime() does diff --git a/Misc/NEWS b/Misc/NEWS index b28aa197848..b1f1498114c 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -20,6 +20,8 @@ Core and Builtins Library ------- +- Issue #24917: time_strftime() buffer over-read. + - Issue #24748: To resolve a compatibility problem found with py2exe and pywin32, imp.load_dynamic() once again ignores previously loaded modules to support Python modules replacing themselves with extension modules. diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 197d2c0b8dd..eca67d9e286 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -610,14 +610,15 @@ time_strftime(PyObject *self, PyObject *args) #if defined(MS_WINDOWS) && !defined(HAVE_WCSFTIME) /* check that the format string contains only valid directives */ - for(outbuf = strchr(fmt, '%'); + for (outbuf = strchr(fmt, '%'); outbuf != NULL; outbuf = strchr(outbuf+2, '%')) { - if (outbuf[1]=='#') + if (outbuf[1] == '#') ++outbuf; /* not documented by python, */ - if ((outbuf[1] == 'y') && buf.tm_year < 0) - { + if (outbuf[1] == '\0') + break; + if ((outbuf[1] == 'y') && buf.tm_year < 0) { PyErr_SetString(PyExc_ValueError, "format %y requires year >= 1900 on Windows"); Py_DECREF(format); @@ -625,10 +626,12 @@ time_strftime(PyObject *self, PyObject *args) } } #elif (defined(_AIX) || defined(sun)) && defined(HAVE_WCSFTIME) - for(outbuf = wcschr(fmt, '%'); + for (outbuf = wcschr(fmt, '%'); outbuf != NULL; outbuf = wcschr(outbuf+2, '%')) { + if (outbuf[1] == L'\0') + break; /* Issue #19634: On AIX, wcsftime("y", (1899, 1, 1, 0, 0, 0, 0, 0, 0)) returns "0/" instead of "99" */ if (outbuf[1] == L'y' && buf.tm_year < 0) { @@ -659,7 +662,8 @@ time_strftime(PyObject *self, PyObject *args) #if defined _MSC_VER && _MSC_VER >= 1400 && defined(__STDC_SECURE_LIB__) err = errno; #endif - if (buflen > 0 || i >= 256 * fmtlen) { + if (buflen > 0 || fmtlen == 0 || + (fmtlen > 4 && i >= 256 * fmtlen)) { /* If the buffer is 256 times as long as the format, it's probably not failing for lack of room! More likely, the format yields an empty result, From aa2fcc6b35e92ee7a8b1396ee454a9f92936c850 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sun, 6 Sep 2015 22:18:36 -0700 Subject: [PATCH 6/7] Issue #24917: time_strftime() buffer over-read. --- Misc/NEWS | 2 ++ Modules/timemodule.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index 7a3c22c4832..ac0541b1ad7 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -81,6 +81,8 @@ Core and Builtins Library ------- +- Issue #24917: time_strftime() buffer over-read. + - Issue #23144: Make sure that HTMLParser.feed() returns all the data, even when convert_charrefs is True. diff --git a/Modules/timemodule.c b/Modules/timemodule.c index d0917a40730..d71b3ac872a 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -655,6 +655,8 @@ time_strftime(PyObject *self, PyObject *args) outbuf != NULL; outbuf = wcschr(outbuf+2, '%')) { + if (outbuf[1] == L'\0') + break; /* Issue #19634: On AIX, wcsftime("y", (1899, 1, 1, 0, 0, 0, 0, 0, 0)) returns "0/" instead of "99" */ if (outbuf[1] == L'y' && buf.tm_year < 0) { From 96d49438465b78abf1f09f190ee46793dcd672b8 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sun, 6 Sep 2015 22:30:40 -0700 Subject: [PATCH 7/7] Reapplied change to test_warnings.py to test_warnings/__init__.py. --- Lib/test/test_warnings/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 991a249f4a4..cea9c574abf 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -44,6 +44,7 @@ class BaseTest: """Basic bookkeeping required for testing.""" def setUp(self): + self.old_unittest_module = unittest.case.warnings # The __warningregistry__ needs to be in a pristine state for tests # to work properly. if '__warningregistry__' in globals(): @@ -55,10 +56,15 @@ class BaseTest: # The 'warnings' module must be explicitly set so that the proper # interaction between _warnings and 'warnings' can be controlled. sys.modules['warnings'] = self.module + # Ensure that unittest.TestCase.assertWarns() uses the same warnings + # module than warnings.catch_warnings(). Otherwise, + # warnings.catch_warnings() will be unable to remove the added filter. + unittest.case.warnings = self.module super(BaseTest, self).setUp() def tearDown(self): sys.modules['warnings'] = original_warnings + unittest.case.warnings = self.old_unittest_module super(BaseTest, self).tearDown() class PublicAPITests(BaseTest):