Bugsnag instrumentation causes URI: on redirect to badly formed (but working) url

[timdiggins]

Describe the bug

In v6.2.1, there is an addition to redact potential secrets from the location of redirects. This uses Bugsnag.cleaner.clean_url on the location which in turn uses URI() - this is more sensitive than browsers and will raise an exception on some RFC 3986 non-compliant but in practice actually working URLs (e.g. with spaces).

https://github.com/bugsnag/bugsnag-ruby/pull/806/files#r1449084164

As a result this addition can cause errors in specs and in practice.

Steps to reproduce

create a route & controller action that uses redirect_to "http://example.com/some spaces" exercise this route & controller action in a test and/or in reality with Bugsnag v6.26.0 -- will work successfully

exercise this route & controller action in a test and/or in reality with Bugsnag v6.26.1 -- will raise an error.

Suggestion

While it's not good practice in client code to have these non-compliant urls in any repo, I don't think Bugsnag should be the point of enforcement. As far as possible Bugsnag should not introduce any new exceptions - maybe worth rescueing any exception during a clean and skip the clean (return input) if the URI raises an error

Environment

• Bugsnag version: v6.26.1
• Ruby version: 3.0.6
• Bundle version: 2.2.33
• Integration framework version:
  • Rack: 2.2.8
  • Rails: 6.1.7.6

[keegnotrub]

Just a note that I've also experienced issues with this, even after the recent v6.26.2 release, causing us to need to revert back to v6.26.0. Checking our code, links like the following seem to fail still in the most recent release:

cleaner = Bugsnag::Cleaner.new(Bugsnag.configuration)
cleaner.clean_url("mailto:?subject=Hello")

[clr182]

Hi @keegnotrub,

This issue has been addressed in v6.26.3 of the bugsnag-ruby notifier. If after upgrade you still see issues, please let us know and we can investigate further.