Mailman 3 September 2019 - Python-checkins

bpo-38108: Makes mock objects inherit from Base (GH-16060) (GH-16470)
by Lisa Roach Sept. 30, 2019

Sept. 30, 2019

https://github.com/python/cpython/commit/b76ab352405df105c2d459fc66ef8dc98e… commit: b76ab352405df105c2d459fc66ef8dc98e47b37c branch: 3.8 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: Lisa Roach <lisaroach14(a)gmail.com> date: 2019-09-29T21:02:46-07:00 summary: bpo-38108: Makes mock objects inherit from Base (GH-16060) (GH-16470) files: A Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst M Lib/unittest/mock.py M Lib/… [View More]unittest/test/testmock/testasync.py M Lib/unittest/test/testmock/testmagicmethods.py diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 5ea562428a12..8c542cfab787 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -415,7 +415,7 @@ def __new__(cls, /, *args, **kw): if _is_async_obj(bound_args[spec_arg[0]]): bases = (AsyncMockMixin, cls,) new = type(cls.__name__, bases, {'__doc__': cls.__doc__}) - instance = object.__new__(new) + instance = _safe_super(NonCallableMock, cls).__new__(new) return instance @@ -995,17 +995,18 @@ def _get_child_mock(self, /, **kw): _type = type(self) if issubclass(_type, MagicMock) and _new_name in _async_method_magics: + # Any asynchronous magic becomes an AsyncMock klass = AsyncMock - elif _new_name in _sync_async_magics: - # Special case these ones b/c users will assume they are async, - # but they are actually sync (ie. __aiter__) - klass = MagicMock elif issubclass(_type, AsyncMockMixin): - klass = AsyncMock + if _new_name in _all_sync_magics: + # Any synchronous magic becomes a MagicMock + klass = MagicMock + else: + klass = AsyncMock elif not issubclass(_type, CallableMixin): if issubclass(_type, NonCallableMagicMock): klass = MagicMock - elif issubclass(_type, NonCallableMock) : + elif issubclass(_type, NonCallableMock): klass = Mock else: klass = _type.__mro__[1] @@ -1872,6 +1873,7 @@ def _patch_stopall(): "round trunc floor ceil " "bool next " "fspath " + "aiter " ) numerics = ( @@ -2010,7 +2012,7 @@ def _set_return_value(mock, method, name): -class MagicMixin(object): +class MagicMixin(Base): def __init__(self, /, *args, **kw): self._mock_set_magics() # make magic work for kwargs in init _safe_super(MagicMixin, self).__init__(*args, **kw) @@ -2018,13 +2020,14 @@ def __init__(self, /, *args, **kw): def _mock_set_magics(self): - these_magics = _magics + orig_magics = _magics | _async_method_magics + these_magics = orig_magics if getattr(self, "_mock_methods", None) is not None: - these_magics = _magics.intersection(self._mock_methods) + these_magics = orig_magics.intersection(self._mock_methods) remove_magics = set() - remove_magics = _magics - these_magics + remove_magics = orig_magics - these_magics for entry in remove_magics: if entry in type(self).__dict__: @@ -2052,33 +2055,13 @@ def mock_add_spec(self, spec, spec_set=False): self._mock_set_magics() -class AsyncMagicMixin: +class AsyncMagicMixin(MagicMixin): def __init__(self, /, *args, **kw): - self._mock_set_async_magics() # make magic work for kwargs in init + self._mock_set_magics() # make magic work for kwargs in init _safe_super(AsyncMagicMixin, self).__init__(*args, **kw) - self._mock_set_async_magics() # fix magic broken by upper level init - - def _mock_set_async_magics(self): - these_magics = _async_magics - - if getattr(self, "_mock_methods", None) is not None: - these_magics = _async_magics.intersection(self._mock_methods) - remove_magics = _async_magics - these_magics - - for entry in remove_magics: - if entry in type(self).__dict__: - # remove unneeded magic methods - delattr(self, entry) - - # don't overwrite existing attributes if called a second time - these_magics = these_magics - set(type(self).__dict__) - - _type = type(self) - for entry in these_magics: - setattr(_type, entry, MagicProxy(entry, self)) - + self._mock_set_magics() # fix magic broken by upper level init -class MagicMock(MagicMixin, AsyncMagicMixin, Mock): +class MagicMock(MagicMixin, Mock): """ MagicMock is a subclass of Mock with default implementations of most of the magic methods. You can use MagicMock without having to @@ -2100,7 +2083,7 @@ def mock_add_spec(self, spec, spec_set=False): -class MagicProxy(object): +class MagicProxy(Base): def __init__(self, name, parent): self.name = name self.parent = parent diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index 86b0d0ee9a13..df5ac5e3528c 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -375,6 +375,43 @@ def test_add_side_effect_iterable(self): RuntimeError('coroutine raised StopIteration') ) +class AsyncMagicMethods(unittest.TestCase): + def test_async_magic_methods_return_async_mocks(self): + m_mock = MagicMock() + self.assertIsInstance(m_mock.__aenter__, AsyncMock) + self.assertIsInstance(m_mock.__aexit__, AsyncMock) + self.assertIsInstance(m_mock.__anext__, AsyncMock) + # __aiter__ is actually a synchronous object + # so should return a MagicMock + self.assertIsInstance(m_mock.__aiter__, MagicMock) + + def test_sync_magic_methods_return_magic_mocks(self): + a_mock = AsyncMock() + self.assertIsInstance(a_mock.__enter__, MagicMock) + self.assertIsInstance(a_mock.__exit__, MagicMock) + self.assertIsInstance(a_mock.__next__, MagicMock) + self.assertIsInstance(a_mock.__len__, MagicMock) + + def test_magicmock_has_async_magic_methods(self): + m_mock = MagicMock() + self.assertTrue(hasattr(m_mock, "__aenter__")) + self.assertTrue(hasattr(m_mock, "__aexit__")) + self.assertTrue(hasattr(m_mock, "__anext__")) + + def test_asyncmock_has_sync_magic_methods(self): + a_mock = AsyncMock() + self.assertTrue(hasattr(a_mock, "__enter__")) + self.assertTrue(hasattr(a_mock, "__exit__")) + self.assertTrue(hasattr(a_mock, "__next__")) + self.assertTrue(hasattr(a_mock, "__len__")) + + def test_magic_methods_are_async_functions(self): + m_mock = MagicMock() + self.assertIsInstance(m_mock.__aenter__, AsyncMock) + self.assertIsInstance(m_mock.__aexit__, AsyncMock) + # AsyncMocks are also coroutine functions + self.assertTrue(asyncio.iscoroutinefunction(m_mock.__aenter__)) + self.assertTrue(asyncio.iscoroutinefunction(m_mock.__aexit__)) class AsyncContextManagerTest(unittest.TestCase): class WithAsyncContextManager: @@ -402,24 +439,6 @@ def __init__(self): val = await response.json() return val - def test_async_magic_methods_are_async_mocks_with_magicmock(self): - cm_mock = MagicMock(self.WithAsyncContextManager()) - self.assertIsInstance(cm_mock.__aenter__, AsyncMock) - self.assertIsInstance(cm_mock.__aexit__, AsyncMock) - - def test_magicmock_has_async_magic_methods(self): - cm = MagicMock(name='magic_cm') - self.assertTrue(hasattr(cm, "__aenter__")) - self.assertTrue(hasattr(cm, "__aexit__")) - - def test_magic_methods_are_async_functions(self): - cm = MagicMock(name='magic_cm') - self.assertIsInstance(cm.__aenter__, AsyncMock) - self.assertIsInstance(cm.__aexit__, AsyncMock) - # AsyncMocks are also coroutine functions - self.assertTrue(asyncio.iscoroutinefunction(cm.__aenter__)) - self.assertTrue(asyncio.iscoroutinefunction(cm.__aexit__)) - def test_set_return_value_of_aenter(self): def inner_test(mock_type): pc = self.ProductionCode() diff --git a/Lib/unittest/test/testmock/testmagicmethods.py b/Lib/unittest/test/testmock/testmagicmethods.py index 57f85e951e20..76b3a560de09 100644 --- a/Lib/unittest/test/testmock/testmagicmethods.py +++ b/Lib/unittest/test/testmock/testmagicmethods.py @@ -271,9 +271,6 @@ def test_magic_mock_equality(self): self.assertEqual(mock == mock, True) self.assertEqual(mock != mock, False) - - # This should be fixed with issue38163 - @unittest.expectedFailure def test_asyncmock_defaults(self): mock = AsyncMock() self.assertEqual(int(mock), 1) diff --git a/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst b/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst new file mode 100644 index 000000000000..d7eea367e482 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst @@ -0,0 +1,2 @@ +Any synchronous magic methods on an AsyncMock now return a MagicMock. Any +asynchronous magic methods on a MagicMock now return an AsyncMock. [View Less]

