Skip to content

Commit

Permalink
[FIX] tools: make module import order consistent
Browse files Browse the repository at this point in the history
A long time ago, in a commit far far away: 65a3751,
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
  • Loading branch information
odony committed Feb 7, 2024
1 parent 262ef7e commit 3b588af
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions odoo/tools/safe_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import dis
import functools
import logging
import sys
import types
from opcode import HAVE_ARGUMENT, opmap, opname
from types import CodeType
Expand All @@ -37,6 +38,17 @@
# 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']

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3b588af

Please sign in to comment.