From 4177e03c2d68de768296f3d335bb44a43542e799 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 7 Feb 2024 09:18:29 +0100 Subject: [PATCH] [FIX] tools: make module import order consistent A long time ago, in a commit far far away: 65a3751613a392ae6a36f6efbcaeade924565d84, we introduced a mechanism to support the "import emulator" feature[^1] of cPython. It's used by some C native modules to trigger an on-the-spot module import during a low-level function call. Some of the modules that do this are `datetime` and `time`: - https://github.com/python/cpython/blob/b6535ea7ecf40c51a9a13089d961c29abeaffa5e/Modules/timemodule.c#L892 - https://github.com/python/cpython/blob/b6535ea7ecf40c51a9a13089d961c29abeaffa5e/Modules/_datetimemodule.c#L1654 Those `PyImport_ImportModuleNoBlock()` calls were initially understood as real calls that needed to return the imported module. But the actual return value is never used by PyImport_Import. As long as the call succeeds, it is sufficient that the modules are imported and present in `sys.modules` afterwards: https://github.com/python/cpython/blob/b6535ea7ecf40c51a9a13089d961c29abeaffa5e/Python/import.c#L1825-L1834 In order to avoid randomizing the order of module imports, which can cause unpredictable behavior depending on the timing of those "emulated imports", it's simpler to provide a "no-op" import function, and ensure that all the allowed modules are imported in advance, when the server starts. This is what this commit does. For good measure, it will raise an error if the module being imported is not already imported, which means it was not one of the expected/allowed modules. In that case, it's sufficient to pre-import the module. [^1]: https://github.com/python/cpython/blob/b6535ea7ecf40c51a9a13089d961c29abeaffa5e/Python/import.c#L1756-L1766 opw-3721469 --- odoo/tools/safe_eval.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/odoo/tools/safe_eval.py b/odoo/tools/safe_eval.py index 333478a91dd..ddec07c2022 100644 --- a/odoo/tools/safe_eval.py +++ b/odoo/tools/safe_eval.py @@ -17,6 +17,7 @@ condition/math builtins. import dis import functools import logging +import sys import types from opcode import HAVE_ARGUMENT, opmap, opname from types import CodeType @@ -37,6 +38,17 @@ __all__ = ['test_expr', 'safe_eval', 'const_eval'] # lp:703841), does import time. _ALLOWED_MODULES = ['_strptime', 'math', 'time'] +# Mock __import__ function, as called by cpython's import emulator `PyImport_Import` inside +# timemodule.c, _datetimemodule.c and others. +# This function does not actually need to do anything, its expected side-effect is to make the +# imported module available in `sys.modules`. The _ALLOWED_MODULES are imported below to make it so. +def _import(name, globals=None, locals=None, fromlist=None, level=-1): + if name not in sys.modules: + raise ImportError(f'module {name} should be imported before calling safe_eval()') + +for module in _ALLOWED_MODULES: + __import__(module) + _UNSAFE_ATTRIBUTES = ['f_builtins', 'f_globals', 'f_locals', 'gi_frame', 'gi_code', 'co_code', 'func_globals'] @@ -264,16 +276,6 @@ def expr_eval(expr): c = test_expr(expr, _EXPR_OPCODES) return unsafe_eval(c) -def _import(name, globals=None, locals=None, fromlist=None, level=-1): - if globals is None: - globals = {} - if locals is None: - locals = {} - if fromlist is None: - fromlist = [] - if name in _ALLOWED_MODULES: - return __import__(name, globals, locals, level) - raise ImportError(name) _BUILTINS = { '__import__': _import, 'True': True,