aboutsummaryrefslogtreecommitdiffstats
path: root/fatcat_scholar
diff options
context:
space:
mode:
authorBryan Newbold <bnewbold@archive.org>2022-08-12 12:20:25 -0700
committerBryan Newbold <bnewbold@archive.org>2022-08-12 12:22:20 -0700
commit9cb5c59114b174a7eb8ec1065ff3321c0b1b86a4 (patch)
treeb533928514866bbd97fd8dc3f35eed9ee1ce8741 /fatcat_scholar
parentcc8a89d9d4a529af3eb87d50ed2ff36051e674f3 (diff)
downloadfatcat-scholar-9cb5c59114b174a7eb8ec1065ff3321c0b1b86a4.tar.gz
fatcat-scholar-9cb5c59114b174a7eb8ec1065ff3321c0b1b86a4.zip
web: refactor i18n template loading
This is an attempt to fix a bug with random HTML template escapes in production. I believe these are being caused by cross-request contamination of template state due to using globals to hold on to per-language jinja2 templates. I originally thought this might be a bug in the jinja2 i18n extension itself, and there may still be concurrency concerns there, but it seems like the proximal cause is the use of globals. This change probably has a negative performance impact, because the jinja2 environment is re-created on every request (though babel files are not reloaded on every request).
Diffstat (limited to 'fatcat_scholar')
-rw-r--r--fatcat_scholar/web.py81
-rw-r--r--fatcat_scholar/web_hacks.py84
2 files changed, 93 insertions, 72 deletions
diff --git a/fatcat_scholar/web.py b/fatcat_scholar/web.py
index 73088d6..1a46bc3 100644
--- a/fatcat_scholar/web.py
+++ b/fatcat_scholar/web.py
@@ -9,8 +9,6 @@ import logging
import urllib.parse
from typing import Any, Dict, List, Optional
-import babel.numbers
-import babel.support
import fastapi_rss
import fatcat_openapi_client
import sentry_sdk
@@ -28,12 +26,7 @@ from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
from starlette.exceptions import HTTPException as StarletteHTTPException
from starlette_prometheus import PrometheusMiddleware, metrics
-from fatcat_scholar.config import GIT_REVISION, settings
-from fatcat_scholar.web_hacks import (
- Jinja2Templates,
- make_access_redirect_url,
- parse_accept_lang,
-)
+from fatcat_scholar.config import GIT_REVISION, I18N_LANG_OPTIONS, settings
from fatcat_scholar.schema import ScholarDoc
from fatcat_scholar.search import (
FulltextHits,
@@ -42,6 +35,7 @@ from fatcat_scholar.search import (
get_es_scholar_doc,
process_query,
)
+from fatcat_scholar.web_hacks import i18n_templates, parse_accept_lang
logger = logging.getLogger()
@@ -170,63 +164,6 @@ def get_work(work_ident: str = Query(..., min_length=20, max_length=20)) -> dict
web = APIRouter()
-def locale_gettext(translations: Any) -> Any:
- def gt(s): # noqa: ANN001,ANN201
- return translations.ugettext(s)
-
- return gt
-
-
-def locale_ngettext(translations: Any) -> Any:
- def ngt(s, p, n): # noqa: ANN001,ANN201
- return translations.ungettext(s, p, n)
-
- return ngt
-
-
-def load_i18n_templates() -> Any:
- """
- This is a hack to work around lack of per-request translation
- (babel/gettext) locale switching in FastAPI and Starlette. Flask (and
- presumably others) get around this using global context (eg, in
- Flask-Babel).
-
- See related issues:
-
- - https://github.com/encode/starlette/issues/279
- - https://github.com/aio-libs/aiohttp-jinja2/issues/187
- """
-
- d = dict()
- for lang_opt in I18N_LANG_OPTIONS:
- translations = babel.support.Translations.load(
- dirname="fatcat_scholar/translations",
- locales=[lang_opt],
- )
- templates = Jinja2Templates(
- directory="fatcat_scholar/templates",
- extensions=["jinja2.ext.i18n", "jinja2.ext.do"],
- )
- templates.env.install_gettext_translations(translations, newstyle=True) # type: ignore
- templates.env.install_gettext_callables( # type: ignore
- locale_gettext(translations),
- locale_ngettext(translations),
- newstyle=True,
- )
- # remove a lot of whitespace in HTML output with these configs
- templates.env.trim_blocks = True
- templates.env.lstrip_blocks = True
- # pass-through application settings to be available in templates
- templates.env.globals["settings"] = settings
- templates.env.globals["babel_numbers"] = babel.numbers
- templates.env.globals["make_access_redirect_url"] = make_access_redirect_url
- d[lang_opt] = templates
- return d
-
-
-I18N_TEMPLATES = load_i18n_templates()
-
-
@web.get("/", include_in_schema=False)
async def web_home(
request: Request,
@@ -235,7 +172,7 @@ async def web_home(
) -> Any:
if content.mimetype == "application/json":
return await home()
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"home.html",
{"request": request, "locale": lang.code, "lang_prefix": lang.prefix},
)
@@ -243,7 +180,7 @@ async def web_home(
@web.get("/about", include_in_schema=False)
async def web_about(request: Request, lang: LangPrefix = Depends(LangPrefix)) -> Any:
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"about.html",
{"request": request, "locale": lang.code, "lang_prefix": lang.prefix},
)
@@ -251,7 +188,7 @@ async def web_about(request: Request, lang: LangPrefix = Depends(LangPrefix)) ->
@web.get("/help", include_in_schema=False)
async def web_help(request: Request, lang: LangPrefix = Depends(LangPrefix)) -> Any:
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"help.html",
{"request": request, "locale": lang.code, "lang_prefix": lang.prefix},
)
@@ -293,7 +230,7 @@ def web_search(
headers[
"Server-Timing"
] += f', es;desc="Search Internal Time";dur={hits.query_time_ms}'
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"search.html",
{
"request": request,
@@ -385,7 +322,7 @@ def web_work(
if not doc:
raise HTTPException(status_code=404, detail="work not found")
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"work.html",
{
"request": request,
@@ -470,7 +407,7 @@ def access_redirect_fallback(
# give up and show an error page
lang = LangPrefix(request)
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"access_404.html",
{
"request": request,
@@ -620,7 +557,7 @@ async def http_exception_handler(request: Request, exc: StarletteHTTPException)
if content.mimetype == "text/html":
lang = LangPrefix(request)
- return I18N_TEMPLATES[lang.code].TemplateResponse(
+ return i18n_templates(lang.code).TemplateResponse(
"error.html",
{
"request": request,
diff --git a/fatcat_scholar/web_hacks.py b/fatcat_scholar/web_hacks.py
index 2be90f0..aa33cbb 100644
--- a/fatcat_scholar/web_hacks.py
+++ b/fatcat_scholar/web_hacks.py
@@ -1,9 +1,13 @@
import typing
+import babel.numbers
+import babel.support
import jinja2
from starlette.background import BackgroundTask
from starlette.templating import _TemplateResponse
+from fatcat_scholar.config import I18N_LANG_OPTIONS, settings
+
class Jinja2Templates:
"""
@@ -53,6 +57,86 @@ class Jinja2Templates:
)
+def load_i18n_files() -> typing.Any:
+ """
+ This is a hack to work around lack of per-request translation
+ (babel/gettext) locale switching in FastAPI and Starlette. Flask (and
+ presumably others) get around this using global context (eg, in
+ Flask-Babel).
+
+ See related issues:
+
+ - https://github.com/encode/starlette/issues/279
+ - https://github.com/aio-libs/aiohttp-jinja2/issues/187
+ """
+
+ d = dict()
+ for lang_opt in I18N_LANG_OPTIONS:
+ translations = babel.support.Translations.load(
+ dirname="fatcat_scholar/translations",
+ locales=[lang_opt],
+ )
+ d[lang_opt] = translations
+ return d
+
+
+I18N_TRANSLATION_FILES = load_i18n_files()
+
+
+def locale_gettext(translations: typing.Any) -> typing.Any:
+ def gt(s): # noqa: ANN001,ANN201
+ return translations.ugettext(s)
+
+ return gt
+
+
+def locale_ngettext(translations: typing.Any) -> typing.Any:
+ def ngt(s, p, n): # noqa: ANN001,ANN201
+ return translations.ungettext(s, p, n)
+
+ return ngt
+
+
+def i18n_templates(locale: str) -> Jinja2Templates:
+ """
+ This is a hack to work around lack of per-request translation
+ (babel/gettext) locale switching in FastAPI and Starlette. Flask (and
+ presumably others) get around this using global context (eg, in
+ Flask-Babel).
+
+ The intent is to call this function and create a new Jinja2 Environment for
+ a specific language separately within a request (aka, not shared between
+ requests), when needed. This is inefficient but should resolve issues with
+ cross-request poisoning, both in threading (threadpool) or async
+ concurrency.
+
+ See related issues:
+
+ - https://github.com/encode/starlette/issues/279
+ - https://github.com/aio-libs/aiohttp-jinja2/issues/187
+ """
+
+ translations = I18N_TRANSLATION_FILES[locale]
+ templates = Jinja2Templates(
+ directory="fatcat_scholar/templates",
+ extensions=["jinja2.ext.i18n", "jinja2.ext.do"],
+ )
+ templates.env.install_gettext_translations(translations, newstyle=True) # type: ignore
+ templates.env.install_gettext_callables( # type: ignore
+ locale_gettext(translations),
+ locale_ngettext(translations),
+ newstyle=True,
+ )
+ # remove a lot of whitespace in HTML output with these configs
+ templates.env.trim_blocks = True
+ templates.env.lstrip_blocks = True
+ # pass-through application settings to be available in templates
+ templates.env.globals["settings"] = settings
+ templates.env.globals["babel_numbers"] = babel.numbers
+ templates.env.globals["make_access_redirect_url"] = make_access_redirect_url
+ return templates
+
+
def parse_accept_lang(header: str, options: typing.List[str]) -> typing.Optional[str]:
"""
Crude HTTP Accept-Language content negotiation.