diff --git a/Procfile b/Procfile index 7e116423..008b8343 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,2 @@ -release: python manage.py migrate +release: python manage.py migrate && python manage.py createcachetable web: gunicorn codethesaurus.wsgi --log-file - \ No newline at end of file diff --git a/codethesaurus/settings.py b/codethesaurus/settings.py index 111aa035..2b5313d8 100644 --- a/codethesaurus/settings.py +++ b/codethesaurus/settings.py @@ -56,7 +56,6 @@ 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', - 'django.middleware.common.BrokenLinkEmailsMiddleware' ] ROOT_URLCONF = 'codethesaurus.urls' @@ -238,3 +237,25 @@ "BACKEND": "whitenoise.storage.CompressedManifestStaticFilesStorage", }, } + +if os.getenv('DJANGO_DEVELOPMENT') == 'true': + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', + } + } +elif SYSTEM_ENV in ('PRODUCTION', 'STAGING'): + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.db.DatabaseCache', + 'LOCATION': 'cache_table', + 'TIMEOUT': 3600, + } + } +else: + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + } + } + diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 06238ce1..7eeaa4cc 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -8,6 +8,8 @@ python manage.py migrate --noinput # Collect static files if needed (usually for production, but good to have) if [ "$SYSTEM_ENV" = "PRODUCTION" ] || [ "$SYSTEM_ENV" = "STAGING" ]; then + echo "Creating cache table..." + python manage.py createcachetable echo "Collecting static files..." python manage.py collectstatic --noinput --clear fi diff --git a/web/management/commands/generate_missing_templates.py b/web/management/commands/generate_missing_templates.py index 7830b4e4..c385f2d6 100644 --- a/web/management/commands/generate_missing_templates.py +++ b/web/management/commands/generate_missing_templates.py @@ -13,18 +13,22 @@ def handle(self, *args, **options): meta_info = ThesaurusMetaInfo() languages = meta_info.languages structures = meta_info.structures - - for language in languages: - versions = ThesaurusEntry(language, languages[language]).versions() - for version in versions: + + for language, language_name in languages.items(): + entry = ThesaurusEntry(language, language_name) + if entry.language_dir is None: + continue + + lang_root = os.path.realpath(entry.language_dir) + for version in entry.versions(): for structure in structures: - file_path = os.path.join( - 'web', - 'thesauruses', - language, + file_path = os.path.realpath(os.path.join( + entry.language_dir, version, structure + '.json' - ) + )) + if not file_path.startswith(lang_root + os.sep): + continue if not os.path.exists(file_path): call_command( 'generate_template', diff --git a/web/management/commands/validatemetainfofile.py b/web/management/commands/validatemetainfofile.py index 9a9b81fb..264efc23 100644 --- a/web/management/commands/validatemetainfofile.py +++ b/web/management/commands/validatemetainfofile.py @@ -4,6 +4,17 @@ from web.models import ThesaurusMetaInfo +import os + + +def _safe_joined_path(root, *parts): + """Join parts under root and return the realpath only if it stays inside root, else None.""" + root_real = os.path.realpath(str(root)) + candidate = os.path.realpath(os.path.join(str(root), *parts)) + if candidate == root_real or candidate.startswith(root_real + os.sep): + return candidate + return None + class Command(BaseCommand): help = "Validate the structure of the meta info file" @@ -43,8 +54,11 @@ def check_category_directories(self): # Check if all categories in meta_info.json have directories for category_key in self.metainfo.categories: - path = self.thesauruses_path / category_key - if not path.is_dir(): + path = _safe_joined_path(self.thesauruses_path, category_key) + if path is None: + self.report_error(f"Category `{category_key}` in `meta_info.json` escapes the thesauruses directory") + continue + if not os.path.isdir(path): self.report_error(f"Category `{category_key}` is listed in `meta_info.json` but directory `{path}` was not found") def check_thesaurus_directories(self): @@ -79,8 +93,8 @@ def check_meta_info_consistency(self): for category_dir in self.thesauruses_path.iterdir(): if not category_dir.is_dir() or category_dir.name == "_meta": continue - path = category_dir / meta_lang - if path.is_dir(): + path = _safe_joined_path(category_dir, meta_lang) + if path is not None and os.path.isdir(path): found = True break if not found: @@ -101,6 +115,9 @@ def check_meta_files_consistency(self): # Check structures listed in ThesaurusMetaInfo have corresponding files in _meta for structure in self.metainfo.structures: - path = self.meta_path / f"{structure}.json" - if not path.is_file(): + path = _safe_joined_path(self.meta_path, f"{structure}.json") + if path is None: + self.report_error(f"{structure} is listed as a structure in `meta_info.json` but its path escapes the _meta directory") + continue + if not os.path.isfile(path): self.report_error(f"{structure} is listed as a structure in `meta_info.json` but the `{path}` file doesn't exist") diff --git a/web/models.py b/web/models.py index 9b094472..29a24648 100644 --- a/web/models.py +++ b/web/models.py @@ -1,9 +1,14 @@ """models of codethesaur.us""" import json +import logging import os -from jsonmerge import merge +import re +from django.core.cache import cache from django.db import models +from jsonmerge import merge + +_CACHE_KEY_SAFE = re.compile(r"[^0-9A-Za-z_.-]") def _is_safe_path_component(value): @@ -137,11 +142,40 @@ def load_concepts(self, structure_key, version): if not file_path.startswith(root_real + os.sep): raise FileNotFoundError( f"Structure/version escape the thesaurus dir: {structure_key!r} / {version!r}") - with open(file_path, 'r', encoding='UTF-8') as file: - file_json = json.load(file) - self.concepts = file_json["concepts"] + self.concepts = self._load_cached_concepts(structure_key, version, file_path) self.version = version + def _load_cached_concepts(self, structure_key, version, file_path): + """ + Returns the parsed concepts for this entry, reading them from the + Django cache when possible so the structure file is only parsed once + + :param structure_key: the key of the structure to load + :param version: the version of the entry + :param file_path: the resolved path to the structure file + :return: dict of concept ID -> concept data + """ + cache_key = f"thesaurus:{self.key}:{version}:{structure_key}" + cache_key = _CACHE_KEY_SAFE.sub("_", cache_key) + # The cache is optional infrastructure -- fall back to parsing the + # file directly if the backend is unavailable (e.g. database is down). + # pylint: disable=broad-exception-caught + try: + concepts = cache.get(cache_key) + if concepts is not None: + return concepts + except Exception as exception: + logging.warning( + "Failed to read thesaurus cache: %s", exception) + with open(file_path, 'r', encoding='UTF-8') as file: + concepts = json.load(file)["concepts"] + try: + cache.set(cache_key, concepts) + except Exception as exception: + logging.warning( + "Failed to write thesaurus cache: %s", exception) + return concepts + def load_filled_concepts(self, structure_key, version): from web.thesaurus_template_generators import generate_entry_template """ diff --git a/web/tests/test_commands.py b/web/tests/test_commands.py index 22419f84..48b66fe8 100644 --- a/web/tests/test_commands.py +++ b/web/tests/test_commands.py @@ -66,6 +66,8 @@ def test_generate_missing_templates_command(self): meta_mock.structures = {test_structure: 'Data Types'} entry_mock = Mock() + entry_mock.language_dir = os.path.join( + 'web', 'thesauruses', 'test_lang') entry_mock.versions.return_value = [test_version] try: diff --git a/web/tests/test_models.py b/web/tests/test_models.py index 95df5772..05957ded 100644 --- a/web/tests/test_models.py +++ b/web/tests/test_models.py @@ -2,9 +2,35 @@ import json import os -from django.test import TestCase +from django.core.cache import cache +from django.test import TestCase, override_settings -from web.models import ThesaurusEntry, ThesaurusMetaInfo, MetaStructure +from web.models import MetaStructure, ThesaurusEntry, ThesaurusMetaInfo + + +@override_settings( + CACHES={'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache'}} +) +class TestCachedStructureLoading(TestCase): + """Tests that loading a structure from disk goes through the Django cache""" + + def setUp(self): + cache.clear() + + def test_load_concepts_reads_structure_file_only_once(self): + """test that loading the same structure/version reads the file only once""" + from unittest.mock import mock_open, patch + + payload = json.dumps({"concepts": {"boolean": {"code": "True"}}}) + with patch("builtins.open", mock_open(read_data=payload)) as mocked_open: + first_entry = ThesaurusEntry("python", "Python") + first_entry.load_concepts("data_types", "3") + second_entry = ThesaurusEntry("python", "Python") + second_entry.load_concepts("data_types", "3") + + self.assertEqual(mocked_open.call_count, 1) + self.assertEqual(first_entry.concepts, {"boolean": {"code": "True"}}) + self.assertEqual(second_entry.concepts, {"boolean": {"code": "True"}}) class TestMetaStructures(TestCase): diff --git a/web/views.py b/web/views.py index 222ff1d7..9597e4fa 100644 --- a/web/views.py +++ b/web/views.py @@ -2,6 +2,7 @@ import logging import os import random +import re from django.conf import settings from django.http import ( @@ -33,6 +34,30 @@ from web.thesaurus_template_generators import generate_entry_template +def _sanitize_query_param(value): + if value is None: + return None + if not isinstance(value, str): + value = str(value) + # Remove control characters and limit length + value = re.sub(r"[\x00-\x1f\x7f]", " ", value) + value = value[:200] + return value.strip() + + +def _log_sanitized(message, *args): + sanitized_args = [] + for arg in args: + if isinstance(arg, str): + sanitized_args.append(_sanitize_query_param(arg) or "") + else: + sanitized_args.append(arg) + try: + logging.warning(message, *sanitized_args) + except Exception: + logging.warning("Log message sanitized due to encoding issues") + + def store_url_info(request): try: if 'HTTP_USER_AGENT' in request.META: @@ -54,7 +79,7 @@ def store_url_info(request): visit.save() return visit except Exception as e: - logging.error(f"Failed to store URL info: {e}") + logging.error("Failed to store URL info: %s", _sanitize_query_param(str(e))) return None @@ -73,7 +98,7 @@ def store_lookup_info(request, visit, entry1, version1, entry2, version2, struct with transaction.atomic(): info.save() except Exception as e: - logging.error(f"Failed to store lookup info: {e}") + logging.error("Failed to store lookup info: %s", _sanitize_query_param(str(e))) def store_missing_info(visit, item_type, item_value, language_context=None): @@ -89,7 +114,7 @@ def store_missing_info(visit, item_type, item_value, language_context=None): with transaction.atomic(): info.save() except Exception as e: - logging.error(f"Failed to store missing info: {e}") + logging.error("Failed to store missing info: %s", _sanitize_query_param(str(e))) @require_http_methods(['GET']) @@ -695,23 +720,23 @@ def api_reference(request, structure_key, lang, version): # Determine if it's a language or structure issue # If ThesaurusEntry(lang, "") failed to find versions, it might be a language issue if not entry_obj.versions(): - store_missing_info(visit, 'language', lang) + store_missing_info(visit, 'language', _sanitize_query_param(lang)) else: - store_missing_info(visit, 'structure', structure_key, lang) + store_missing_info(visit, 'structure', _sanitize_query_param(structure_key), _sanitize_query_param(lang)) return error_handler_404_not_found(request, e) if response is False: - store_missing_info(visit, 'structure', structure_key, lang) + store_missing_info(visit, 'structure', _sanitize_query_param(structure_key), _sanitize_query_param(lang)) return HttpResponseNotFound() store_lookup_info( request, visit, - lang, - version, + _sanitize_query_param(lang) or lang, + _sanitize_query_param(version) or version, "", "", - structure_key + _sanitize_query_param(structure_key) or structure_key ) return HttpResponse(response, content_type="application/json") @@ -731,24 +756,39 @@ def api_compare(request, structure_key, lang1, version1, lang2, version2): visit = store_url_info(request) try: - response = ThesaurusEntry(lang1, "").load_comparison(structure_key, lang2, version2, version1) + response = ThesaurusEntry(_sanitize_query_param(lang1) or lang1, "").load_comparison( + _sanitize_query_param(structure_key) or structure_key, + _sanitize_query_param(lang2) or lang2, + _sanitize_query_param(version2) or version2, + _sanitize_query_param(version1) or version1 + ) except Exception: # Simple logging for now - store_missing_info(visit, 'structure', structure_key, f"{lang1}/{lang2}") + store_missing_info( + visit, + 'structure', + _sanitize_query_param(structure_key) or structure_key, + _sanitize_query_param(f"{lang1}/{lang2}") or f"{lang1}/{lang2}" + ) return HttpResponseNotFound() if response is False: - store_missing_info(visit, 'structure', structure_key, f"{lang1}/{lang2}") + store_missing_info( + visit, + 'structure', + _sanitize_query_param(structure_key) or structure_key, + _sanitize_query_param(f"{lang1}/{lang2}") or f"{lang1}/{lang2}" + ) return HttpResponseNotFound() store_lookup_info( request, visit, - lang1, - version1, - lang2, - version2, - structure_key + _sanitize_query_param(lang1) or lang1, + _sanitize_query_param(version1) or version1, + _sanitize_query_param(lang2) or lang2, + _sanitize_query_param(version2) or version2, + _sanitize_query_param(structure_key) or structure_key ) return HttpResponse(response, content_type="application/json")