Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Sanitize URLs in Span descriptions and breadcrumbs #7667

Merged
merged 9 commits into from
Mar 30, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Mar 30, 2023

This PR is a follow-up on #7206. just opening a new one because I can't push to the fork.
It has already been thoroughly reviewed and I just added a few clarifying comments and removed the deprecation as discussed in #7206 (comment).

All credits go to @aldenquimby

refs getsentry/team-webplatform-meta#2

@Lms24 Lms24 requested review from a team, mydea and lforst and removed request for a team March 30, 2023 10:17
@Lms24 Lms24 changed the title feat(node) Sanitize URLs in Span descriptions and breadcrumbs feat(node): Sanitize URLs in Span descriptions and breadcrumbs Mar 30, 2023
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.62 KB (+0.01% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 64.4 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 19.16 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 56.78 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 21.53 KB (0%)
@sentry/browser - Webpack (minified) 72 KB (0%)
@sentry/react - Webpack (gzipped + minified) 21.55 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 52.05 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 28.21 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.41 KB (+0.01% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 44.74 KB (+0.01% 🔺)
@sentry/replay - Webpack (gzipped + minified) 38.86 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 63.46 KB (+0.01% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 56.49 KB (+0.01% 🔺)

@AbhiPrasad
Copy link
Member

Merging so we can include with next release

@AbhiPrasad AbhiPrasad merged commit 9ecd152 into develop Mar 30, 2023
@AbhiPrasad AbhiPrasad deleted the lms/node-sanitize-span-descr-breadcrumbs branch March 30, 2023 10:51
Lms24 added a commit that referenced this pull request Apr 5, 2023
In #7667 we missed that our `urlToOptions` helper function is actually called to normalize request options that are then passed to the actual http client. Meaning, we shouldn't have redacted the authority in this function but at a later time when we extract the sanitized version (`extractUrl`). This PR changes the redaction location accordingly and hence fixes requests with authority not being sent properly. 

Co-authored-by: Luca Forstner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants