Handle empty and FTS5-operator search queries without raising a 500 (#481)

* 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.

* 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.

* 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.

* Scrub invalid UTF-8 before sanitizing search queries

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.

* 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.

Author: jeremy

app/helpers/searches_helper.rb

@@ -23,6 +23,6 @@ def highlight_searched_content(leaf, content, query)
 
   private
     def whole_word_matchers(terms)
-      terms.map { |term| /\b#{term}\b/ }
+      terms.map { |term| /\b#{Regexp.escape(term)}\b/ }
     end
 end

app/models/leaf/searchable.rb

@@ -15,9 +15,9 @@ def reindex_all
     end
 
     def sanitize_query_syntax(terms)
-      terms = terms.to_s
+      terms = terms.to_s.scrub
       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, '<mark>', '</mark>')"))
 
       content ? unique_matching_terms(content) : []
+    else
+      []
     end
   end
 
@@ -106,12 +108,22 @@ 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.
+        #
+        # 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+/)
+            .filter_map { |token| token.delete('"').presence }
+            .map { |token| %("#{token}") }
+            .join(" ")
         end
     end
 end

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_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_path(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!
 

test/controllers/leafables_controller_test.rb

@@ -30,6 +30,44 @@ 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_in_body "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 raise when a phrase match spans regex metacharacters" do
+    sign_out
+    books(:handbook).update!(published: true)
+
+    sections(:welcome).update!(body: "alpha(beta gamma in the body")
+    leaves(:welcome_section).reindex
+
+    get leafable_slug_path(leaves(:welcome_section)), 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
 

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 "<mark>findme</mark> text", sanitize_search_result("<mark>findme</mark> text")
   end
@@ -16,4 +18,14 @@ class SearchesHelperTest < ActionView::TestCase
   test "sanitize_search_result strips attributes from mark tags" do
     assert_equal "<mark>findme</mark> text", sanitize_search_result('<mark class="hidden">findme</mark> 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, "<mark>alpha(beta</mark>"
+  end
 end

test/models/leaf/searchable_test.rb

@@ -38,6 +38,41 @@ 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
+    assert_empty Leaf.search("OR")
+    assert_empty Leaf.search("great AND NOT")
+    assert_empty Leaf.search("great OR handbook")
+
+    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
+    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 "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 <img src=x onerror="alert(1)">')
     books(:handbook).press(section, title: "Safe Title")