1 0

bpo-38161: Removes _AwaitEvent from AsyncMock. (GH-16443)
by Lisa Roach Sept. 30, 2019

Sept. 30, 2019

https://github.com/python/cpython/commit/25e115ec00b5f75e3589c9f21013c47c21… commit: 25e115ec00b5f75e3589c9f21013c47c21e1753f branch: master author: Lisa Roach <lisaroach14(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2019-09-29T21:01:28-07:00 summary: bpo-38161: Removes _AwaitEvent from AsyncMock. (GH-16443) files: A Misc/NEWS.d/next/Library/2019-09-27-16-31-28.bpo-38161.zehai1.rst M Lib/unittest/mock.py M Lib/unittest/test/testmock/testasync.py diff --git a/Lib/… [View More]unittest/mock.py b/Lib/unittest/mock.py index 27aa3bc65c9a..e797d2f5b5e1 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -246,7 +246,6 @@ def _setup_async_mock(mock): mock.await_count = 0 mock.await_args = None mock.await_args_list = _CallList() - mock.awaited = _AwaitEvent(mock) # Mock is not configured yet so the attributes are set # to a function and then the corresponding mock helper function @@ -2116,7 +2115,6 @@ def __get__(self, obj, _type=None): class AsyncMockMixin(Base): - awaited = _delegating_property('awaited') await_count = _delegating_property('await_count') await_args = _delegating_property('await_args') await_args_list = _delegating_property('await_args_list') @@ -2130,7 +2128,6 @@ def __init__(self, /, *args, **kwargs): # It is set through __dict__ because when spec_set is True, this # attribute is likely undefined. self.__dict__['_is_coroutine'] = asyncio.coroutines._is_coroutine - self.__dict__['_mock_awaited'] = _AwaitEvent(self) self.__dict__['_mock_await_count'] = 0 self.__dict__['_mock_await_args'] = None self.__dict__['_mock_await_args_list'] = _CallList() @@ -2159,7 +2156,6 @@ def __init__(self, /, *args, **kwargs): self.await_count += 1 self.await_args = _call self.await_args_list.append(_call) - await self.awaited._notify() return await proxy() @@ -2890,35 +2886,3 @@ def __aiter__(self): except StopIteration: pass raise StopAsyncIteration - - -class _AwaitEvent: - def __init__(self, mock): - self._mock = mock - self._condition = None - - async def _notify(self): - condition = self._get_condition() - try: - await condition.acquire() - condition.notify_all() - finally: - condition.release() - - def _get_condition(self): - """ - Creation of condition is delayed, to minimize the chance of using the - wrong loop. - A user may create a mock with _AwaitEvent before selecting the - execution loop. Requiring a user to delay creation is error-prone and - inflexible. Instead, condition is created when user actually starts to - use the mock. - """ - # No synchronization is needed: - # - asyncio is thread unsafe - # - there are no awaits here, method will be executed without - # switching asyncio context. - if self._condition is None: - self._condition = asyncio.Condition() - - return self._condition diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index 624fadca2649..5a9e93e40bc4 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -4,7 +4,7 @@ import unittest from unittest.mock import (ANY, call, AsyncMock, patch, MagicMock, - create_autospec, _AwaitEvent, sentinel, _CallList) + create_autospec, sentinel, _CallList) def tearDownModule(): @@ -178,7 +178,6 @@ def test_create_autospec(self): self.assertEqual(spec.await_count, 0) self.assertIsNone(spec.await_args) self.assertEqual(spec.await_args_list, []) - self.assertIsInstance(spec.awaited, _AwaitEvent) spec.assert_not_awaited() asyncio.run(main()) @@ -212,7 +211,6 @@ def test_patch_with_autospec(self): self.assertEqual(mock_method.await_count, 0) self.assertEqual(mock_method.await_args_list, []) self.assertIsNone(mock_method.await_args) - self.assertIsInstance(mock_method.awaited, _AwaitEvent) mock_method.assert_not_awaited() await awaitable diff --git a/Misc/NEWS.d/next/Library/2019-09-27-16-31-28.bpo-38161.zehai1.rst b/Misc/NEWS.d/next/Library/2019-09-27-16-31-28.bpo-38161.zehai1.rst new file mode 100644 index 000000000000..0077033c654f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-27-16-31-28.bpo-38161.zehai1.rst @@ -0,0 +1 @@ +Removes _AwaitEvent from AsyncMock. [View Less]

1 0

