Handle empty and FTS5-operator search queries without raising a 500 - #481
Conversation
Two adjacent crashes on the public book search path, both reachable via `?search=`: - A query that sanitizes down to nothing (e.g. `^$`, an emoji, `!!!`, a bare `"`) made `matches_for_highlight` return nil, so the highlight helper hit `nil.map`. Return an empty match set instead, which renders as normal un-highlighted content. - Bare FTS5 boolean operators (`OR`, `AND`, `NOT`, `NEAR`, ...) survived character sanitization and reached SQLite as an FTS5 syntax error. Rebuild the query from its balanced quoted phrases and bare words, quoting every token as a string literal so arbitrary input is matched literally instead of parsed as FTS5 syntax. This also subsumes the old unbalanced-quote handling. Ordinary term and phrase searches are unaffected.
There was a problem hiding this comment.
Pull request overview
Prevents malformed search input from causing server errors while preserving normal search behavior.
Changes:
- Quotes sanitized FTS5 tokens and handles empty queries.
- Adds model and controller regression coverage.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
app/models/leaf/searchable.rb |
Safely sanitizes queries and returns empty highlight matches. |
test/models/leaf/searchable_test.rb |
Covers empty and operator queries. |
test/controllers/leafables_controller_test.rb |
Covers safe leaf rendering. |
test/controllers/books/searches_controller_test.rb |
Covers safe search responses and normal matches. |
Suppressed comments (1)
test/controllers/books/searches_controller_test.rb:53
- Use the relative route helper in this integration test; a full URL is unnecessary because the request does not cross hosts.
post book_search_url(books(:handbook)), params: { search: "great handbook" }
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08658bd9eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
FTS5 highlight() spans can include document punctuation, so a phrase match against content like "alpha(beta" hands the highlight helper a term containing regex metacharacters. Interpolating it straight into /\b...\b/ raised a RegexpError (e.g. `?search=alpha_beta` on a page containing "alpha(beta"), breaking the page render. Regexp.escape the term so it is matched literally.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 225c7f01a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… empty-quote phrase boundary - Use book_search_path / leaves(:welcome_section) fixture / assert_in_body in the new search tests to match the repo's testing conventions. - quote_query_tokens: consume empty quote pairs in place so a stray "" no longer shifts a following phrase's quote boundaries and splits it into separate word matches; drop empty tokens.
String#gsub raises ArgumentError on invalid byte sequences, so a malformed query string could raise in sanitize_query_syntax. Rails rejects malformed request encoding with a 400 before either search controller runs, so this was not reachable as a 500 over HTTP — but scrubbing keeps the shared search sink total for every caller.
Drop the change-narrating comments on the scrub call, the highlight-term escaping, and the new tests; the code and test names carry the intent. Keep the quote_query_tokens comment, which documents non-obvious FTS5 behavior.
Three ways a weird query on the book search path could raise an unhandled exception and break the page render. All are reachable through the
?search=parameter (the search dialog and the highlighted leaf views:Books::SearchesController#createandLeafablesController#show).1. Query sanitizes to empty →
nil.mapA query made entirely of characters we strip (
^$, an emoji,!!!, a bare") sanitizes down to nothing.matches_for_highlightthen returnednil, and the highlight helper callednil.map. Now it returns[], which renders as normal un-highlighted content — matching the "no matches" behaviour the results view already has.2. FTS5 boolean operators → SQLite syntax error
Bare FTS5 operators (
OR,AND,NOT,NEAR, …) are word characters, so they survived character sanitization and reached SQLite as a raw FTS5 query, raisingfts5: syntax error near "OR".quote_query_tokensnow rebuilds the sanitized query from its balanced quoted phrases and bare words, quoting every token as a string literal so arbitrary input is matched literally. This also subsumes the previous unbalanced-quote handling (a stray"is simply never captured as a token).3. Punctuated phrase match →
RegexpErrorin the highlighterFTS5
highlight()spans can include document punctuation, so a phrase match against content likealpha(betahands the highlight helper a term containing regex metacharacters. Interpolating it straight into/\b...\b/raised aRegexpError(e.g.?search=alpha_betaon a page containingalpha(beta). The term is nowRegexp.escaped.Ordinary term and phrase searches are unaffected —
great handbookand"great handbook"still match as before. Boolean-operator queries now match their operators literally rather than as syntax; this is a deliberate tradeoff for a free-text search box that never documented Boolean support.Tests
matches_for_highlightis empty for empty-sanitizing queries;searchtreats operators literally without raising and keeps normal/phrase matches working.leafables#showand book search render:successfor empty-sanitizing, operator, and punctuation-spanning queries; ordinary multi-word search still returns results.Full suite green; rubocop and brakeman clean.