Invalid combinators to raise and visitors to tolerate combinator case…

… and symbol differences

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Dangerous searches (e.g. ransack!) will now raise an ArgumentError when passed an unrecognised combinator rather than silently fall back on "AND".
+* Combinator values "OR" and :or are now treated as the expected 'or'.
+
 ## 4.1.0 - 2023-10-23
 
 ### 🚀 Features

lib/ransack/nodes/condition.rb

@@ -122,8 +122,9 @@ def combinator
       end
 
       def combinator=(val)
-        @combinator = Constants::AND_OR.detect { |v| v == val.to_s } || nil
+        super
       end
+
       alias :m= :combinator=
       alias :m :combinator
 

lib/ransack/nodes/grouping.rb

@@ -11,9 +11,13 @@ class Grouping < Node
 
       delegate :each, to: :values
 
+      def combinator=(val)
+        super
+      end
+
       def initialize(context, combinator = nil)
         super(context)
-        self.combinator = combinator.to_s if combinator
+        self.combinator = combinator if combinator
       end
 
       def persisted?

lib/ransack/nodes/node.rb

@@ -29,6 +29,10 @@ def translate(key, options = {})
         end
       end
 
+      def combinator=(val)
+        @combinator = Constants::AND_OR.detect { |v| v == val&.downcase&.to_s } || nil
+      end
+
     end
   end
 end

lib/ransack/search.rb

@@ -51,6 +51,13 @@ def build(params)
         elsif @context.ransackable_scope?(key, @context.object)
           add_scope(key, value)
         elsif base.attribute_method?(key)
+          if (key == Constants::COMBINATOR &&
+              Constants::AND_OR.exclude?(value.to_s) &&
+              (!Ransack.options[:ignore_unknown_conditions] || !@ignore_unknown_conditions))
+
+            raise ArgumentError, "Invalid combinator #{value}"
+          end
+
           base.send("#{key}=", value)
         elsif !Ransack.options[:ignore_unknown_conditions] || !@ignore_unknown_conditions
           raise ArgumentError, "Invalid search term #{key}"

spec/ransack/search_spec.rb

@@ -307,6 +307,52 @@ module Ransack
         end
       end
 
+      context 'with an invalid combinator' do
+        subject { Search.new(Person, name_eq: 'foobar', combinator: 'unknown') }
+
+        context 'when ignore_unknown_conditions configuration option is false' do
+          before do
+            Ransack.configure { |c| c.ignore_unknown_conditions = false }
+          end
+
+          specify { expect { subject }.to raise_error ArgumentError }
+        end
+
+        context 'when ignore_unknown_conditions configuration option is true' do
+          before do
+            Ransack.configure { |c| c.ignore_unknown_conditions = true }
+          end
+
+          specify { expect { subject }.not_to raise_error }
+        end
+
+        subject(:with_ignore_unknown_conditions_false) {
+          Search.new(Person,
+            { name_eq: 'foobar', combinator: 'unknown' },
+            { ignore_unknown_conditions: false }
+          )
+        }
+
+        subject(:with_ignore_unknown_conditions_true) {
+          Search.new(Person,
+            { name_eq: 'foobar', combinator: 'unknown' },
+            { ignore_unknown_conditions: true }
+          )
+        }
+
+        context 'when ignore_unknown_conditions search parameter is absent' do
+          specify { expect { subject }.not_to raise_error }
+        end
+
+        context 'when ignore_unknown_conditions search parameter is false' do
+          specify { expect { with_ignore_unknown_conditions_false }.to raise_error ArgumentError }
+        end
+
+        context 'when ignore_unknown_conditions search parameter is true' do
+          specify { expect { with_ignore_unknown_conditions_true }.not_to raise_error }
+        end
+      end
+
       it 'does not modify the parameters' do
         params = { name_eq: '' }
         expect { Search.new(Person, params) }.not_to change { params }

spec/ransack/visitor_spec.rb

@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+module Ransack
+  describe Visitor do
+
+    let(:viz) { Visitor.new }
+
+    shared_examples 'an or combinator' do | combinator |
+      it 'routes to #visit_or' do
+        expect(viz).to     receive(:visit_or)
+        expect(viz).to_not receive(:visit_and)
+
+        grouping = Ransack::Nodes::Grouping.new(nil, combinator)
+        viz.visit_Ransack_Nodes_Grouping(grouping)
+      end
+    end
+
+    shared_examples 'not an or combinator' do | combinator |
+      it 'routes to #visit_or' do
+        expect(viz).to_not receive(:visit_or)
+        expect(viz).to     receive(:visit_and)
+
+        grouping = Ransack::Nodes::Grouping.new(nil, combinator)
+        viz.visit_Ransack_Nodes_Grouping(grouping)
+      end
+    end
+
+    context 'combinator "or"'  do include_examples 'an or combinator', 'or' end
+    context 'combinator "OR"'  do include_examples 'an or combinator', 'OR' end
+    context 'combinator ":or"' do include_examples 'an or combinator', :or  end
+
+    context 'combinator "and"'     do include_examples 'not an or combinator', 'and'     end
+    context 'combinator "AND"'     do include_examples 'not an or combinator', 'AND'     end
+    context 'combinator ":and"'    do include_examples 'not an or combinator', :and      end
+    context 'combinator "unknown"' do include_examples 'not an or combinator', 'unknown' end
+  end
+end