mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-26 12:43:19 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7cb6b32a8f
commit
885bc2fdf3
@@ -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",
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
@@ -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
|
||||
+30
-6
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user