From d64e8c601e77b8862d026d4355ea689d9ae2783d Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:38:00 -0700 Subject: [PATCH] Cover boolean/non-numeric/non-list tag id rejection; fix overflow comment that maybe triggered Copilot --- src/documents/serialisers.py | 11 +--- src/documents/tests/test_api_documents.py | 78 +++++++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index e77a5249c..d0e2f487d 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -775,14 +775,9 @@ class _BatchingManyRelatedField(serializers.ManyRelatedField): item_pks = [(item, self._normalize_pk(item)) for item in data] candidate_pks = {pk for _, pk in item_pks if pk is not None} - # Django's IntegerFieldOverflow guard (-> EmptyResultSet, i.e. no - # match) only covers exact/gt/gte/lt/lte lookups, not `in` -- an - # out-of-range int in `pk__in=` reaches the DB driver as-is and - # raises OverflowError (SQLite) / DataError (Postgres) instead of - # cleanly matching nothing. The per-item `exact`-lookup fallback - # below IS covered, so on that failure just skip the batch and let - # every item resolve individually -- each still costs one query, - # but reports the normal validation error instead of a raw 500. + # Django's overflow guard covers exact/gt/gte/lt/lte, not `in` -- an + # out-of-range pk in `pk__in=` can hit the driver raw (OverflowError + # on SQLite). Fall back to per-item resolution, which is guarded. try: resolved_by_pk = { obj.pk: obj diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index 2664f6314..ceaf54427 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -331,6 +331,84 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_document_update_tags_rejects_boolean_id(self) -> None: + """ + GIVEN: + - A document is being updated with a boolean submitted as a tag id + - A tag exists with id 1 + WHEN: + - API PATCH request is made setting the document's tags to [true] + THEN: + - A normal 400 validation error is returned + - The document is not silently given tag id 1 (`True == 1`) + """ + # Explicit pk, not TagFactory()'s auto-assigned id: Postgres sequences + # aren't rolled back between tests in the same TestCase (only the + # rows are), so relying on this being the first tag created would be + # order-dependent. + Tag.objects.create(name="t", pk=1) + doc = Document.objects.create( + title="none", + checksum="123", + mime_type="application/pdf", + ) + + response = self.client.patch( + f"/api/documents/{doc.pk}/", + {"tags": [True]}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + doc.refresh_from_db() + self.assertEqual(doc.tags.count(), 0) + + def test_document_update_tags_rejects_non_numeric_id(self) -> None: + """ + GIVEN: + - A document is being updated with a non-numeric tag id + WHEN: + - API PATCH request is made setting the document's tags + THEN: + - A normal 400 validation error is returned + """ + doc = Document.objects.create( + title="none", + checksum="123", + mime_type="application/pdf", + ) + + response = self.client.patch( + f"/api/documents/{doc.pk}/", + {"tags": ["not-a-number"]}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_document_update_tags_rejects_non_list(self) -> None: + """ + GIVEN: + - A document is being updated + WHEN: + - API PATCH request is made with a non-list value for tags + THEN: + - A normal 400 validation error is returned + """ + doc = Document.objects.create( + title="none", + checksum="123", + mime_type="application/pdf", + ) + + response = self.client.patch( + f"/api/documents/{doc.pk}/", + {"tags": "not-a-list"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_document_update_legacy_created_format(self) -> None: """ GIVEN: