feat: filter username and password in URLs #864
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #864 +/- ##
==========================================
- Coverage 73.06% 73.03% -0.03%
==========================================
Files 64 64
Lines 7207 7266 +59
==========================================
+ Hits 5266 5307 +41
- Misses 1941 1959 +18 🚀 New features to boost your workflow:
|
| let _ = url.set_username(PII_REPLACEMENT); | ||
| } | ||
| if url.password().is_some() { | ||
| let _ = url.set_password(Some(PII_REPLACEMENT)); |
There was a problem hiding this comment.
The cases in which these can fail are documented as follows:
If this URL is cannot-be-a-base or does not have a host, do nothing and return `Err`.
This happens for instance if the URL is relative. In these cases we just ignore the errors.
There was a problem hiding this comment.
Please add this information as a code comment, as it could be helpful for future developers
There was a problem hiding this comment.
Mostly looks good, please address my inline comments before merging.
Also, we discussed this offline: as a further improvement (so not blocking this PR), I would suggest enforcing the URL stripping via the type system, by creating a new type called something like SafeUrl or StrippedUrl, which would wrap the existing Url type, and requiring the SafeUrl to be used when sending URLs to Sentry. This SafeUrl would ensure via the constructor that any URLs it is wrapping are stripped. This avoids the possibility of a future SDK dev forgetting to strip the URL
| let _ = url.set_username(PII_REPLACEMENT); | ||
| } | ||
| if url.password().is_some() { | ||
| let _ = url.set_password(Some(PII_REPLACEMENT)); |
There was a problem hiding this comment.
Please add this information as a code comment, as it could be helpful for future developers
Close #516