Skip to content

Commit

Permalink
Do not mutate search_state when mutating search_state.to_h
Browse files Browse the repository at this point in the history
`Blacklight::SearchState#to_h` needs to return a dupe of it's internal
state @params not a reference to it.  Otherwise whatever mutation you do
to the reference gets reflected in the Blacklight::SearchState instance.

It's unexpected behavior for the `to_h` method to expose the
`Blacklight::SearchState` instance's copy of `@params` and it can lead
to very confusing bugs.
  • Loading branch information
dkinzer committed Sep 25, 2020
1 parent d582cf5 commit ba5e3a6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/blacklight/search_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def self.normalize_params(untrusted_params = {})
end

def to_hash
@params
@params.dup
end
alias to_h to_hash

Expand Down
12 changes: 12 additions & 0 deletions spec/lib/blacklight/search_state_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@
expect(search_state.to_h).to include f: { field: %w[first second] }
end
end

context 'deleting item from to_h' do
let(:params) { { q: 'foo', q_1: 'bar' } }

it 'does not mutate search_state to mutate search_state.to_h' do
params = search_state.to_h
params.delete(:q_1)

expect(search_state.to_h).to eq('q' => 'foo', 'q_1' => 'bar')
expect(params).to eq('q' => 'foo')
end
end
end

describe 'interface compatibility with params' do
Expand Down

0 comments on commit ba5e3a6

Please sign in to comment.