From bda506968bd9ab91ebef0441b9ff9744007c54b7 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:35:06 -0700 Subject: [PATCH] Handles a bad client sending malformed JSON or non-int primary keys --- src/documents/serialisers.py | 56 ++++++++++++++----- src/documents/tests/test_api_custom_fields.py | 35 ++++++++++++ 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 4c58767ee..a439e198b 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -909,24 +909,48 @@ class _CachingCustomFieldPrimaryKeyField(serializers.PrimaryKeyRelatedField): def _shared_cache(self) -> dict[int, CustomField]: return self.context.setdefault(_CUSTOM_FIELD_CONTEXT_CACHE_KEY, {}) - def prefetch(self, ids: Iterable[int]) -> None: + @staticmethod + def _normalize_pk(data: Any) -> int | None: + """ + Returns `data` coerced to the int a valid CustomField pk would be, + or None if `data` isn't a plausible pk (wrong type, unhashable, + non-numeric, or a bool -- DRF itself rejects bools as pks since + `True == 1` would otherwise silently match). None tells callers to + leave `data` alone and let `super().to_internal_value()` report the + normal validation error instead of touching the cache/queryset with + it directly. + """ + if isinstance(data, bool): + return None + try: + return int(data) + except (TypeError, ValueError): + return None + + def prefetch(self, ids: Iterable[Any]) -> None: shared_cache = self._shared_cache() - missing = {i for i in ids if i not in self._cache and i not in shared_cache} + candidates = {pk for i in ids if (pk := self._normalize_pk(i)) is not None} + missing = { + i for i in candidates if i not in self._cache and i not in shared_cache + } if missing: for obj in self.get_queryset().filter(pk__in=missing): shared_cache[obj.pk] = obj - for i in ids: + for i in candidates: obj = shared_cache.get(i) if obj is not None: self._cache[i] = obj - def to_internal_value(self, data: int) -> CustomField: - if data in self._cache: - return self._cache[data] + def to_internal_value(self, data: Any) -> CustomField: + pk = self._normalize_pk(data) + if pk is None: + return super().to_internal_value(data) + if pk in self._cache: + return self._cache[pk] shared_cache = self._shared_cache() - if data in shared_cache: - obj = shared_cache[data] - self._cache[data] = obj + if pk in shared_cache: + obj = shared_cache[pk] + self._cache[pk] = obj return obj obj: CustomField = super().to_internal_value(data) self._cache[obj.pk] = obj @@ -937,11 +961,15 @@ class _CachingCustomFieldPrimaryKeyField(serializers.PrimaryKeyRelatedField): class CustomFieldInstanceListSerializer(serializers.ListSerializer): def to_internal_value(self, data: Any) -> list[Any]: if isinstance(data, list): - field_ids = { - item["field"] - for item in data - if isinstance(item, dict) and "field" in item - } + field_ids = [] + for item in data: + if not isinstance(item, dict) or "field" not in item: + continue + try: + hash(item["field"]) + except TypeError: + continue + field_ids.append(item["field"]) if field_ids: self.child.fields["field"].prefetch(field_ids) return super().to_internal_value(data) diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 3b81c4593..f5e208582 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -630,6 +630,41 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): f"the already-resolved CustomField, got: {custom_field_lookups}", ) + def test_custom_field_validation_rejects_malformed_field_value(self) -> None: + """ + GIVEN: + - A document is being validated with a malformed custom_fields + entry whose "field" value is neither a valid CustomField id + nor a type DRF's own PrimaryKeyRelatedField can safely reject + on its own (unhashable, or a non-numeric scalar) + WHEN: + - The serializer is validated + THEN: + - A normal validation error is raised, not an unhandled + TypeError/ValueError escaping past DRF's validation layer + """ + doc = DocumentFactory(mime_type="application/pdf") + + bad_field_values = { + "unhashable-list": [], + "unhashable-dict": {}, + "non-numeric-scalar": "abc", + } + for case_id, bad_field_value in bad_field_values.items(): + with self.subTest(case_id): + serializer = DocumentSerializer( + doc, + data={ + "custom_fields": [ + {"field": bad_field_value, "value": "test value"}, + ], + }, + partial=True, + ) + + self.assertFalse(serializer.is_valid()) + self.assertIn("custom_fields", serializer.errors) + def test_change_custom_field_instance_value(self) -> None: """ GIVEN: