Skip to content

Commit 49f3392

Browse files
committed
Fix _discard leaking into SQL for multi-select enum filters
When an enum filter is used in multi-select mode, _discard gets submitted as a literal value on subsequent page loads, producing invalid SQL like: WHERE (col IN ('_discard','val1','val2')) Client-side: deselect _discard option when initializing multi-select. Server-side: strip _discard from array values as defense-in-depth.
1 parent d8e0809 commit 49f3392

5 files changed

Lines changed: 22 additions & 3 deletions

File tree

lib/rails_admin/abstract_model.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,13 @@ def initialize(column, type, value, operator)
140140
end
141141

142142
def to_statement
143-
return if [@operator, @value].any? { |v| v == '_discard' }
143+
return if @operator == '_discard'
144+
return if @value == '_discard'
145+
146+
if @value.is_a?(Array) && @value.include?('_discard')
147+
@value = @value.reject { |v| v == '_discard' }
148+
return if @value.empty?
149+
end
144150

145151
unary_operators[@operator] || unary_operators[@value] ||
146152
build_statement_for_type_generic

spec/controllers/rails_admin/main_controller_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ def get(action, params)
472472
'delete_paperclip_asset' => 'test',
473473
'should_not_be_here' => 'test',
474474
}.merge(defined?(ActiveStorage) ? {'active_storage_asset' => 'test', 'remove_active_storage_asset' => 'test', 'active_storage_assets' => 'test', 'remove_active_storage_assets' => 'test'} : {}).
475-
merge(defined?(Shrine) ? {'shrine_asset' => 'test', 'remove_shrine_asset' => 'test'} : {}),
475+
merge(defined?(Shrine) ? {'shrine_asset' => 'test', 'remove_shrine_asset' => 'test'} : {}),
476476
)
477477

478478
controller.send(:sanitize_params_for!, :create, RailsAdmin.config(FieldTest), controller.params['field_test'])

spec/rails_admin/adapters/active_record_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,11 @@ def build_statement(type, value, operator)
305305
end
306306
end
307307

308+
it "strips '_discard' from array values" do
309+
expect(build_statement(:enum, ['_discard'], nil)).to be_nil
310+
expect(build_statement(:enum, %w[_discard foo bar], nil)).to eq(['(field IN (?))', %w[foo bar]])
311+
end
312+
308313
describe 'string type queries' do
309314
it 'supports string type query' do
310315
expect(build_statement(:string, '', nil)).to be_nil

spec/rails_admin/adapters/mongoid_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,11 @@ def parse_value(value)
277277
end
278278
end
279279

280+
it "strips '_discard' from array values" do
281+
expect(@abstract_model.send(:build_statement, :name, :enum, ['_discard'], nil)).to be_nil
282+
expect(@abstract_model.send(:build_statement, :name, :enum, %w[_discard foo bar], nil)).to eq(name: {'$in' => %w[foo bar]})
283+
end
284+
280285
it "supports '_blank' operator" do
281286
[['_blank', ''], ['', '_blank']].each do |value, operator|
282287
expect(@abstract_model.send(:build_statement, :name, :string, value, operator)).to eq(name: {'$in' => [nil, '']})

src/rails_admin/filter-box.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ import flatpickr from "flatpickr";
102102
$(this).attr("selected", true);
103103
});
104104
if (multiple)
105-
control.find("option[value^=_],option[disabled]").hide();
105+
control
106+
.find("option[value^=_],option[disabled]")
107+
.hide()
108+
.prop("selected", false);
106109
}
107110
break;
108111
case "citext":

0 commit comments

Comments
 (0)