Skip to content

Commit

Permalink
filters/keys: duplicate hashes while filtering
Browse files Browse the repository at this point in the history
Fixes #592 (Session deep values are modified if it contains filtered (blocklist)
keys)
  • Loading branch information
kyrylo committed Jul 31, 2020
1 parent ca4f1ad commit c95c571
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ Airbrake Ruby Changelog

### master

* Fixed unwanted mutation of nested hashes passed through `Notice#[]=`
([#597](https://github.com/airbrake/airbrake-ruby/pull/597))

### [v5.0.0.rc.2][v5.0.0.rc.2] (July 29, 2020)

* Started sending information about notifier name & version, operating system &
Expand Down
23 changes: 17 additions & 6 deletions lib/airbrake-ruby/filters/keys_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ def call(notice)
validate_patterns
end

FILTERABLE_KEYS.each { |key| filter_hash(notice[key]) }
FILTERABLE_KEYS.each do |key|
notice[key] = filter_hash(notice[key])
end

FILTERABLE_CONTEXT_KEYS.each { |key| filter_context_key(notice, key) }

return unless notice[:context][:url]
Expand All @@ -81,15 +84,21 @@ def should_filter?(_key)
def filter_hash(hash)
return hash unless hash.is_a?(Hash)

hash_copy = hash.dup

hash.each_key do |key|
if should_filter?(key.to_s)
hash[key] = FILTERED
elsif hash[key].is_a?(Hash)
filter_hash(hash[key])
hash_copy[key] = FILTERED
elsif hash_copy[key].is_a?(Hash)
hash_copy[key] = filter_hash(hash_copy[key])
elsif hash[key].is_a?(Array)
hash[key].each { |h| filter_hash(h) }
hash_copy[key].each_with_index do |h, i|
hash_copy[key][i] = filter_hash(h)
end
end
end

hash_copy
end

def filter_url_params(url)
Expand Down Expand Up @@ -142,7 +151,9 @@ def validate_patterns
def filter_context_key(notice, key)
return unless notice[:context][key]
return if notice[:context][key] == FILTERED
return filter_hash(notice[:context][key]) unless should_filter?(key)
unless should_filter?(key)
return notice[:context][key] = filter_hash(notice[:context][key])
end

notice[:context][key] = FILTERED
end
Expand Down
11 changes: 11 additions & 0 deletions spec/filters/keys_blocklist_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,17 @@
{ bongo: { bish: '[Filtered]' } },
],
)

it "doesn't mutate the original hash" do
blocklist = described_class.new([:bish])

params = { bongo: { bish: 'bash' } }
notice[:params] = params

blocklist.call(notice)

expect(params[:bongo][:bish]).to eq('bash')
end
end

context "and it is recursive" do
Expand Down
2 changes: 1 addition & 1 deletion spec/notice_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def to_json(*)
it "sets a payload value" do
hash = { bingo: 'bango' }
notice[:params] = hash
expect(notice[:params]).to equal(hash)
expect(notice[:params]).to eq(hash)
end

it "raises error if notice is ignored" do
Expand Down

0 comments on commit c95c571

Please sign in to comment.