diff --git a/frappe/tests/test_safe_exec.py b/frappe/tests/test_safe_exec.py index eb65e6beea..94b1c7422c 100644 --- a/frappe/tests/test_safe_exec.py +++ b/frappe/tests/test_safe_exec.py @@ -2,6 +2,7 @@ import types import frappe from frappe.tests import IntegrationTestCase +from frappe.utils.jinja import get_jenv from frappe.utils.safe_exec import ServerScriptNotEnabled, get_safe_globals, safe_exec @@ -127,3 +128,17 @@ class TestSafeExec(IntegrationTestCase): class TestNoSafeExec(IntegrationTestCase): def test_safe_exec_disabled_by_default(self): self.assertRaises(ServerScriptNotEnabled, safe_exec, "pass") + + +class TestJinjaGlobals(IntegrationTestCase): + def test_jenv_thread_safety(self): + first = get_jenv() + # reinit to create a new local ctx, this "simulates" two request running in two diff + # thread. + frappe.init(frappe.local.site, force=True) + second = get_jenv() + self.assertIsNot(first, second) + self.assertIsNot(first.globals, second.globals) + self.assertIsNot(first.filters, second.filters) + self.assertIsNot(first.globals["frappe"], second.globals["frappe"]) + self.assertIsNot(first.globals["frappe"]["form_dict"], second.globals["frappe"]["form_dict"]) diff --git a/frappe/utils/jinja.py b/frappe/utils/jinja.py index 63f48a9b9f..af4153e42e 100644 --- a/frappe/utils/jinja.py +++ b/frappe/utils/jinja.py @@ -8,18 +8,39 @@ def get_jenv(): import frappe from frappe.utils.safe_exec import get_safe_globals - jenv = _get_jenv() - jenv.globals.update(get_safe_globals()) # TODO: This isn't thread safe. + if jenv := getattr(frappe.local, "jenv", None): + return jenv + + default_jenv = _get_jenv() + jenv = default_jenv.overlay() + # XXX: This is safe to share between requests, the only reason why we are overlaying jenv is to + # reuse cache but still have request specific jenv object. + if not frappe._dev_server: + jenv.cache = default_jenv.cache + + jenv.globals = default_jenv.globals.copy() + jenv.filters = default_jenv.filters.copy() + + jenv.globals.update(get_safe_globals()) + methods, filters = get_jinja_hooks() + jenv.globals.update(methods or {}) + jenv.filters.update(filters or {}) + frappe.local.jenv = jenv + return jenv @site_cache(ttl=10 * 60, maxsize=4) def _get_jenv(): + # XXX: DO NOT use any thread/request specific data in this function! + # Some functionality like `get_safe_globals` appears safe but internally uses request local + # data. + from jinja2 import DebugUndefined from jinja2.sandbox import SandboxedEnvironment - from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES, get_safe_globals + from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES UNSAFE_ATTRIBUTES = UNSAFE_ATTRIBUTES - {"format", "format_map"} @@ -31,13 +52,9 @@ def _get_jenv(): return super().is_safe_attribute(obj, attr, *args, **kwargs) # frappe will be loaded last, so app templates will get precedence - jenv = FrappeSandboxedEnvironment(loader=get_jloader(), undefined=DebugUndefined) + jenv = FrappeSandboxedEnvironment(loader=get_jloader(), undefined=DebugUndefined, cache_size=32) set_filters(jenv) - methods, filters = get_jinja_hooks() - jenv.globals.update(methods or {}) - jenv.filters.update(filters or {}) - return jenv