Skip to content

Validate search query values before sending to solr#2161

Merged
alastair merged 2 commits into
masterfrom
skip-invalid-search-queries
Jul 13, 2026
Merged

Validate search query values before sending to solr#2161
alastair merged 2 commits into
masterfrom
skip-invalid-search-queries

Conversation

@alastair

Copy link
Copy Markdown
Member

Description
We found that scrapers/attackers construct search urls incorrectly
(they double-escape spaces) so we end up with filters like
samplerate:22050+tag:"x" which parse as samplerate:22050+tag
and causes solr to raise an error.

Validate filter strings and values before sending the result to solr.
If it looks invalid then directly return an error page instead of
trying the search.

Check for errors on search, tag, map bytearray, and clustering views

Deployment steps:
This should have a "real" effect on search results, I expect at least the following:

  • real number of queries to solr will go down if we drop the requests - we don't count the number of invalid responses but we could probably simulate it based on (requests)-(queries to solr)
  • we may be able to handle "more concurrent search requests", simply because we return a response quickly instead sending to solr: The nginx limit is based on "concurrent requests" so invalid searches will allow us to move on to handling the next search request faster
  • slightly possible that real queries will hit this and get an error page. Not sure the best way to track this - maybe we can look for "searches that have a real referrer and have an error", but I don't think there's a way that we can identify this case in our stats. we might want to make a prometheus counter.

alastair added 2 commits July 13, 2026 15:48
We found that scrapers/attackers construct search urls incorrectly
(they double-escape spaces) so we end up with filters like
`samplerate:22050+tag:"x"` which parse as `samplerate:22050+tag`
and causes solr to raise an error.

Validate filter strings and values before sending the result to solr.
If it looks invalid then directly return an error page instead of
trying the search.

Check for errors on search, tag, map bytearray, and clustering views
get_clusters_for_query returns the {"clusters": None} sentinel
when the clustering task fails or times out, so don't try and load
results["graph"] unconditionally
@alastair alastair force-pushed the skip-invalid-search-queries branch from 4dfef66 to c8ef2a5 Compare July 13, 2026 13:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds validation for malformed/abuse-style f= filter querystrings so invalid filter values are detected early and do not reach Solr (avoiding Solr parse exceptions and reducing wasted query load).

Changes:

  • Introduce utils.search.filter_validation to parse filters and validate that typed filter fields (e.g., samplerate, bitrate) have values castable to the expected type.
  • Integrate filter-type validation into SearchQueryProcessor, and prevent query param generation when filter errors exist.
  • Short-circuit clustering and map-bytearray endpoints when SearchQueryProcessor reports filter errors, with accompanying test coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
utils/tests/test_filter_validation.py Adds unit tests covering malformed filter encodings, dropped filters, and SQP behavior when errors are present.
utils/search/search_query_processor.py Uses shared filter parsing + type validation and raises when attempting to build query params while errors exist.
utils/search/filter_validation.py New module implementing filter parsing, typed-field validation, and detection of silently dropped +-prefixed filters.
search/views.py Avoids clustering work / returns error JSON when SQP has invalid filters and fixes missing return on error response.
search/tests/test_views.py Adds regression tests ensuring clustering endpoints short-circuit on invalid filters and timeout-like results.
geotags/views.py Avoids Solr querying for map bytearray endpoint when SQP has invalid filters.
geotags/tests.py Adds test ensuring invalid filters yield an empty bytearray response.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ffont ffont left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a comment with potential changes but maybe for another PR. Also, wouldn't this be useful to avoid using our rather hacky code that compensates for the AND/OR filter terms default change that happened in solr some time ago? Remember we wrote some ugly code to add the + signs? Or did we already get rid of that? :S

import luqum.tree
from luqum.parser import parser

FIELD_TYPES_MAP = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are starting to "hardcode" search fields information in several places (see

SEARCH_SOUNDS_FIELD_ID = "sound_id"
). In some cases at the Solr level, in other cases at a more abstract "could be applied to any search backend" level. I wonder if we should start thinking of an alternative way of doing that. Probably one single dict in settings which also includes the types... For now maybe simply add a comment here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, yeah. we could have a dict of dataclasses or something - with field name, type, default weights, facet values, etc. it should end up consolidating all search field settings into a single location

@alastair alastair merged commit 7fad5db into master Jul 13, 2026
3 checks passed
@alastair alastair deleted the skip-invalid-search-queries branch July 13, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants