Skip to content

Commit

Permalink
Fix Array type wrapper in QueryParamsSanitizer (#825)
Browse files Browse the repository at this point in the history
When an value sanitized by the QueryParamsSanitizer was an Array at the
root level, and `only_top_level = true` was set, it would not return a
sanitized Array object, only the child values.

The Array wrapper value would be removed, returning an inaccurate
picture of the structure of the sanitized value.

```ruby
# Before change
> Appsignal::Utils::QueryParamsSanitizer.sanitize(["foo" => "bar"], false)
# [{"foo"=>"?"}]
> Appsignal::Utils::QueryParamsSanitizer.sanitize(["foo" => "bar"], true)
# {"foo"=>"?"}

# After change
> Appsignal::Utils::QueryParamsSanitizer.sanitize(["foo" => "bar"], false)
# [{"foo"=>"?"}]
> Appsignal::Utils::QueryParamsSanitizer.sanitize(["foo" => "bar"], true)
# [{"foo"=>"?"}]
```

Part of #820
  • Loading branch information
tombruijn committed Feb 22, 2022
1 parent 53e8425 commit d73905d
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 7 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-wrapper-array-around-sanitized-values.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix sanitized values wrapped in Arrays. When a value like `[{ "foo" => "bar" }]` was sanitized it would be stored as `{ "foo" => "?" }`, omitting the parent value's Array square brackets. Now values will appear with the same structure as they were originally sanitized. This only applies to certain integrations like MongoDB, moped and ElasticSearch.
2 changes: 1 addition & 1 deletion lib/appsignal/utils/query_params_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def sanitize_hash(hash, only_top_level, key_sanitizer)

def sanitize_array(array, only_top_level, key_sanitizer)
if only_top_level
sanitize(array[0], only_top_level, key_sanitizer)
[sanitize(array[0], only_top_level, key_sanitizer)]
else
array.map do |value|
sanitize(value, only_top_level, key_sanitizer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
)
end

it { is_expected.to eq ["Insert", '{:database=>"database.collection", :documents=>{"_id"=>"?", "events"=>"?"}, :count=>2, :flags=>[]}'] }
it { is_expected.to eq ["Insert", '{:database=>"database.collection", :documents=>[{"_id"=>"?", "events"=>"?"}], :count=>2, :flags=>[]}'] }
end

context "Moped::Protocol::Update" do
Expand Down
10 changes: 5 additions & 5 deletions spec/lib/appsignal/utils/query_params_sanitizer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
let(:value) { ["foo" => "bar"] }

it "should sanitize all hash values with a questionmark" do
expect(subject).to eq("foo" => "?")
expect(subject).to eq(["foo" => "?"])
end

it "should not modify source value" do
Expand All @@ -45,8 +45,8 @@
context "when value is an array" do
let(:value) { %w[foo bar] }

it "should only return the first level of the object" do
expect(subject).to eq("?")
it "sanitizes all array values" do
expect(subject).to eq(["?"])
end

it "should not modify source value" do
Expand All @@ -58,8 +58,8 @@
context "when value is a mixed array" do
let(:value) { [nil, "foo", "bar"] }

it "should sanitize all hash values with a single questionmark" do
expect(subject).to eq("?")
it "should sanitize all array values with a single questionmark" do
expect(subject).to eq(["?"])
end
end

Expand Down

0 comments on commit d73905d

Please sign in to comment.