From 08658bd9ebaaabdb69b92cf497e7be605c525767 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 25 Aug 2026 22:13:39 -0700 Subject: [PATCH 1/5] Handle empty and FTS5-operator search queries without raising a 500 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. --- app/models/leaf/searchable.rb | 20 +++++++++------ .../books/searches_controller_test.rb | 15 +++++++++++ test/controllers/leafables_controller_test.rb | 25 +++++++++++++++++++ test/models/leaf/searchable_test.rb | 18 +++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/app/models/leaf/searchable.rb b/app/models/leaf/searchable.rb index d90c5c91..79d04acc 100644 --- a/app/models/leaf/searchable.rb +++ b/app/models/leaf/searchable.rb @@ -17,7 +17,7 @@ def reindex_all def sanitize_query_syntax(terms) terms = terms.to_s terms = remove_invalid_search_characters(terms) - terms = remove_unbalanced_quotes(terms) + terms = quote_query_tokens(terms) terms.presence end @@ -50,6 +50,8 @@ def matches_for_highlight(terms) .pick(Arel.sql("highlight(leaf_search_index, 1, '', '')")) content ? unique_matching_terms(content) : [] + else + [] end end @@ -106,12 +108,16 @@ def remove_invalid_search_characters(terms) terms.gsub(/[^\w"]/, " ") end - def remove_unbalanced_quotes(terms) - if terms.count("\"").even? - terms - else - terms.gsub("\"", " ") - end + # After stripping the characters FTS5 can't tokenize, the remaining + # input may still be an FTS5 boolean operator (AND/OR/NOT/NEAR) or + # carry an unbalanced double quote — either of which makes SQLite raise + # a syntax error. Rebuild the query from its balanced "quoted phrases" + # and bare words, wrapping every token as a quoted string literal so + # arbitrary input is matched literally instead of parsed as syntax. + def quote_query_tokens(terms) + terms.scan(/"[^"]+"|\w+/) + .map { |token| %("#{token.delete('"')}") } + .join(" ") end end end diff --git a/test/controllers/books/searches_controller_test.rb b/test/controllers/books/searches_controller_test.rb index 00777312..17525a02 100644 --- a/test/controllers/books/searches_controller_test.rb +++ b/test/controllers/books/searches_controller_test.rb @@ -41,6 +41,21 @@ class Books::SearchesControllerTest < ActionDispatch::IntegrationTest assert_select "p", text: /no matches/i end + test "create shows no matches when the search uses FTS5 operator syntax" do + [ "OR", "AND", "NOT", "great OR", "NEAR handbook", "great AND NOT" ].each do |query| + post book_search_url(books(:handbook)), params: { search: query } + + assert_response :success, "expected #{query.inspect} to render without error" + end + end + + test "create still finds matches for an ordinary multi-word query" do + post book_search_url(books(:handbook)), params: { search: "great handbook" } + + assert_response :success + assert_select "a.search__result" + end + test "create does not find trashed pages" do leaves(:summary_page).trashed! diff --git a/test/controllers/leafables_controller_test.rb b/test/controllers/leafables_controller_test.rb index d9e383c7..80b94cda 100644 --- a/test/controllers/leafables_controller_test.rb +++ b/test/controllers/leafables_controller_test.rb @@ -30,6 +30,31 @@ class LeafablesControllerTest < ActionDispatch::IntegrationTest assert_select "mark", "great" end + test "show does not raise when the search query sanitizes to empty" do + sign_out + books(:handbook).update!(published: true) + Leaf.reindex_all + + [ "^$", "!!!", "🙂", "\"" ].each do |query| + get leafable_slug_path(leaves(:welcome_page)), params: { search: query } + + assert_response :success, "expected #{query.inspect} to render without error" + assert_select "p", "This is such a great handbook." + end + end + + test "show does not raise when the search query uses FTS5 operator syntax" do + sign_out + books(:handbook).update!(published: true) + Leaf.reindex_all + + [ "OR", "AND", "NOT", "great OR", "NEAR handbook", "great AND NOT" ].each do |query| + get leafable_slug_path(leaves(:welcome_page)), params: { search: query } + + assert_response :success, "expected #{query.inspect} to render without error" + end + end + test "show does not allow public access to an unpublished book" do sign_out diff --git a/test/models/leaf/searchable_test.rb b/test/models/leaf/searchable_test.rb index b94d77b5..152d6b3c 100644 --- a/test/models/leaf/searchable_test.rb +++ b/test/models/leaf/searchable_test.rb @@ -38,6 +38,24 @@ class Leaf::SearchableTest < ActiveSupport::TestCase assert_empty markup end + test "matches_for_highlight is empty when the query sanitizes to nothing" do + assert_empty leaves(:welcome_page).matches_for_highlight("^$") + assert_empty leaves(:welcome_page).matches_for_highlight("🙂") + assert_empty leaves(:welcome_page).matches_for_highlight("\"") + end + + test "search treats FTS5 operators as literal terms rather than syntax" do + # Operator tokens are searched literally, so they match only if the document + # actually contains that word — never raising an FTS5 syntax error. + assert_empty Leaf.search("OR") + assert_empty Leaf.search("great AND NOT") + assert_empty Leaf.search("great OR handbook") + + # A legitimate multi-term query keeps working after the escaping change. + assert_includes Leaf.search("great handbook"), leaves(:welcome_page) + assert_includes Leaf.search("\"great handbook\""), leaves(:welcome_page) + end + test "indexing sanitizes section body" do section = Section.new(body: 'findme Tom & Jerry ') books(:handbook).press(section, title: "Safe Title") From 225c7f01a899153051809c2f05752fa51a078656 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 25 Aug 2026 22:32:45 -0700 Subject: [PATCH 2/5] Escape highlight terms so punctuated phrase matches don't raise 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. --- app/helpers/searches_helper.rb | 5 ++++- test/controllers/leafables_controller_test.rb | 14 ++++++++++++++ test/helpers/searches_helper_test.rb | 12 ++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/app/helpers/searches_helper.rb b/app/helpers/searches_helper.rb index f3b22e69..e134759d 100644 --- a/app/helpers/searches_helper.rb +++ b/app/helpers/searches_helper.rb @@ -22,7 +22,10 @@ def highlight_searched_content(leaf, content, query) end private + # Terms come from FTS5 highlight() spans, which can include document + # punctuation (e.g. a phrase match spanning "alpha(beta"). Escape them so + # metacharacters are matched literally instead of raising a RegexpError. def whole_word_matchers(terms) - terms.map { |term| /\b#{term}\b/ } + terms.map { |term| /\b#{Regexp.escape(term)}\b/ } end end diff --git a/test/controllers/leafables_controller_test.rb b/test/controllers/leafables_controller_test.rb index 80b94cda..0b37b888 100644 --- a/test/controllers/leafables_controller_test.rb +++ b/test/controllers/leafables_controller_test.rb @@ -55,6 +55,20 @@ class LeafablesControllerTest < ActionDispatch::IntegrationTest end end + test "show does not raise when a phrase match spans regex metacharacters" do + sign_out + books(:handbook).update!(published: true) + + section = Section.new(body: "alpha(beta gamma in the body") + books(:handbook).press(section, title: "Punctuated") + section.leaf.reindex + + get leafable_slug_path(section.leaf), params: { search: "alpha_beta" } + + assert_response :success + assert_select "mark", text: /alpha\(beta/ + end + test "show does not allow public access to an unpublished book" do sign_out diff --git a/test/helpers/searches_helper_test.rb b/test/helpers/searches_helper_test.rb index 7b144406..8dc6cb7f 100644 --- a/test/helpers/searches_helper_test.rb +++ b/test/helpers/searches_helper_test.rb @@ -1,6 +1,8 @@ require "test_helper" class SearchesHelperTest < ActionView::TestCase + include PagesHelper + test "sanitize_search_result preserves mark tags" do assert_equal "findme text", sanitize_search_result("findme text") end @@ -16,4 +18,14 @@ class SearchesHelperTest < ActionView::TestCase test "sanitize_search_result strips attributes from mark tags" do assert_equal "findme text", sanitize_search_result(' text') end + + test "highlight_searched_content handles matched terms containing regex metacharacters" do + leaf = Struct.new(:terms) do + def matches_for_highlight(_query) = terms + end.new([ "alpha(beta" ]) + + result = highlight_searched_content(leaf, "alpha(beta in the body", "alpha beta") + + assert_includes result, "alpha(beta" + end end From 2c7e990726249442a553b91e421a965de5ccc53f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 25 Aug 2026 23:58:08 -0700 Subject: [PATCH 3/5] Address review: path helpers, response-body assertion, fixture reuse, 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. --- app/models/leaf/searchable.rb | 10 ++++++++-- test/controllers/books/searches_controller_test.rb | 4 ++-- test/controllers/leafables_controller_test.rb | 9 ++++----- test/models/leaf/searchable_test.rb | 13 +++++++++++++ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/app/models/leaf/searchable.rb b/app/models/leaf/searchable.rb index 79d04acc..436a0261 100644 --- a/app/models/leaf/searchable.rb +++ b/app/models/leaf/searchable.rb @@ -114,9 +114,15 @@ def remove_invalid_search_characters(terms) # a syntax error. Rebuild the query from its balanced "quoted phrases" # and bare words, wrapping every token as a quoted string literal so # arbitrary input is matched literally instead of parsed as syntax. + # + # Match empty quote pairs too (`[^"]*`, not `+`) so a stray `""` is + # consumed in place rather than pairing its closing quote with the next + # opening one — which would shift the boundaries of a following phrase + # and split it into separate word matches. Empty tokens are then dropped. def quote_query_tokens(terms) - terms.scan(/"[^"]+"|\w+/) - .map { |token| %("#{token.delete('"')}") } + terms.scan(/"[^"]*"|\w+/) + .filter_map { |token| token.delete('"').presence } + .map { |token| %("#{token}") } .join(" ") end end diff --git a/test/controllers/books/searches_controller_test.rb b/test/controllers/books/searches_controller_test.rb index 17525a02..13be87b5 100644 --- a/test/controllers/books/searches_controller_test.rb +++ b/test/controllers/books/searches_controller_test.rb @@ -43,14 +43,14 @@ class Books::SearchesControllerTest < ActionDispatch::IntegrationTest test "create shows no matches when the search uses FTS5 operator syntax" do [ "OR", "AND", "NOT", "great OR", "NEAR handbook", "great AND NOT" ].each do |query| - post book_search_url(books(:handbook)), params: { search: query } + post book_search_path(books(:handbook)), params: { search: query } assert_response :success, "expected #{query.inspect} to render without error" end end test "create still finds matches for an ordinary multi-word query" do - post book_search_url(books(:handbook)), params: { search: "great handbook" } + post book_search_path(books(:handbook)), params: { search: "great handbook" } assert_response :success assert_select "a.search__result" diff --git a/test/controllers/leafables_controller_test.rb b/test/controllers/leafables_controller_test.rb index 0b37b888..626ac569 100644 --- a/test/controllers/leafables_controller_test.rb +++ b/test/controllers/leafables_controller_test.rb @@ -39,7 +39,7 @@ class LeafablesControllerTest < ActionDispatch::IntegrationTest get leafable_slug_path(leaves(:welcome_page)), params: { search: query } assert_response :success, "expected #{query.inspect} to render without error" - assert_select "p", "This is such a great handbook." + assert_in_body "a great handbook." end end @@ -59,11 +59,10 @@ class LeafablesControllerTest < ActionDispatch::IntegrationTest sign_out books(:handbook).update!(published: true) - section = Section.new(body: "alpha(beta gamma in the body") - books(:handbook).press(section, title: "Punctuated") - section.leaf.reindex + sections(:welcome).update!(body: "alpha(beta gamma in the body") + leaves(:welcome_section).reindex - get leafable_slug_path(section.leaf), params: { search: "alpha_beta" } + get leafable_slug_path(leaves(:welcome_section)), params: { search: "alpha_beta" } assert_response :success assert_select "mark", text: /alpha\(beta/ diff --git a/test/models/leaf/searchable_test.rb b/test/models/leaf/searchable_test.rb index 152d6b3c..53d6a9e2 100644 --- a/test/models/leaf/searchable_test.rb +++ b/test/models/leaf/searchable_test.rb @@ -56,6 +56,19 @@ class Leaf::SearchableTest < ActiveSupport::TestCase assert_includes Leaf.search("\"great handbook\""), leaves(:welcome_page) end + test "a stray empty quote pair does not split a following phrase" do + # "" must be consumed in place; otherwise the phrase "great handbook" + # degrades into separate word matches that also hit documents where the + # two words are present but non-adjacent. + sections(:welcome).update!(body: "great old handbook") + leaves(:welcome_section).reindex + + results = Leaf.search("\"\" \"great handbook\"") + + assert_includes results, leaves(:welcome_page) + assert_not_includes results, leaves(:welcome_section) + end + test "indexing sanitizes section body" do section = Section.new(body: 'findme Tom & Jerry ') books(:handbook).press(section, title: "Safe Title") From 0a156882aeb29629b3cab405061f98533281de1e Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 26 Aug 2026 00:16:28 -0700 Subject: [PATCH 4/5] Scrub invalid UTF-8 before sanitizing search queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/models/leaf/searchable.rb | 6 +++++- test/models/leaf/searchable_test.rb | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/leaf/searchable.rb b/app/models/leaf/searchable.rb index 436a0261..42d8aa86 100644 --- a/app/models/leaf/searchable.rb +++ b/app/models/leaf/searchable.rb @@ -15,7 +15,11 @@ def reindex_all end def sanitize_query_syntax(terms) - terms = terms.to_s + # scrub replaces any invalid UTF-8 bytes so the gsub below can't raise an + # ArgumentError on a malformed string. Reachable over HTTP only in theory + # — Rails rejects malformed query/body encoding with a 400 first — but it + # keeps this shared sink total for every caller. + terms = terms.to_s.scrub terms = remove_invalid_search_characters(terms) terms = quote_query_tokens(terms) terms.presence diff --git a/test/models/leaf/searchable_test.rb b/test/models/leaf/searchable_test.rb index 53d6a9e2..1e2a2f98 100644 --- a/test/models/leaf/searchable_test.rb +++ b/test/models/leaf/searchable_test.rb @@ -69,6 +69,16 @@ class Leaf::SearchableTest < ActiveSupport::TestCase assert_not_includes results, leaves(:welcome_section) end + test "search does not raise on invalid UTF-8 byte sequences" do + malformed = "caf\xFF".dup.force_encoding("UTF-8") + assert_not malformed.valid_encoding? + + assert_nothing_raised do + assert_empty Leaf.search(malformed) + assert_empty leaves(:welcome_page).matches_for_highlight(malformed) + end + end + test "indexing sanitizes section body" do section = Section.new(body: 'findme Tom & Jerry ') books(:handbook).press(section, title: "Safe Title") From 58075dd913ccb6f88d288e6f7c2cc35fda8d4008 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 26 Aug 2026 00:30:30 -0700 Subject: [PATCH 5/5] Trim breadcrumby commentary from the search changes 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. --- app/helpers/searches_helper.rb | 3 --- app/models/leaf/searchable.rb | 4 ---- test/models/leaf/searchable_test.rb | 6 ------ 3 files changed, 13 deletions(-) diff --git a/app/helpers/searches_helper.rb b/app/helpers/searches_helper.rb index e134759d..754f898d 100644 --- a/app/helpers/searches_helper.rb +++ b/app/helpers/searches_helper.rb @@ -22,9 +22,6 @@ def highlight_searched_content(leaf, content, query) end private - # Terms come from FTS5 highlight() spans, which can include document - # punctuation (e.g. a phrase match spanning "alpha(beta"). Escape them so - # metacharacters are matched literally instead of raising a RegexpError. def whole_word_matchers(terms) terms.map { |term| /\b#{Regexp.escape(term)}\b/ } end diff --git a/app/models/leaf/searchable.rb b/app/models/leaf/searchable.rb index 42d8aa86..672e8048 100644 --- a/app/models/leaf/searchable.rb +++ b/app/models/leaf/searchable.rb @@ -15,10 +15,6 @@ def reindex_all end def sanitize_query_syntax(terms) - # scrub replaces any invalid UTF-8 bytes so the gsub below can't raise an - # ArgumentError on a malformed string. Reachable over HTTP only in theory - # — Rails rejects malformed query/body encoding with a 400 first — but it - # keeps this shared sink total for every caller. terms = terms.to_s.scrub terms = remove_invalid_search_characters(terms) terms = quote_query_tokens(terms) diff --git a/test/models/leaf/searchable_test.rb b/test/models/leaf/searchable_test.rb index 1e2a2f98..5ab163ea 100644 --- a/test/models/leaf/searchable_test.rb +++ b/test/models/leaf/searchable_test.rb @@ -45,21 +45,15 @@ class Leaf::SearchableTest < ActiveSupport::TestCase end test "search treats FTS5 operators as literal terms rather than syntax" do - # Operator tokens are searched literally, so they match only if the document - # actually contains that word — never raising an FTS5 syntax error. assert_empty Leaf.search("OR") assert_empty Leaf.search("great AND NOT") assert_empty Leaf.search("great OR handbook") - # A legitimate multi-term query keeps working after the escaping change. assert_includes Leaf.search("great handbook"), leaves(:welcome_page) assert_includes Leaf.search("\"great handbook\""), leaves(:welcome_page) end test "a stray empty quote pair does not split a following phrase" do - # "" must be consumed in place; otherwise the phrase "great handbook" - # degrades into separate word matches that also hit documents where the - # two words are present but non-adjacent. sections(:welcome).update!(body: "great old handbook") leaves(:welcome_section).reindex