From 885bc2fdf3aa525b993713f2a7e24edf08acc8c2 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:30:30 -0700 Subject: [PATCH] fix(search): cap query length at the shared search-param helper (F3) whoosh-compat's fieldname tagger is O(n^2) in plain word characters, reachable only through SearchMode.QUERY's whoosh grammar. Measured against the real field registry: ~1s at 10k chars, ~3.7s at 20k, ~14.4s at 40k. The POST selection-filter path (bulk edit, bulk download) has no server-imposed length bound the way the GET path incidentally does via header limits, making an unbounded query a single-request CPU exhaustion vector. Cap both entry points at their shared choke point, _get_tantivy_query_and_mode, with a new QueryTooLongError that reuses the existing SearchQueryError -> 400 routing both callers already have. 4096 chars bounds the worst case to roughly 0.16s by quadratic extrapolation, far beyond any plausible hand-typed query. TEXT and TITLE modes route through simple_search_tokens instead and measure linear even at 20k chars, so the same cap is hygiene for them rather than a fix. Hardcoded rather than a PAPERLESS_* setting: this is a security boundary, and a raisable ceiling could reintroduce the exact hazard it exists to close. Co-Authored-By: Claude Opus 5 --- src/documents/search/__init__.py | 2 + src/documents/search/_errors.py | 17 ++ .../tests/test_api_search_query_length.py | 151 ++++++++++++++++++ src/documents/views.py | 36 ++++- 4 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 src/documents/tests/test_api_search_query_length.py diff --git a/src/documents/search/__init__.py b/src/documents/search/__init__.py index 22247ec55..0e89caa6f 100644 --- a/src/documents/search/__init__.py +++ b/src/documents/search/__init__.py @@ -9,6 +9,7 @@ from documents.search._backend import reset_backend from documents.search._errors import InvalidDateQuery from documents.search._errors import InvalidNumberQuery from documents.search._errors import MultipleSearchQueryErrors +from documents.search._errors import QueryTooLongError from documents.search._errors import SearchQueryError from documents.search._errors import search_query_error_messages from documents.search._schema import needs_rebuild @@ -18,6 +19,7 @@ __all__ = [ "InvalidDateQuery", "InvalidNumberQuery", "MultipleSearchQueryErrors", + "QueryTooLongError", "SearchHit", "SearchIndexLockError", "SearchMode", diff --git a/src/documents/search/_errors.py b/src/documents/search/_errors.py index ba04bf287..a7bf4be04 100644 --- a/src/documents/search/_errors.py +++ b/src/documents/search/_errors.py @@ -34,6 +34,23 @@ class InvalidNumberQuery(SearchQueryError): super().__init__(f"Invalid numeric value {value!r} for field {field!r}.") +class QueryTooLongError(SearchQueryError): + """Raised when a query string exceeds the maximum allowed length. + + whoosh-compat's fieldname tagger is O(n^2) in plain word characters, so an + unbounded query is a CPU-exhaustion vector against a single request + handler. This is a hard boundary, not a validation nicety. + """ + + def __init__(self, length: int, limit: int) -> None: + self.length = length + self.limit = limit + super().__init__( + f"The search query is too long ({length} characters). " + f"The maximum allowed length is {limit} characters.", + ) + + class MultipleSearchQueryErrors(SearchQueryError): """Aggregates every user-fixable error from one parse, not just the first.""" diff --git a/src/documents/tests/test_api_search_query_length.py b/src/documents/tests/test_api_search_query_length.py new file mode 100644 index 000000000..9c2c39647 --- /dev/null +++ b/src/documents/tests/test_api_search_query_length.py @@ -0,0 +1,151 @@ +"""The query-length cap in ``_get_tantivy_query_and_mode`` (F3). + +whoosh-compat's fieldname tagger is O(n^2) in plain word characters, so an +unbounded ``query`` (SearchMode.QUERY) string is a CPU-exhaustion vector +against a single request handler. The GET search endpoint is incidentally +bounded by the web server's header limit, but the POST selection-filter +path (bulk edit, bulk download) is not -- that is the real vector, so it +must be pinned here too, not just the GET path. + +The cap is enforced once, in the shared helper both entry points call, so +these tests exercise the real endpoints rather than the helper directly: +a construct that looks right in isolation has repeatedly behaved +differently end to end on this branch. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest +from rest_framework import status + +from documents.tests.factories import DocumentFactory +from documents.views import _MAX_QUERY_LENGTH + +if TYPE_CHECKING: + from rest_framework.test import APIClient + + from documents.models import Document + +pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures("_search_index")] + + +@pytest.fixture +def indexed_document() -> Document: + from documents.search import get_backend + + doc = DocumentFactory.create(title="quarterly invoice", content="acme corp") + get_backend().add_or_update(doc) + return doc + + +class TestGetSearchEndpointEnforcesTheCap: + def test_query_one_over_the_cap_is_a_400( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + query = "a" * (_MAX_QUERY_LENGTH + 1) + + response = admin_client.get("/api/documents/", {"query": query}) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + message = str(response.data["query"]) + assert str(_MAX_QUERY_LENGTH) in message + assert str(_MAX_QUERY_LENGTH + 1) in message + + def test_query_at_exactly_the_cap_is_accepted( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + query = "a" * _MAX_QUERY_LENGTH + + response = admin_client.get("/api/documents/", {"query": query}) + + assert response.status_code == status.HTTP_200_OK + + def test_an_ordinary_query_is_unaffected( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + response = admin_client.get("/api/documents/", {"query": "invoice"}) + + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == 1 + + +class TestPostSelectionPathsEnforceTheCap: + """The bulk-edit and bulk-download selection filters share the same + helper the GET search path uses. This is the path that actually + matters: it is not bounded by a web server's header-length limit the + way the GET path incidentally is.""" + + def test_bulk_edit_query_one_over_the_cap_is_a_400( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + query = "a" * (_MAX_QUERY_LENGTH + 1) + + response = admin_client.post( + "/api/documents/bulk_edit/", + { + "documents": [], + "all": True, + "filters": {"query": query}, + "method": "set_document_type", + "parameters": {"document_type": None}, + }, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + message = str(response.data["query"]) + assert str(_MAX_QUERY_LENGTH) in message + assert str(_MAX_QUERY_LENGTH + 1) in message + + def test_bulk_edit_query_at_exactly_the_cap_is_accepted( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + query = "a" * _MAX_QUERY_LENGTH + + response = admin_client.post( + "/api/documents/bulk_edit/", + { + "documents": [], + "all": True, + "filters": {"query": query}, + "method": "set_document_type", + "parameters": {"document_type": None}, + }, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + + def test_bulk_download_query_one_over_the_cap_is_a_400( + self, + admin_client: APIClient, + indexed_document: Document, + ) -> None: + query = "a" * (_MAX_QUERY_LENGTH + 1) + + response = admin_client.post( + "/api/documents/bulk_download/", + { + "documents": [], + "all": True, + "filters": {"query": query}, + }, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + message = str(response.data["query"]) + assert str(_MAX_QUERY_LENGTH) in message + assert str(_MAX_QUERY_LENGTH + 1) in message diff --git a/src/documents/views.py b/src/documents/views.py index 345c3c35b..d4ce37fe3 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -16,6 +16,7 @@ from time import mktime from time import sleep from typing import TYPE_CHECKING from typing import Any +from typing import Final from typing import Literal from typing import NamedTuple from unicodedata import normalize @@ -279,17 +280,40 @@ logger = logging.getLogger("paperless.api") _TANTIVY_INTERSECT_THRESHOLD = 5_000 _TANTIVY_SEARCH_PARAM_NAMES = ("text", "title_search", "query", "more_like_id") +# whoosh-compat's fieldname tagger (used only for SearchMode.QUERY, via the +# whoosh grammar in parse_user_query) is O(n^2) in plain word characters: +# measured at ~0.96s/10k chars, ~3.67s/20k, ~14.4s/40k against the real field +# registry. Django's DATA_UPLOAD_MAX_MEMORY_SIZE default (2.5 MB) does not +# bound this on the POST-body selection-filter path, so an unbounded query +# is a single-request CPU exhaustion vector. 4096 chars caps the worst case +# at roughly 0.16s (quadratic extrapolation from the measurements above), +# far beyond any plausible hand-typed advanced query, while still being fast +# enough to absorb inside a request handler. Applied to all three modes at +# this shared choke point: TEXT and TITLE route through simple_search_tokens +# instead and measure linear even at 20k chars, so the cap is hygiene for +# them, not a fix, but a single limit here is simpler than one exemption. +# Not exposed as a PAPERLESS_* setting: this is a hard security boundary, +# not a tunable, and a raisable ceiling would let a misconfiguration +# reintroduce the exact hazard this exists to close. +_MAX_QUERY_LENGTH: Final[int] = 4096 + def _get_tantivy_query_and_mode(params): + from documents.search import QueryTooLongError from documents.search import SearchMode if "text" in params: - return str(params["text"]), SearchMode.TEXT - if "title_search" in params: - return str(params["title_search"]), SearchMode.TITLE - if "query" in params: - return str(params["query"]), SearchMode.QUERY - return None # pragma: no cover + raw, mode = str(params["text"]), SearchMode.TEXT + elif "title_search" in params: + raw, mode = str(params["title_search"]), SearchMode.TITLE + elif "query" in params: + raw, mode = str(params["query"]), SearchMode.QUERY + else: + return None # pragma: no cover + + if len(raw) > _MAX_QUERY_LENGTH: + raise QueryTooLongError(len(raw), _MAX_QUERY_LENGTH) + return raw, mode def _get_more_like_id(query_params: dict[str, Any], user: User | None) -> int: