diff --git a/frappe/core/doctype/navbar_settings/navbar_settings.py b/frappe/core/doctype/navbar_settings/navbar_settings.py index 08fda8a8b6..dc3263a27b 100644 --- a/frappe/core/doctype/navbar_settings/navbar_settings.py +++ b/frappe/core/doctype/navbar_settings/navbar_settings.py @@ -24,8 +24,10 @@ class NavbarSettings(Document): def get_app_logo(): - app_logo = frappe.get_website_settings("app_logo") or frappe.db.get_single_value( - "Navbar Settings", "app_logo", cache=True + app_logo = frappe.get_website_settings("app_logo") or frappe.get_cached_value( + "Navbar Settings", + "Navbar Settings", + "app_logo", ) if not app_logo: 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/tests/test_utils.py b/frappe/tests/test_utils.py index 36f622932b..105f0f3eed 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -68,6 +68,7 @@ from frappe.utils.data import ( get_url_to_form, get_year_ending, getdate, + is_invalid_date_string, now_datetime, nowtime, pretty_date, @@ -669,11 +670,15 @@ class TestDateUtils(IntegrationTestCase): @given(st.datetimes()) def test_get_datetime(self, original): + if is_invalid_date_string(str(original)): + return parsed = get_datetime(str(original)) self.assertEqual(parsed, original) @given(st.datetimes(timezones=st.timezones())) def test_get_datetime_tz_aware(self, original): + if is_invalid_date_string(str(original)): + return parsed = get_datetime(str(original)) self.assertEqual(parsed, original) diff --git a/frappe/utils/jinja.py b/frappe/utils/jinja.py index 4e5a2b6a1a..7216479b9c 100644 --- a/frappe/utils/jinja.py +++ b/frappe/utils/jinja.py @@ -6,35 +6,57 @@ from frappe.utils.caching import site_cache def get_jenv(): import frappe + from frappe.utils.safe_exec import get_safe_globals - if not getattr(frappe.local, "jenv", None): - from jinja2 import DebugUndefined - from jinja2.sandbox import SandboxedEnvironment + if jenv := getattr(frappe.local, "jenv", None): + return jenv - from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES, get_safe_globals + 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 - UNSAFE_ATTRIBUTES = UNSAFE_ATTRIBUTES - {"format", "format_map"} + # Note: Overlay by default is "linked", we need to copy everything we are updating. + jenv.globals = default_jenv.globals.copy() + jenv.filters = default_jenv.filters.copy() - class FrappeSandboxedEnvironment(SandboxedEnvironment): - def is_safe_attribute(self, obj, attr, *args, **kwargs): - if attr in UNSAFE_ATTRIBUTES: - return False + jenv.globals.update(get_safe_globals()) + methods, filters = get_jinja_hooks() + jenv.globals.update(methods or {}) + jenv.filters.update(filters or {}) - return super().is_safe_attribute(obj, attr, *args, **kwargs) + frappe.local.jenv = jenv - # frappe will be loaded last, so app templates will get precedence - jenv = FrappeSandboxedEnvironment(loader=get_jloader(), undefined=DebugUndefined) - set_filters(jenv) + return jenv - jenv.globals.update(get_safe_globals()) - methods, filters = get_jinja_hooks() - jenv.globals.update(methods or {}) - jenv.filters.update(filters or {}) +@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. - frappe.local.jenv = jenv + from jinja2 import DebugUndefined + from jinja2.sandbox import SandboxedEnvironment - return frappe.local.jenv + from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES + + UNSAFE_ATTRIBUTES = UNSAFE_ATTRIBUTES - {"format", "format_map"} + + class FrappeSandboxedEnvironment(SandboxedEnvironment): + def is_safe_attribute(self, obj, attr, *args, **kwargs): + if attr in UNSAFE_ATTRIBUTES: + return False + + 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, cache_size=32) + set_filters(jenv) + + return jenv def get_template(path): @@ -95,7 +117,7 @@ def render_template(template, context=None, is_path=None, safe_render=True): try: if is_path or guess_is_path(template): is_path = True - compiled_template = compile_template(template) + compiled_template = get_template(template) else: jenv: SandboxedEnvironment = get_jenv() if safe_render and ".__" in template: @@ -146,12 +168,6 @@ def get_jloader(): return jloader -@site_cache(ttl=10 * 60, maxsize=16) -def compile_template(path): - jenv = get_jenv() - return jenv.get_template(path) - - @site_cache(ttl=10 * 60, maxsize=8) def _get_jloader(): from jinja2 import ChoiceLoader, PackageLoader, PrefixLoader