Skip to content

Conversation

@lcian
Copy link
Member

@lcian lcian commented Feb 25, 2025

📜 Description

This changes the internal logic used for parsing URLs in UrlUtils from regex based to using java.net.URI.
This will allow the SDK to handle the cases described in #2690 and getsentry/team-sdks#39.
java.net.URI is known to still not be fully compliant with RFCs 3986 and 3987 (https://cr.openjdk.org/%7Edfuchs/writeups/updating-uri/) but this is still an improvement with respect to the cases described in the issues linked above, and could also ensure correct handling for other corner cases that we are not aware of.
We still use java.net.URL but just for validating that an URI is a valid URL.

This could affect grouping for existing issues that use (parts of) an URL as the message when the URL falls in the affected corner cases.

💡 Motivation and Context

Closes #2690

💚 How did you test it?

Added some tests and changed existing ones

📝 Checklist

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 400.52 ms 417.83 ms 17.31 ms
Size 1.58 MiB 2.21 MiB 642.01 KiB

Previous results on branch: lcian/ref/urlutils

Startup times

Revision Plain With Sentry Diff
59da7c4 344.18 ms 425.04 ms 80.86 ms

App size

Revision Plain With Sentry Diff
59da7c4 1.58 MiB 2.21 MiB 641.66 KiB

Copy link
Member

@adinauer adinauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM

@lcian lcian enabled auto-merge (squash) March 6, 2025 12:36
@lcian lcian merged commit 44791e1 into main Mar 6, 2025
35 checks passed
@lcian lcian deleted the lcian/ref/urlutils branch March 6, 2025 12:48
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.

Correctly filter URL with an email address in it

3 participants