Skip to content

Commit

Permalink
Filter invalid URLs with query strings
Browse files Browse the repository at this point in the history
As reported in #810, we don't handle invalid URLs in clean_url so raise
if the URL isn't valid

If the URL doesn't have a query string then it's not important that it
fails validation and so we can return it as-is

If the URL does have a query string and cannot be parsed, we can't
redact parameters individually and so redact the whole query string
instead — this way the URL could still be useful and we don't risk
leaking sensitive data
  • Loading branch information
imjoehaines committed Jan 16, 2024
1 parent 3084374 commit 6965366
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
10 changes: 9 additions & 1 deletion lib/bugsnag/cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,16 @@ def clean_object(object)
# @return [String]
def clean_url(url)
return url if @configuration.meta_data_filters.empty? && @configuration.redacted_keys.empty?
return url unless url.include?('?')

begin
uri = URI(url)
rescue URI::InvalidURIError
pre_query_string, query_string = url.split('?', 2)

return "#{pre_query_string}?#{FILTERED}"
end

uri = URI(url)
return url unless uri.query

query_params = uri.query.split('&').map { |pair| pair.split('=') }
Expand Down
12 changes: 12 additions & 0 deletions spec/cleaner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -540,5 +540,17 @@ def to_s
let(:url) { "https://host.example/sessions?access_token=abc123" }
it { should eq "https://host.example/sessions?access_token=[FILTERED]" }
end

context "with an invalid URL" do
let(:filters) { [/token/] }
let(:url) { "https://host.example/a b c d e f g?access_token=abc123&password=secret&token2=xyz987" }
it { should eq "https://host.example/a b c d e f g?[FILTERED]" }
end

context "with an invalid URL and no query string" do
let(:filters) { [/token/] }
let(:url) { "https://host.example/a b c d e f g" }
it { should eq "https://host.example/a b c d e f g" }
end
end
end

0 comments on commit 6965366

Please sign in to comment.