From 1855e57bfa193ea8d201e64656d071f6fb1a6148 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 18 Aug 2026 15:46:17 +0000 Subject: [PATCH] Fix search queries without a positive non-empty clause being allowed (#40196) --- app/lib/search_query_transformer.rb | 17 +++--- spec/lib/search_query_transformer_spec.rb | 64 ++++++++++++++++++++--- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/app/lib/search_query_transformer.rb b/app/lib/search_query_transformer.rb index fae2403281e..b484c0fcea1 100644 --- a/app/lib/search_query_transformer.rb +++ b/app/lib/search_query_transformer.rb @@ -20,6 +20,7 @@ class SearchQueryTransformer < Parslet::Transform @options = options flags_from_clauses! + validate_clauses! end def request @@ -34,6 +35,11 @@ class SearchQueryTransformer < Parslet::Transform private + def validate_clauses! + # At least one clause should be a positive match unless searching within the library + raise 'Empty query not supported' if @flags['in'] != 'library' && (must_clauses + filter_clauses).none? { |clause| clause.is_a?(TermClause) && clause.term.present? } + end + def clauses_by_operator @clauses_by_operator ||= @clauses.compact.group_by(&:operator) end @@ -130,16 +136,9 @@ class SearchQueryTransformer < Parslet::Transform end end - class PhraseClause - attr_reader :operator, :phrase - - def initialize(operator, phrase) - @operator = Operator.symbol(operator) - @phrase = phrase - end - + class PhraseClause < TermClause def to_query - { match_phrase: { text: { query: @phrase } } } + { match_phrase: { text: { query: @term } } } end end diff --git a/spec/lib/search_query_transformer_spec.rb b/spec/lib/search_query_transformer_spec.rb index e77d54f3635..8d0b032ddf5 100644 --- a/spec/lib/search_query_transformer_spec.rb +++ b/spec/lib/search_query_transformer_spec.rb @@ -18,13 +18,13 @@ RSpec.describe SearchQueryTransformer do ['"12345678"', '12345678'], ['"2024-10-31T23:47:20Z"', '2024-10-31T23:47:20Z'], ].each do |value, parsed| - context "with #{operator}:#{value}" do - let(:query) { "#{operator}:#{value}" } + context "with \"foo #{operator}:#{value}\"" do + let(:query) { "foo #{operator}:#{value}" } it 'transforms clauses' do ops = statement_operations.index_with { |_op| parsed } - expect(subject.send(:must_clauses)).to be_empty + expect(subject.send(:must_clauses).map(&:term)).to contain_exactly('foo') expect(subject.send(:must_not_clauses)).to be_empty expect(subject.send(:filter_clauses).map(&:term)).to contain_exactly(**ops, time_zone: 'UTC') end @@ -40,6 +40,56 @@ RSpec.describe SearchQueryTransformer do end end + context 'when there is no positive clause' do + context 'with "-hello"' do + let(:query) { '-hello' } + + it 'raises an exception' do + expect { subject }.to raise_error(RuntimeError) + end + end + + context 'with "after:0000"' do + let(:query) { 'after:0000' } + + it 'raises an exception' do + expect { subject }.to raise_error(RuntimeError) + end + end + + context 'with "before:9999"' do + let(:query) { 'before:9999' } + + it 'raises an exception' do + expect { subject }.to raise_error(RuntimeError) + end + end + + context 'with "is:reply"' do + let(:query) { 'is:reply' } + + it 'raises an exception' do + expect { subject }.to raise_error(RuntimeError) + end + end + + context 'with \'is:reply " "\'' do + let(:query) { 'is:reply " "' } + + it 'raises an exception' do + expect { subject }.to raise_error(RuntimeError) + end + end + + context 'with "in:library after:0000"' do + let(:query) { 'in:library after:0000' } + + it 'does not raise an exception' do + expect { subject }.to_not raise_error + end + end + end + context 'with "hello world"' do let(:query) { 'hello world' } @@ -94,17 +144,17 @@ RSpec.describe SearchQueryTransformer do let(:query) { '"hello world"' } it 'transforms clauses' do - expect(subject.send(:must_clauses).map(&:phrase)).to contain_exactly('hello world') + expect(subject.send(:must_clauses).map(&:term)).to contain_exactly('hello world') expect(subject.send(:must_not_clauses)).to be_empty expect(subject.send(:filter_clauses)).to be_empty end end - context 'with \'is:"foo bar"\'' do - let(:query) { 'is:"foo bar"' } + context 'with \'foo is:"foo bar"\'' do + let(:query) { 'foo is:"foo bar"' } it 'transforms clauses' do - expect(subject.send(:must_clauses)).to be_empty + expect(subject.send(:must_clauses).map(&:term)).to contain_exactly('foo') expect(subject.send(:must_not_clauses)).to be_empty expect(subject.send(:filter_clauses).map(&:term)).to contain_exactly('foo bar') end