bpo-38317: Fix PyConfig.warnoptions priority (GH-16478)
by Miss Islington (bot) Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/c9ed9e6fc76323ed537fb79d4232bcd27d… commit: c9ed9e6fc76323ed537fb79d4232bcd27d82c57e branch: 3.8 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date: 2019-09-29T16:58:57-07:00 summary: bpo-38317: Fix PyConfig.warnoptions priority (GH-16478) Fix warnings options priority: PyConfig.warnoptions has the highest priority, as stated in the PEP 587. * Document options … [View More]order in PyConfig.warnoptions documentation. * Make PyWideStringList_INIT macro private: replace "Py" prefix with "_Py". * test_embed: add test_init_warnoptions(). (cherry picked from commit fb4ae152a9930f0e00cae8b2807f534058cf341a) Co-authored-by: Victor Stinner <vstinner(a)redhat.com> files: A Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst M Doc/c-api/init_config.rst M Include/cpython/initconfig.h M Include/internal/pycore_initconfig.h M Include/internal/pycore_pylifecycle.h M Lib/test/test_embed.py M Programs/_testembed.c M Python/initconfig.c M Python/preconfig.c M Python/sysmodule.c diff --git a/Doc/c-api/init_config.rst b/Doc/c-api/init_config.rst index 58e417441d13..4c77d5d1e0e8 100644 --- a/Doc/c-api/init_config.rst +++ b/Doc/c-api/init_config.rst @@ -704,7 +704,13 @@ PyConfig .. c:member:: PyWideStringList warnoptions - Options of the :mod:`warnings` module to build warnings filters. + :data:`sys.warnoptions`: options of the :mod:`warnings` module to build + warnings filters: lowest to highest priority. + + The :mod:`warnings` module adds :data:`sys.warnoptions` in the reverse + order: the last :c:member:`PyConfig.warnoptions` item becomes the first + item of :data:`warnings.filters` which is checked first (highest + priority). .. c:member:: int write_bytecode diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index d5effb8397fc..8ce5622c5c40 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -220,7 +220,10 @@ typedef struct { wchar_t *program_name; PyWideStringList xoptions; /* Command line -X options */ - PyWideStringList warnoptions; /* Warnings options */ + + /* Warnings options: lowest to highest priority. warnings.filters + is built in the reverse order (highest to lowest priority). */ + PyWideStringList warnoptions; /* If equal to zero, disable the import of the module site and the site-dependent manipulations of sys.path that it entails. Also disable diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h index dcdb61695cab..eb6490f5a87f 100644 --- a/Include/internal/pycore_initconfig.h +++ b/Include/internal/pycore_initconfig.h @@ -45,7 +45,7 @@ extern "C" { /* --- PyWideStringList ------------------------------------------------ */ -#define PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} +#define _PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} #ifndef NDEBUG PyAPI_FUNC(int) _PyWideStringList_CheckConsistency(const PyWideStringList *list); diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index cb568d28fb99..d4f0ae2da72e 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -41,7 +41,7 @@ extern PyStatus _PySys_Create( PyInterpreterState *interp, PyObject **sysmod_p); extern PyStatus _PySys_SetPreliminaryStderr(PyObject *sysdict); -extern PyStatus _PySys_ReadPreinitWarnOptions(PyConfig *config); +extern PyStatus _PySys_ReadPreinitWarnOptions(PyWideStringList *options); extern PyStatus _PySys_ReadPreinitXOptions(PyConfig *config); extern int _PySys_InitMain( _PyRuntimeState *runtime, diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index ed2b96fbfc1f..da79d7af0502 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -746,9 +746,9 @@ def test_init_from_config(self): 'cmdline_xoption', ], 'warnoptions': [ - 'config_warnoption', 'cmdline_warnoption', 'default::BytesWarning', + 'config_warnoption', ], 'run_command': 'pass\n', @@ -951,9 +951,9 @@ def test_init_sys_add(self): 'faulthandler', ], 'warnoptions': [ - 'ignore:::config_warnoption', 'ignore:::cmdline_warnoption', 'ignore:::sysadd_warnoption', + 'ignore:::config_warnoption', ], } self.check_all_configs("test_init_sys_add", config, api=API_PYTHON) @@ -1267,6 +1267,30 @@ def get_func(name): self.assertEqual(Py_GetProgramFullPath(), config['executable']) self.assertEqual(Py_GetPythonHome(), config['home']) + def test_init_warnoptions(self): + # lowest to highest priority + warnoptions = [ + 'ignore:::PyConfig_Insert0', # PyWideStringList_Insert(0) + 'default', # PyConfig.dev_mode=1 + 'ignore:::env1', # PYTHONWARNINGS env var + 'ignore:::env2', # PYTHONWARNINGS env var + 'ignore:::cmdline1', # -W opt command line option + 'ignore:::cmdline2', # -W opt command line option + 'default::BytesWarning', # PyConfig.bytes_warnings=1 + 'ignore:::PySys_AddWarnOption1', # PySys_AddWarnOption() + 'ignore:::PySys_AddWarnOption2', # PySys_AddWarnOption() + 'ignore:::PyConfig_BeforeRead', # PyConfig.warnoptions + 'ignore:::PyConfig_AfterRead'] # PyWideStringList_Append() + preconfig = dict(allocator=PYMEM_ALLOCATOR_DEBUG) + config = { + 'dev_mode': 1, + 'faulthandler': 1, + 'bytes_warning': 1, + 'warnoptions': warnoptions, + } + self.check_all_configs("test_init_warnoptions", config, preconfig, + api=API_PYTHON) + class AuditingTests(EmbeddingTestsMixin, unittest.TestCase): def test_open_code_hook(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst new file mode 100644 index 000000000000..b6d07474cf52 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst @@ -0,0 +1,2 @@ +Fix warnings options priority: ``PyConfig.warnoptions`` has the highest +priority, as stated in the :pep:`587`. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index a37594524f3d..c8600d58f0b4 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1603,6 +1603,64 @@ static int test_init_setpythonhome(void) } +static int test_init_warnoptions(void) +{ + PyStatus status; + putenv("PYTHONWARNINGS=ignore:::env1,ignore:::env2"); + + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption1"); + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption2"); + + PyConfig config; + config.struct_size = sizeof(PyConfig); + + status = PyConfig_InitPythonConfig(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + config.dev_mode = 1; + config.bytes_warning = 1; + + config_set_program_name(&config); + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_BeforeRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + wchar_t* argv[] = { + L"python3", + L"-Wignore:::cmdline1", + L"-Wignore:::cmdline2"}; + config_set_argv(&config, Py_ARRAY_LENGTH(argv), argv); + config.parse_argv = 1; + + status = PyConfig_Read(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_AfterRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Insert(&config.warnoptions, + 0, L"ignore:::PyConfig_Insert0"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + init_from_config_clear(&config); + dump_config(); + Py_Finalize(); + return 0; +} + + static void configure_init_main(PyConfig *config) { wchar_t* argv[] = { @@ -1746,6 +1804,7 @@ static struct TestCase TestCases[] = { {"test_init_setpath", test_init_setpath}, {"test_init_setpath_config", test_init_setpath_config}, {"test_init_setpythonhome", test_init_setpythonhome}, + {"test_init_warnoptions", test_init_warnoptions}, {"test_run_main", test_run_main}, {"test_open_code_hook", test_open_code_hook}, diff --git a/Python/initconfig.c b/Python/initconfig.c index dbc9de55a582..fb04b4a2d22b 100644 --- a/Python/initconfig.c +++ b/Python/initconfig.c @@ -273,7 +273,7 @@ _PyWideStringList_Copy(PyWideStringList *list, const PyWideStringList *list2) return 0; } - PyWideStringList copy = PyWideStringList_INIT; + PyWideStringList copy = _PyWideStringList_INIT; size_t size = list2->length * sizeof(list2->items[0]); copy.items = PyMem_RawMalloc(size); @@ -2095,63 +2095,83 @@ config_init_env_warnoptions(PyConfig *config, PyWideStringList *warnoptions) static PyStatus -config_add_warnoption(PyConfig *config, const wchar_t *option) +warnoptions_append(PyConfig *config, PyWideStringList *options, + const wchar_t *option) { + /* config_init_warnoptions() add existing config warnoptions at the end: + ensure that the new option is not already present in this list to + prevent change the options order whne config_init_warnoptions() is + called twice. */ if (_PyWideStringList_Find(&config->warnoptions, option)) { /* Already present: do nothing */ return _PyStatus_OK(); } - return PyWideStringList_Append(&config->warnoptions, option); + if (_PyWideStringList_Find(options, option)) { + /* Already present: do nothing */ + return _PyStatus_OK(); + } + return PyWideStringList_Append(options, option); +} + + +static PyStatus +warnoptions_extend(PyConfig *config, PyWideStringList *options, + const PyWideStringList *options2) +{ + const Py_ssize_t len = options2->length; + wchar_t *const *items = options2->items; + + for (Py_ssize_t i = 0; i < len; i++) { + PyStatus status = warnoptions_append(config, options, items[i]); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + } + return _PyStatus_OK(); } static PyStatus config_init_warnoptions(PyConfig *config, const PyWideStringList *cmdline_warnoptions, - const PyWideStringList *env_warnoptions) + const PyWideStringList *env_warnoptions, + const PyWideStringList *sys_warnoptions) { PyStatus status; + PyWideStringList options = _PyWideStringList_INIT; - /* The priority order for warnings configuration is (highest precedence - * first): + /* Priority of warnings options, lowest to highest: * - * - early PySys_AddWarnOption() calls - * - the BytesWarning filter, if needed ('-b', '-bb') - * - any '-W' command line options; then - * - the 'PYTHONWARNINGS' environment variable; then - * - the dev mode filter ('-X dev', 'PYTHONDEVMODE'); then * - any implicit filters added by _warnings.c/warnings.py + * - PyConfig.dev_mode: "default" filter + * - PYTHONWARNINGS environment variable + * - '-W' command line options + * - PyConfig.bytes_warning ('-b' and '-bb' command line options): + * "default::BytesWarning" or "error::BytesWarning" filter + * - early PySys_AddWarnOption() calls + * - PyConfig.warnoptions * - * All settings except the last are passed to the warnings module via - * the `sys.warnoptions` list. Since the warnings module works on the basis - * of "the most recently added filter will be checked first", we add - * the lowest precedence entries first so that later entries override them. + * PyConfig.warnoptions is copied to sys.warnoptions. Since the warnings + * module works on the basis of "the most recently added filter will be + * checked first", we add the lowest precedence entries first so that later + * entries override them. */ if (config->dev_mode) { - status = config_add_warnoption(config, L"default"); + status = warnoptions_append(config, &options, L"default"); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - Py_ssize_t i; - const PyWideStringList *options; - - options = env_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, env_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } - options = cmdline_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, cmdline_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } /* If the bytes_warning_flag isn't set, bytesobject.c and bytearrayobject.c @@ -2166,19 +2186,30 @@ config_init_warnoptions(PyConfig *config, else { filter = L"default::BytesWarning"; } - status = config_add_warnoption(config, filter); + status = warnoptions_append(config, &options, filter); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - /* Handle early PySys_AddWarnOption() calls */ - status = _PySys_ReadPreinitWarnOptions(config); + status = warnoptions_extend(config, &options, sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; + } + + /* Always add all PyConfig.warnoptions options */ + status = _PyWideStringList_Extend(&options, &config->warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } + _PyWideStringList_Clear(&config->warnoptions); + config->warnoptions = options; return _PyStatus_OK(); + +error: + _PyWideStringList_Clear(&options); + return status; } @@ -2186,7 +2217,7 @@ static PyStatus config_update_argv(PyConfig *config, Py_ssize_t opt_index) { const PyWideStringList *cmdline_argv = &config->argv; - PyWideStringList config_argv = PyWideStringList_INIT; + PyWideStringList config_argv = _PyWideStringList_INIT; /* Copy argv to be able to modify it (to force -c/-m) */ if (cmdline_argv->length <= opt_index) { @@ -2270,8 +2301,9 @@ static PyStatus config_read_cmdline(PyConfig *config) { PyStatus status; - PyWideStringList cmdline_warnoptions = PyWideStringList_INIT; - PyWideStringList env_warnoptions = PyWideStringList_INIT; + PyWideStringList cmdline_warnoptions = _PyWideStringList_INIT; + PyWideStringList env_warnoptions = _PyWideStringList_INIT; + PyWideStringList sys_warnoptions = _PyWideStringList_INIT; if (config->parse_argv < 0) { config->parse_argv = 1; @@ -2304,9 +2336,16 @@ config_read_cmdline(PyConfig *config) } } + /* Handle early PySys_AddWarnOption() calls */ + status = _PySys_ReadPreinitWarnOptions(&sys_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto done; + } + status = config_init_warnoptions(config, &cmdline_warnoptions, - &env_warnoptions); + &env_warnoptions, + &sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { goto done; } @@ -2316,6 +2355,7 @@ config_read_cmdline(PyConfig *config) done: _PyWideStringList_Clear(&cmdline_warnoptions); _PyWideStringList_Clear(&env_warnoptions); + _PyWideStringList_Clear(&sys_warnoptions); return status; } @@ -2386,7 +2426,7 @@ PyStatus PyConfig_Read(PyConfig *config) { PyStatus status; - PyWideStringList orig_argv = PyWideStringList_INIT; + PyWideStringList orig_argv = _PyWideStringList_INIT; status = config_check_struct_size(config); if (_PyStatus_EXCEPTION(status)) { diff --git a/Python/preconfig.c b/Python/preconfig.c index d18b01dc4586..01c72f5d6bad 100644 --- a/Python/preconfig.c +++ b/Python/preconfig.c @@ -75,7 +75,7 @@ _Py_SetFileSystemEncoding(const char *encoding, const char *errors) PyStatus _PyArgv_AsWstrList(const _PyArgv *args, PyWideStringList *list) { - PyWideStringList wargv = PyWideStringList_INIT; + PyWideStringList wargv = _PyWideStringList_INIT; if (args->use_bytes_argv) { size_t size = sizeof(wchar_t*) * args->argc; wargv.items = (wchar_t **)PyMem_RawMalloc(size); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 5175158a3b00..5b0fb813b456 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2071,13 +2071,13 @@ _clear_preinit_entries(_Py_PreInitEntry *optionlist) PyStatus -_PySys_ReadPreinitWarnOptions(PyConfig *config) +_PySys_ReadPreinitWarnOptions(PyWideStringList *options) { PyStatus status; _Py_PreInitEntry entry; for (entry = _preinit_warnoptions; entry != NULL; entry = entry->next) { - status = PyWideStringList_Append(&config->warnoptions, entry->value); + status = PyWideStringList_Append(options, entry->value); if (_PyStatus_EXCEPTION(status)) { return status; } [View Less]

1 0

bpo-38317: Fix PyConfig.warnoptions priority (GH-16478)
by Victor Stinner Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/fb4ae152a9930f0e00cae8b2807f534058… commit: fb4ae152a9930f0e00cae8b2807f534058cf341a branch: master author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2019-09-30T01:40:17+02:00 summary: bpo-38317: Fix PyConfig.warnoptions priority (GH-16478) Fix warnings options priority: PyConfig.warnoptions has the highest priority, as stated in the PEP 587. * Document options order in PyConfig.warnoptions … [View More]documentation. * Make PyWideStringList_INIT macro private: replace "Py" prefix with "_Py". * test_embed: add test_init_warnoptions(). files: A Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst M Doc/c-api/init_config.rst M Include/cpython/initconfig.h M Include/internal/pycore_initconfig.h M Include/internal/pycore_pylifecycle.h M Lib/test/test_embed.py M Programs/_testembed.c M Python/initconfig.c M Python/preconfig.c M Python/sysmodule.c diff --git a/Doc/c-api/init_config.rst b/Doc/c-api/init_config.rst index 58e417441d13..4c77d5d1e0e8 100644 --- a/Doc/c-api/init_config.rst +++ b/Doc/c-api/init_config.rst @@ -704,7 +704,13 @@ PyConfig .. c:member:: PyWideStringList warnoptions - Options of the :mod:`warnings` module to build warnings filters. + :data:`sys.warnoptions`: options of the :mod:`warnings` module to build + warnings filters: lowest to highest priority. + + The :mod:`warnings` module adds :data:`sys.warnoptions` in the reverse + order: the last :c:member:`PyConfig.warnoptions` item becomes the first + item of :data:`warnings.filters` which is checked first (highest + priority). .. c:member:: int write_bytecode diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index d5effb8397fc..8ce5622c5c40 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -220,7 +220,10 @@ typedef struct { wchar_t *program_name; PyWideStringList xoptions; /* Command line -X options */ - PyWideStringList warnoptions; /* Warnings options */ + + /* Warnings options: lowest to highest priority. warnings.filters + is built in the reverse order (highest to lowest priority). */ + PyWideStringList warnoptions; /* If equal to zero, disable the import of the module site and the site-dependent manipulations of sys.path that it entails. Also disable diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h index dcdb61695cab..eb6490f5a87f 100644 --- a/Include/internal/pycore_initconfig.h +++ b/Include/internal/pycore_initconfig.h @@ -45,7 +45,7 @@ extern "C" { /* --- PyWideStringList ------------------------------------------------ */ -#define PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} +#define _PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} #ifndef NDEBUG PyAPI_FUNC(int) _PyWideStringList_CheckConsistency(const PyWideStringList *list); diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index cdf5c09a4262..b0a98f5d2597 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -41,7 +41,7 @@ extern PyStatus _PySys_Create( PyThreadState *tstate, PyObject **sysmod_p); extern PyStatus _PySys_SetPreliminaryStderr(PyObject *sysdict); -extern PyStatus _PySys_ReadPreinitWarnOptions(PyConfig *config); +extern PyStatus _PySys_ReadPreinitWarnOptions(PyWideStringList *options); extern PyStatus _PySys_ReadPreinitXOptions(PyConfig *config); extern int _PySys_InitMain( _PyRuntimeState *runtime, diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index ab086e2332da..b87863a372a6 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -746,9 +746,9 @@ def test_init_from_config(self): 'cmdline_xoption', ], 'warnoptions': [ - 'config_warnoption', 'cmdline_warnoption', 'default::BytesWarning', + 'config_warnoption', ], 'run_command': 'pass\n', @@ -952,9 +952,9 @@ def test_init_sys_add(self): 'faulthandler', ], 'warnoptions': [ - 'ignore:::config_warnoption', 'ignore:::cmdline_warnoption', 'ignore:::sysadd_warnoption', + 'ignore:::config_warnoption', ], } self.check_all_configs("test_init_sys_add", config, api=API_PYTHON) @@ -1268,6 +1268,30 @@ def get_func(name): self.assertEqual(Py_GetProgramFullPath(), config['executable']) self.assertEqual(Py_GetPythonHome(), config['home']) + def test_init_warnoptions(self): + # lowest to highest priority + warnoptions = [ + 'ignore:::PyConfig_Insert0', # PyWideStringList_Insert(0) + 'default', # PyConfig.dev_mode=1 + 'ignore:::env1', # PYTHONWARNINGS env var + 'ignore:::env2', # PYTHONWARNINGS env var + 'ignore:::cmdline1', # -W opt command line option + 'ignore:::cmdline2', # -W opt command line option + 'default::BytesWarning', # PyConfig.bytes_warnings=1 + 'ignore:::PySys_AddWarnOption1', # PySys_AddWarnOption() + 'ignore:::PySys_AddWarnOption2', # PySys_AddWarnOption() + 'ignore:::PyConfig_BeforeRead', # PyConfig.warnoptions + 'ignore:::PyConfig_AfterRead'] # PyWideStringList_Append() + preconfig = dict(allocator=PYMEM_ALLOCATOR_DEBUG) + config = { + 'dev_mode': 1, + 'faulthandler': 1, + 'bytes_warning': 1, + 'warnoptions': warnoptions, + } + self.check_all_configs("test_init_warnoptions", config, preconfig, + api=API_PYTHON) + class AuditingTests(EmbeddingTestsMixin, unittest.TestCase): def test_open_code_hook(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst new file mode 100644 index 000000000000..b6d07474cf52 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst @@ -0,0 +1,2 @@ +Fix warnings options priority: ``PyConfig.warnoptions`` has the highest +priority, as stated in the :pep:`587`. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index a37594524f3d..c8600d58f0b4 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1603,6 +1603,64 @@ static int test_init_setpythonhome(void) } +static int test_init_warnoptions(void) +{ + PyStatus status; + putenv("PYTHONWARNINGS=ignore:::env1,ignore:::env2"); + + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption1"); + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption2"); + + PyConfig config; + config.struct_size = sizeof(PyConfig); + + status = PyConfig_InitPythonConfig(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + config.dev_mode = 1; + config.bytes_warning = 1; + + config_set_program_name(&config); + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_BeforeRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + wchar_t* argv[] = { + L"python3", + L"-Wignore:::cmdline1", + L"-Wignore:::cmdline2"}; + config_set_argv(&config, Py_ARRAY_LENGTH(argv), argv); + config.parse_argv = 1; + + status = PyConfig_Read(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_AfterRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Insert(&config.warnoptions, + 0, L"ignore:::PyConfig_Insert0"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + init_from_config_clear(&config); + dump_config(); + Py_Finalize(); + return 0; +} + + static void configure_init_main(PyConfig *config) { wchar_t* argv[] = { @@ -1746,6 +1804,7 @@ static struct TestCase TestCases[] = { {"test_init_setpath", test_init_setpath}, {"test_init_setpath_config", test_init_setpath_config}, {"test_init_setpythonhome", test_init_setpythonhome}, + {"test_init_warnoptions", test_init_warnoptions}, {"test_run_main", test_run_main}, {"test_open_code_hook", test_open_code_hook}, diff --git a/Python/initconfig.c b/Python/initconfig.c index 62c868d5cbc9..dec4bf2e4d57 100644 --- a/Python/initconfig.c +++ b/Python/initconfig.c @@ -273,7 +273,7 @@ _PyWideStringList_Copy(PyWideStringList *list, const PyWideStringList *list2) return 0; } - PyWideStringList copy = PyWideStringList_INIT; + PyWideStringList copy = _PyWideStringList_INIT; size_t size = list2->length * sizeof(list2->items[0]); copy.items = PyMem_RawMalloc(size); @@ -2095,63 +2095,83 @@ config_init_env_warnoptions(PyConfig *config, PyWideStringList *warnoptions) static PyStatus -config_add_warnoption(PyConfig *config, const wchar_t *option) +warnoptions_append(PyConfig *config, PyWideStringList *options, + const wchar_t *option) { + /* config_init_warnoptions() add existing config warnoptions at the end: + ensure that the new option is not already present in this list to + prevent change the options order whne config_init_warnoptions() is + called twice. */ if (_PyWideStringList_Find(&config->warnoptions, option)) { /* Already present: do nothing */ return _PyStatus_OK(); } - return PyWideStringList_Append(&config->warnoptions, option); + if (_PyWideStringList_Find(options, option)) { + /* Already present: do nothing */ + return _PyStatus_OK(); + } + return PyWideStringList_Append(options, option); +} + + +static PyStatus +warnoptions_extend(PyConfig *config, PyWideStringList *options, + const PyWideStringList *options2) +{ + const Py_ssize_t len = options2->length; + wchar_t *const *items = options2->items; + + for (Py_ssize_t i = 0; i < len; i++) { + PyStatus status = warnoptions_append(config, options, items[i]); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + } + return _PyStatus_OK(); } static PyStatus config_init_warnoptions(PyConfig *config, const PyWideStringList *cmdline_warnoptions, - const PyWideStringList *env_warnoptions) + const PyWideStringList *env_warnoptions, + const PyWideStringList *sys_warnoptions) { PyStatus status; + PyWideStringList options = _PyWideStringList_INIT; - /* The priority order for warnings configuration is (highest precedence - * first): + /* Priority of warnings options, lowest to highest: * - * - early PySys_AddWarnOption() calls - * - the BytesWarning filter, if needed ('-b', '-bb') - * - any '-W' command line options; then - * - the 'PYTHONWARNINGS' environment variable; then - * - the dev mode filter ('-X dev', 'PYTHONDEVMODE'); then * - any implicit filters added by _warnings.c/warnings.py + * - PyConfig.dev_mode: "default" filter + * - PYTHONWARNINGS environment variable + * - '-W' command line options + * - PyConfig.bytes_warning ('-b' and '-bb' command line options): + * "default::BytesWarning" or "error::BytesWarning" filter + * - early PySys_AddWarnOption() calls + * - PyConfig.warnoptions * - * All settings except the last are passed to the warnings module via - * the `sys.warnoptions` list. Since the warnings module works on the basis - * of "the most recently added filter will be checked first", we add - * the lowest precedence entries first so that later entries override them. + * PyConfig.warnoptions is copied to sys.warnoptions. Since the warnings + * module works on the basis of "the most recently added filter will be + * checked first", we add the lowest precedence entries first so that later + * entries override them. */ if (config->dev_mode) { - status = config_add_warnoption(config, L"default"); + status = warnoptions_append(config, &options, L"default"); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - Py_ssize_t i; - const PyWideStringList *options; - - options = env_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, env_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } - options = cmdline_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, cmdline_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } /* If the bytes_warning_flag isn't set, bytesobject.c and bytearrayobject.c @@ -2166,19 +2186,30 @@ config_init_warnoptions(PyConfig *config, else { filter = L"default::BytesWarning"; } - status = config_add_warnoption(config, filter); + status = warnoptions_append(config, &options, filter); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - /* Handle early PySys_AddWarnOption() calls */ - status = _PySys_ReadPreinitWarnOptions(config); + status = warnoptions_extend(config, &options, sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; + } + + /* Always add all PyConfig.warnoptions options */ + status = _PyWideStringList_Extend(&options, &config->warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } + _PyWideStringList_Clear(&config->warnoptions); + config->warnoptions = options; return _PyStatus_OK(); + +error: + _PyWideStringList_Clear(&options); + return status; } @@ -2186,7 +2217,7 @@ static PyStatus config_update_argv(PyConfig *config, Py_ssize_t opt_index) { const PyWideStringList *cmdline_argv = &config->argv; - PyWideStringList config_argv = PyWideStringList_INIT; + PyWideStringList config_argv = _PyWideStringList_INIT; /* Copy argv to be able to modify it (to force -c/-m) */ if (cmdline_argv->length <= opt_index) { @@ -2306,8 +2337,9 @@ static PyStatus config_read_cmdline(PyConfig *config) { PyStatus status; - PyWideStringList cmdline_warnoptions = PyWideStringList_INIT; - PyWideStringList env_warnoptions = PyWideStringList_INIT; + PyWideStringList cmdline_warnoptions = _PyWideStringList_INIT; + PyWideStringList env_warnoptions = _PyWideStringList_INIT; + PyWideStringList sys_warnoptions = _PyWideStringList_INIT; if (config->parse_argv < 0) { config->parse_argv = 1; @@ -2351,9 +2383,16 @@ config_read_cmdline(PyConfig *config) } } + /* Handle early PySys_AddWarnOption() calls */ + status = _PySys_ReadPreinitWarnOptions(&sys_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto done; + } + status = config_init_warnoptions(config, &cmdline_warnoptions, - &env_warnoptions); + &env_warnoptions, + &sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { goto done; } @@ -2363,6 +2402,7 @@ config_read_cmdline(PyConfig *config) done: _PyWideStringList_Clear(&cmdline_warnoptions); _PyWideStringList_Clear(&env_warnoptions); + _PyWideStringList_Clear(&sys_warnoptions); return status; } @@ -2433,7 +2473,7 @@ PyStatus PyConfig_Read(PyConfig *config) { PyStatus status; - PyWideStringList orig_argv = PyWideStringList_INIT; + PyWideStringList orig_argv = _PyWideStringList_INIT; status = config_check_struct_size(config); if (_PyStatus_EXCEPTION(status)) { diff --git a/Python/preconfig.c b/Python/preconfig.c index d18b01dc4586..01c72f5d6bad 100644 --- a/Python/preconfig.c +++ b/Python/preconfig.c @@ -75,7 +75,7 @@ _Py_SetFileSystemEncoding(const char *encoding, const char *errors) PyStatus _PyArgv_AsWstrList(const _PyArgv *args, PyWideStringList *list) { - PyWideStringList wargv = PyWideStringList_INIT; + PyWideStringList wargv = _PyWideStringList_INIT; if (args->use_bytes_argv) { size_t size = sizeof(wchar_t*) * args->argc; wargv.items = (wchar_t **)PyMem_RawMalloc(size); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 9cc7b4b27c15..92935408a58e 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2037,13 +2037,13 @@ _clear_preinit_entries(_Py_PreInitEntry *optionlist) PyStatus -_PySys_ReadPreinitWarnOptions(PyConfig *config) +_PySys_ReadPreinitWarnOptions(PyWideStringList *options) { PyStatus status; _Py_PreInitEntry entry; for (entry = _preinit_warnoptions; entry != NULL; entry = entry->next) { - status = PyWideStringList_Append(&config->warnoptions, entry->value); + status = PyWideStringList_Append(options, entry->value); if (_PyStatus_EXCEPTION(status)) { return status; } [View Less]

1 0

bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472)
by Miss Islington (bot) Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/19cd5951ec4480c7cfcbcc379b36902312… commit: 19cd5951ec4480c7cfcbcc379b36902312cfb8b8 branch: 3.8 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date: 2019-09-29T05:20:15-07:00 summary: bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472) (cherry picked from commit 58498bc7178608b1ab031994ca09c43889ce3e76) Co-authored-by: Andrew … [View More]Svetlov <andrew.svetlov(a)gmail.com> files: A Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst M Lib/asyncio/unix_events.py M Lib/test/test_asyncio/test_unix_events.py diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index cbbb1065aeda..d8f653045aee 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -445,6 +445,7 @@ def __init__(self, loop, pipe, protocol, waiter=None, extra=None): self._fileno = pipe.fileno() self._protocol = protocol self._closing = False + self._paused = False mode = os.fstat(self._fileno).st_mode if not (stat.S_ISFIFO(mode) or @@ -506,10 +507,20 @@ def _read_ready(self): self._loop.call_soon(self._call_connection_lost, None) def pause_reading(self): + if self._closing or self._paused: + return + self._paused = True self._loop._remove_reader(self._fileno) + if self._loop.get_debug(): + logger.debug("%r pauses reading", self) def resume_reading(self): + if self._closing or not self._paused: + return + self._paused = False self._loop._add_reader(self._fileno, self._read_ready) + if self._loop.get_debug(): + logger.debug("%r resumes reading", self) def set_protocol(self, protocol): self._protocol = protocol diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index 02ce7e89da09..5487b7afef83 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -736,6 +736,7 @@ def test_pause_reading(self, m_read): @mock.patch('os.read') def test_resume_reading(self, m_read): tr = self.read_pipe_transport() + tr.pause_reading() tr.resume_reading() self.loop.assert_reader(5, tr._read_ready) @@ -790,6 +791,32 @@ def test__call_connection_lost_with_err(self): self.assertIsNone(tr._protocol) self.assertIsNone(tr._loop) + def test_pause_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.pause_reading() + + def test_pause_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + tr.pause_reading() + # the second call should do nothing + tr.pause_reading() + + def test_resume_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.resume_reading() + + def test_resume_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + # the pipe is not paused + # resuming should do nothing + tr.resume_reading() + class UnixWritePipeTransportTests(test_utils.TestCase): diff --git a/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst new file mode 100644 index 000000000000..aa5a23de2460 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst @@ -0,0 +1 @@ +Correctly handle pause/resume reading of closed asyncio unix pipe. [View Less]

1 0

bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472)
by Miss Islington (bot) Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/1c3e4691bb41fa070d6f4836cb03005123… commit: 1c3e4691bb41fa070d6f4836cb03005123e53e4b branch: 3.7 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date: 2019-09-29T05:19:11-07:00 summary: bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472) (cherry picked from commit 58498bc7178608b1ab031994ca09c43889ce3e76) Co-authored-by: Andrew … [View More]Svetlov <andrew.svetlov(a)gmail.com> files: A Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst M Lib/asyncio/unix_events.py M Lib/test/test_asyncio/test_unix_events.py diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index a0fc996d23a0..7034fc28d3e1 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -431,6 +431,7 @@ def __init__(self, loop, pipe, protocol, waiter=None, extra=None): self._fileno = pipe.fileno() self._protocol = protocol self._closing = False + self._paused = False mode = os.fstat(self._fileno).st_mode if not (stat.S_ISFIFO(mode) or @@ -492,10 +493,20 @@ def _read_ready(self): self._loop.call_soon(self._call_connection_lost, None) def pause_reading(self): + if self._closing or self._paused: + return + self._paused = True self._loop._remove_reader(self._fileno) + if self._loop.get_debug(): + logger.debug("%r pauses reading", self) def resume_reading(self): + if self._closing or not self._paused: + return + self._paused = False self._loop._add_reader(self._fileno, self._read_ready) + if self._loop.get_debug(): + logger.debug("%r resumes reading", self) def set_protocol(self, protocol): self._protocol = protocol diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index ec171fa83da3..66a6bc168031 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -722,6 +722,7 @@ def test_pause_reading(self, m_read): @mock.patch('os.read') def test_resume_reading(self, m_read): tr = self.read_pipe_transport() + tr.pause_reading() tr.resume_reading() self.loop.assert_reader(5, tr._read_ready) @@ -776,6 +777,32 @@ def test__call_connection_lost_with_err(self): self.assertIsNone(tr._protocol) self.assertIsNone(tr._loop) + def test_pause_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.pause_reading() + + def test_pause_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + tr.pause_reading() + # the second call should do nothing + tr.pause_reading() + + def test_resume_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.resume_reading() + + def test_resume_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + # the pipe is not paused + # resuming should do nothing + tr.resume_reading() + class UnixWritePipeTransportTests(test_utils.TestCase): diff --git a/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst new file mode 100644 index 000000000000..aa5a23de2460 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst @@ -0,0 +1 @@ +Correctly handle pause/resume reading of closed asyncio unix pipe. [View Less]

1 0

bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472)
by Andrew Svetlov Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/58498bc7178608b1ab031994ca09c43889… commit: 58498bc7178608b1ab031994ca09c43889ce3e76 branch: master author: Andrew Svetlov <andrew.svetlov(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2019-09-29T15:00:35+03:00 summary: bpo-38019: correctly handle pause/resume reading of closed asyncio unix pipe (GH-16472) files: A Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst M Lib/asyncio/unix_events.py M Lib/test/… [View More]test_asyncio/test_unix_events.py diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index cbbb1065aeda..d8f653045aee 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -445,6 +445,7 @@ def __init__(self, loop, pipe, protocol, waiter=None, extra=None): self._fileno = pipe.fileno() self._protocol = protocol self._closing = False + self._paused = False mode = os.fstat(self._fileno).st_mode if not (stat.S_ISFIFO(mode) or @@ -506,10 +507,20 @@ def _read_ready(self): self._loop.call_soon(self._call_connection_lost, None) def pause_reading(self): + if self._closing or self._paused: + return + self._paused = True self._loop._remove_reader(self._fileno) + if self._loop.get_debug(): + logger.debug("%r pauses reading", self) def resume_reading(self): + if self._closing or not self._paused: + return + self._paused = False self._loop._add_reader(self._fileno, self._read_ready) + if self._loop.get_debug(): + logger.debug("%r resumes reading", self) def set_protocol(self, protocol): self._protocol = protocol diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index 02ce7e89da09..5487b7afef83 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -736,6 +736,7 @@ def test_pause_reading(self, m_read): @mock.patch('os.read') def test_resume_reading(self, m_read): tr = self.read_pipe_transport() + tr.pause_reading() tr.resume_reading() self.loop.assert_reader(5, tr._read_ready) @@ -790,6 +791,32 @@ def test__call_connection_lost_with_err(self): self.assertIsNone(tr._protocol) self.assertIsNone(tr._loop) + def test_pause_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.pause_reading() + + def test_pause_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + tr.pause_reading() + # the second call should do nothing + tr.pause_reading() + + def test_resume_reading_on_closed_pipe(self): + tr = self.read_pipe_transport() + tr.close() + test_utils.run_briefly(self.loop) + self.assertIsNone(tr._loop) + tr.resume_reading() + + def test_resume_reading_on_paused_pipe(self): + tr = self.read_pipe_transport() + # the pipe is not paused + # resuming should do nothing + tr.resume_reading() + class UnixWritePipeTransportTests(test_utils.TestCase): diff --git a/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst new file mode 100644 index 000000000000..aa5a23de2460 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-29-13-50-24.bpo-38019.6MoOE3.rst @@ -0,0 +1 @@ +Correctly handle pause/resume reading of closed asyncio unix pipe. [View Less]

1 0

bpo-38108: Makes mock objects inherit from Base (GH-16060)
by Lisa Roach Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/9a7d9519506ae807ca48ff02e2ea117eba… commit: 9a7d9519506ae807ca48ff02e2ea117ebac3450e branch: master author: Lisa Roach <lisaroach14(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2019-09-28T18:42:44-07:00 summary: bpo-38108: Makes mock objects inherit from Base (GH-16060) files: A Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst M Lib/unittest/mock.py M Lib/unittest/test/testmock/testasync.py M Lib/unittest/… [View More]test/testmock/testmagicmethods.py diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 0cd7af65b940..27aa3bc65c9a 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -409,7 +409,7 @@ def __new__(cls, /, *args, **kw): if spec_arg and _is_async_obj(spec_arg): bases = (AsyncMockMixin, cls) new = type(cls.__name__, bases, {'__doc__': cls.__doc__}) - instance = object.__new__(new) + instance = _safe_super(NonCallableMock, cls).__new__(new) return instance @@ -990,17 +990,18 @@ def _get_child_mock(self, /, **kw): _type = type(self) if issubclass(_type, MagicMock) and _new_name in _async_method_magics: + # Any asynchronous magic becomes an AsyncMock klass = AsyncMock - elif _new_name in _sync_async_magics: - # Special case these ones b/c users will assume they are async, - # but they are actually sync (ie. __aiter__) - klass = MagicMock elif issubclass(_type, AsyncMockMixin): - klass = AsyncMock + if _new_name in _all_sync_magics: + # Any synchronous magic becomes a MagicMock + klass = MagicMock + else: + klass = AsyncMock elif not issubclass(_type, CallableMixin): if issubclass(_type, NonCallableMagicMock): klass = MagicMock - elif issubclass(_type, NonCallableMock) : + elif issubclass(_type, NonCallableMock): klass = Mock else: klass = _type.__mro__[1] @@ -1886,6 +1887,7 @@ def _patch_stopall(): "round trunc floor ceil " "bool next " "fspath " + "aiter " ) numerics = ( @@ -2024,7 +2026,7 @@ def _set_return_value(mock, method, name): -class MagicMixin(object): +class MagicMixin(Base): def __init__(self, /, *args, **kw): self._mock_set_magics() # make magic work for kwargs in init _safe_super(MagicMixin, self).__init__(*args, **kw) @@ -2032,13 +2034,14 @@ def __init__(self, /, *args, **kw): def _mock_set_magics(self): - these_magics = _magics + orig_magics = _magics | _async_method_magics + these_magics = orig_magics if getattr(self, "_mock_methods", None) is not None: - these_magics = _magics.intersection(self._mock_methods) + these_magics = orig_magics.intersection(self._mock_methods) remove_magics = set() - remove_magics = _magics - these_magics + remove_magics = orig_magics - these_magics for entry in remove_magics: if entry in type(self).__dict__: @@ -2066,33 +2069,13 @@ def mock_add_spec(self, spec, spec_set=False): self._mock_set_magics() -class AsyncMagicMixin: +class AsyncMagicMixin(MagicMixin): def __init__(self, /, *args, **kw): - self._mock_set_async_magics() # make magic work for kwargs in init + self._mock_set_magics() # make magic work for kwargs in init _safe_super(AsyncMagicMixin, self).__init__(*args, **kw) - self._mock_set_async_magics() # fix magic broken by upper level init - - def _mock_set_async_magics(self): - these_magics = _async_magics - - if getattr(self, "_mock_methods", None) is not None: - these_magics = _async_magics.intersection(self._mock_methods) - remove_magics = _async_magics - these_magics - - for entry in remove_magics: - if entry in type(self).__dict__: - # remove unneeded magic methods - delattr(self, entry) - - # don't overwrite existing attributes if called a second time - these_magics = these_magics - set(type(self).__dict__) - - _type = type(self) - for entry in these_magics: - setattr(_type, entry, MagicProxy(entry, self)) - + self._mock_set_magics() # fix magic broken by upper level init -class MagicMock(MagicMixin, AsyncMagicMixin, Mock): +class MagicMock(MagicMixin, Mock): """ MagicMock is a subclass of Mock with default implementations of most of the magic methods. You can use MagicMock without having to @@ -2114,7 +2097,7 @@ def mock_add_spec(self, spec, spec_set=False): -class MagicProxy(object): +class MagicProxy(Base): def __init__(self, name, parent): self.name = name self.parent = parent diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py index fde1e4a690b2..624fadca2649 100644 --- a/Lib/unittest/test/testmock/testasync.py +++ b/Lib/unittest/test/testmock/testasync.py @@ -379,6 +379,43 @@ def test_add_side_effect_iterable(self): RuntimeError('coroutine raised StopIteration') ) +class AsyncMagicMethods(unittest.TestCase): + def test_async_magic_methods_return_async_mocks(self): + m_mock = MagicMock() + self.assertIsInstance(m_mock.__aenter__, AsyncMock) + self.assertIsInstance(m_mock.__aexit__, AsyncMock) + self.assertIsInstance(m_mock.__anext__, AsyncMock) + # __aiter__ is actually a synchronous object + # so should return a MagicMock + self.assertIsInstance(m_mock.__aiter__, MagicMock) + + def test_sync_magic_methods_return_magic_mocks(self): + a_mock = AsyncMock() + self.assertIsInstance(a_mock.__enter__, MagicMock) + self.assertIsInstance(a_mock.__exit__, MagicMock) + self.assertIsInstance(a_mock.__next__, MagicMock) + self.assertIsInstance(a_mock.__len__, MagicMock) + + def test_magicmock_has_async_magic_methods(self): + m_mock = MagicMock() + self.assertTrue(hasattr(m_mock, "__aenter__")) + self.assertTrue(hasattr(m_mock, "__aexit__")) + self.assertTrue(hasattr(m_mock, "__anext__")) + + def test_asyncmock_has_sync_magic_methods(self): + a_mock = AsyncMock() + self.assertTrue(hasattr(a_mock, "__enter__")) + self.assertTrue(hasattr(a_mock, "__exit__")) + self.assertTrue(hasattr(a_mock, "__next__")) + self.assertTrue(hasattr(a_mock, "__len__")) + + def test_magic_methods_are_async_functions(self): + m_mock = MagicMock() + self.assertIsInstance(m_mock.__aenter__, AsyncMock) + self.assertIsInstance(m_mock.__aexit__, AsyncMock) + # AsyncMocks are also coroutine functions + self.assertTrue(asyncio.iscoroutinefunction(m_mock.__aenter__)) + self.assertTrue(asyncio.iscoroutinefunction(m_mock.__aexit__)) class AsyncContextManagerTest(unittest.TestCase): @@ -406,24 +443,6 @@ def __init__(self): val = await response.json() return val - def test_async_magic_methods_are_async_mocks_with_magicmock(self): - cm_mock = MagicMock(self.WithAsyncContextManager()) - self.assertIsInstance(cm_mock.__aenter__, AsyncMock) - self.assertIsInstance(cm_mock.__aexit__, AsyncMock) - - def test_magicmock_has_async_magic_methods(self): - cm = MagicMock(name='magic_cm') - self.assertTrue(hasattr(cm, "__aenter__")) - self.assertTrue(hasattr(cm, "__aexit__")) - - def test_magic_methods_are_async_functions(self): - cm = MagicMock(name='magic_cm') - self.assertIsInstance(cm.__aenter__, AsyncMock) - self.assertIsInstance(cm.__aexit__, AsyncMock) - # AsyncMocks are also coroutine functions - self.assertTrue(asyncio.iscoroutinefunction(cm.__aenter__)) - self.assertTrue(asyncio.iscoroutinefunction(cm.__aexit__)) - def test_set_return_value_of_aenter(self): def inner_test(mock_type): pc = self.ProductionCode() diff --git a/Lib/unittest/test/testmock/testmagicmethods.py b/Lib/unittest/test/testmock/testmagicmethods.py index 57f85e951e20..76b3a560de09 100644 --- a/Lib/unittest/test/testmock/testmagicmethods.py +++ b/Lib/unittest/test/testmock/testmagicmethods.py @@ -271,9 +271,6 @@ def test_magic_mock_equality(self): self.assertEqual(mock == mock, True) self.assertEqual(mock != mock, False) - - # This should be fixed with issue38163 - @unittest.expectedFailure def test_asyncmock_defaults(self): mock = AsyncMock() self.assertEqual(int(mock), 1) diff --git a/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst b/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst new file mode 100644 index 000000000000..d7eea367e482 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-25-21-37-02.bpo-38108.Jr9HU6.rst @@ -0,0 +1,2 @@ +Any synchronous magic methods on an AsyncMock now return a MagicMock. Any +asynchronous magic methods on a MagicMock now return an AsyncMock. [View Less]

1 0

bpo-38310: Predict BUILD_MAP_UNPACK_WITH_CALL -> CALL_FUNCTION_EX. (GH-16467)
by Raymond Hettinger Sept. 29, 2019

Sept. 29, 2019

https://github.com/python/cpython/commit/f185a73249189bc3b75b4dc26e9ec2990a… commit: f185a73249189bc3b75b4dc26e9ec2990a587b17 branch: master author: Brandt Bucher <brandtbucher(a)gmail.com> committer: Raymond Hettinger <rhettinger(a)users.noreply.github.com> date: 2019-09-28T17:12:49-07:00 summary: bpo-38310: Predict BUILD_MAP_UNPACK_WITH_CALL -> CALL_FUNCTION_EX. (GH-16467) files: A Misc/NEWS.d/next/Core and Builtins/2019-09-28-22-54-25.bpo-38310.YDTbEo.rst M Python/ceval.c … [View More]

1 0

[3.6] bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16448) (GH-16462)
by Ned Deily Sept. 28, 2019

Sept. 28, 2019

https://github.com/python/cpython/commit/5b18ce60b432d1dfa6f6988be07dd55646… commit: 5b18ce60b432d1dfa6f6988be07dd55646201a9b branch: 3.6 author: Jason R. Coombs <jaraco(a)jaraco.com> committer: Ned Deily <nad(a)python.org> date: 2019-09-28T12:44:12-04:00 summary: [3.6] bpo-38216, bpo-36274: Allow subclasses to separately override validation and encoding behavior (GH-16448) (GH-16462) (cherry picked from commit 7774d7831e8809795c64ce27f7df52674581d298) Co-authored-by: Jason R. … [View More]Coombs <jaraco(a)jaraco.com> files: A Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst M Lib/http/client.py M Lib/test/test_httplib.py diff --git a/Lib/http/client.py b/Lib/http/client.py index 1a6bd8ac42eb..d4821f1a96e0 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -1117,19 +1117,15 @@ def putrequest(self, method, url, skip_host=False, else: raise CannotSendRequest(self.__state) - # Save the method we use, we need it later in the response phase + # Save the method for use later in the response phase self._method = method - if not url: - url = '/' - # Prevent CVE-2019-9740. - match = _contains_disallowed_url_pchar_re.search(url) - if match: - raise InvalidURL(f"URL can't contain control characters. {url!r} " - f"(found at least {match.group()!r})") + + url = url or '/' + self._validate_path(url) + request = '%s %s %s' % (method, url, self._http_vsn_str) - # Non-ASCII characters should have been eliminated earlier - self._output(request.encode('ascii')) + self._output(self._encode_request(request)) if self._http_vsn == 11: # Issue some standard headers for better HTTP/1.1 compliance @@ -1207,6 +1203,18 @@ def putrequest(self, method, url, skip_host=False, # For HTTP/1.0, the server will assume "not chunked" pass + def _encode_request(self, request): + # ASCII also helps prevent CVE-2019-9740. + return request.encode('ascii') + + def _validate_path(self, url): + """Validate a url for putrequest.""" + # Prevent CVE-2019-9740. + match = _contains_disallowed_url_pchar_re.search(url) + if match: + raise InvalidURL(f"URL can't contain control characters. {url!r} " + f"(found at least {match.group()!r})") + def putheader(self, header, *values): """Send a request header line to the server. diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 714d521cf0e5..14d42d483773 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1132,6 +1132,34 @@ def run_server(): thread.join() self.assertEqual(result, b"proxied data\n") + def test_putrequest_override_validation(self): + """ + It should be possible to override the default validation + behavior in putrequest (bpo-38216). + """ + class UnsafeHTTPConnection(client.HTTPConnection): + def _validate_path(self, url): + pass + + conn = UnsafeHTTPConnection('example.com') + conn.sock = FakeSocket('') + conn.putrequest('GET', '/\x00') + + def test_putrequest_override_encoding(self): + """ + It should be possible to override the default encoding + to transmit bytes in another encoding even if invalid + (bpo-36274). + """ + class UnsafeHTTPConnection(client.HTTPConnection): + def _encode_request(self, str_url): + return str_url.encode('utf-8') + + conn = UnsafeHTTPConnection('example.com') + conn.sock = FakeSocket('') + conn.putrequest('GET', '/☃') + + class ExtendedReadTest(TestCase): """ Test peek(), read1(), readline() @@ -1256,6 +1284,7 @@ def test_peek_0(self): p = self.resp.peek(0) self.assertLessEqual(0, len(p)) + class ExtendedReadTestChunked(ExtendedReadTest): """ Test peek(), read1(), readline() in chunked mode diff --git a/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst b/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst new file mode 100644 index 000000000000..ac8e2b042d92 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-27-15-24-45.bpo-38216.-7yvZR.rst @@ -0,0 +1,4 @@ +Allow the rare code that wants to send invalid http requests from the +`http.client` library a way to do so. The fixes for bpo-30458 led to +breakage for some projects that were relying on this ability to test their +own behavior in the face of bad requests. [View Less]

1 0