diff --git a/.craft.yml b/.craft.yml index b4944ec337..32dd9d5c7d 100644 --- a/.craft.yml +++ b/.craft.yml @@ -57,3 +57,5 @@ targets: maven:io.sentry:sentry-apollo-3: maven:io.sentry:sentry-android-sqlite: maven:io.sentry:sentry-android-replay: + maven:io.sentry:sentry-apollo-4: + maven:io.sentry:sentry-reactor: diff --git a/CHANGELOG.md b/CHANGELOG.md index b7376d0392..1b1508b4b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ - The SDK now automatically propagates the trace-context to the native layer. This allows to connect errors on different layers of the application. ([#4137](https://github.com/getsentry/sentry-java/pull/4137)) +### Behavioural Changes + +- Use `java.net.URI` for parsing URLs in `UrlUtils` ([#4210](https://github.com/getsentry/sentry-java/pull/4210)) + - This could affect grouping for issues with messages containing URLs that fall in known corner cases that were handled incorrectly previously (e.g. email in URL path) + ### Dependencies - Bump Native SDK from v0.7.20 to v0.8.1 ([#4137](https://github.com/getsentry/sentry-java/pull/4137)) diff --git a/README.md b/README.md index 50c281cdfa..f2fed34a82 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ Sentry SDK for Java and Android | sentry-jdbc | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-jdbc/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-jdbc) | | sentry-apollo | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo) | 21 | | sentry-apollo-3 | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo-3/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo-3) | 21 | +| sentry-apollo-4 | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo-4/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-apollo-4) | 21 | | sentry-kotlin-extensions | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-kotlin-extensions/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-kotlin-extensions) | 21 | | sentry-servlet | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-servlet/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-servlet) | | | sentry-servlet-jakarta | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-servlet-jakarta/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-servlet-jakarta) | | @@ -56,6 +57,7 @@ Sentry SDK for Java and Android | sentry-opentelemetry-agentcustomization | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-opentelemetry-agentcustomization/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-opentelemetry-agentcustomization) | | sentry-opentelemetry-core | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-opentelemetry-core/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-opentelemetry-core) | | sentry-okhttp | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-okhttp/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-okhttp) | +| sentry-reactor | [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-reactor/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.sentry/sentry-reactor) | # Releases diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index dc36ba678c..6c70cea049 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -3,10 +3,7 @@ import io.sentry.ISpan; import io.sentry.SpanDataConvention; import io.sentry.protocol.Request; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import java.net.URI; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -15,123 +12,55 @@ public final class UrlUtils { public static final @NotNull String SENSITIVE_DATA_SUBSTITUTE = "[Filtered]"; - private static final @NotNull Pattern AUTH_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); public static @Nullable UrlDetails parseNullable(final @Nullable String url) { - if (url == null) { - return null; - } - - return parse(url); + return url == null ? null : parse(url); } public static @NotNull UrlDetails parse(final @NotNull String url) { - if (isAbsoluteUrl(url)) { - return splitAbsoluteUrl(url); - } else { - return splitRelativeUrl(url); - } - } - - private static boolean isAbsoluteUrl(@NotNull String url) { - return url.contains("://"); - } - - private static @NotNull UrlDetails splitRelativeUrl(final @NotNull String url) { - final int queryParamSeparatorIndex = url.indexOf("?"); - final int fragmentSeparatorIndex = url.indexOf("#"); - - final @Nullable String baseUrl = - extractBaseUrl(url, queryParamSeparatorIndex, fragmentSeparatorIndex); - final @Nullable String query = - extractQuery(url, queryParamSeparatorIndex, fragmentSeparatorIndex); - final @Nullable String fragment = extractFragment(url, fragmentSeparatorIndex); + try { + URI uri = new URI(url); + if (uri.isAbsolute() && !isValidAbsoluteUrl(uri)) { + return new UrlDetails(null, null, null); + } - return new UrlDetails(baseUrl, query, fragment); - } + final @NotNull String schemeAndSeparator = + uri.getScheme() == null ? "" : (uri.getScheme() + "://"); + final @NotNull String authority = uri.getRawAuthority() == null ? "" : uri.getRawAuthority(); + final @NotNull String path = uri.getRawPath() == null ? "" : uri.getRawPath(); + final @Nullable String query = uri.getRawQuery(); + final @Nullable String fragment = uri.getRawFragment(); - private static @Nullable String extractBaseUrl( - final @NotNull String url, - final int queryParamSeparatorIndex, - final int fragmentSeparatorIndex) { - if (queryParamSeparatorIndex >= 0) { - return url.substring(0, queryParamSeparatorIndex).trim(); - } else if (fragmentSeparatorIndex >= 0) { - return url.substring(0, fragmentSeparatorIndex).trim(); - } else { - return url; - } - } + final @NotNull String filteredUrl = schemeAndSeparator + filterUserInfo(authority) + path; - private static @Nullable String extractQuery( - final @NotNull String url, - final int queryParamSeparatorIndex, - final int fragmentSeparatorIndex) { - if (queryParamSeparatorIndex > 0) { - if (fragmentSeparatorIndex > 0 && fragmentSeparatorIndex > queryParamSeparatorIndex) { - return url.substring(queryParamSeparatorIndex + 1, fragmentSeparatorIndex).trim(); - } else { - return url.substring(queryParamSeparatorIndex + 1).trim(); - } - } else { - return null; - } - } - - private static @Nullable String extractFragment( - final @NotNull String url, final int fragmentSeparatorIndex) { - if (fragmentSeparatorIndex > 0) { - return url.substring(fragmentSeparatorIndex + 1).trim(); - } else { - return null; + return new UrlDetails(filteredUrl, query, fragment); + } catch (Exception e) { + return new UrlDetails(null, null, null); } } - private static @NotNull UrlDetails splitAbsoluteUrl(final @NotNull String url) { + private static boolean isValidAbsoluteUrl(final @NotNull URI uri) { try { - final @NotNull String filteredUrl = urlWithAuthRemoved(url); - final @NotNull URL urlObj = new URL(url); - final @NotNull String baseUrl = baseUrlOnly(filteredUrl); - if (baseUrl.contains("#")) { - // url considered malformed because it has fragment - return new UrlDetails(null, null, null); - } else { - final @Nullable String query = urlObj.getQuery(); - final @Nullable String fragment = urlObj.getRef(); - return new UrlDetails(baseUrl, query, fragment); - } - } catch (MalformedURLException e) { - return new UrlDetails(null, null, null); + uri.toURL(); + } catch (Exception e) { + return false; } + return true; } - private static @NotNull String urlWithAuthRemoved(final @NotNull String url) { - final @NotNull Matcher userInfoMatcher = AUTH_REGEX.matcher(url); - if (userInfoMatcher.matches() && userInfoMatcher.groupCount() == 3) { - final @NotNull String userInfoString = userInfoMatcher.group(2); - final @NotNull String replacementString = - userInfoString.contains(":") - ? (SENSITIVE_DATA_SUBSTITUTE + ":" + SENSITIVE_DATA_SUBSTITUTE + "@") - : (SENSITIVE_DATA_SUBSTITUTE + "@"); - return userInfoMatcher.group(1) + replacementString + userInfoMatcher.group(3); - } else { + private static @NotNull String filterUserInfo(final @NotNull String url) { + if (!url.contains("@")) { return url; } - } - - private static @NotNull String baseUrlOnly(final @NotNull String url) { - final int queryParamSeparatorIndex = url.indexOf("?"); - - if (queryParamSeparatorIndex >= 0) { - return url.substring(0, queryParamSeparatorIndex).trim(); - } else { - final int fragmentSeparatorIndex = url.indexOf("#"); - if (fragmentSeparatorIndex >= 0) { - return url.substring(0, fragmentSeparatorIndex).trim(); - } else { - return url; - } + if (url.startsWith("@")) { + return SENSITIVE_DATA_SUBSTITUTE + url; } + final @NotNull String userInfo = url.substring(0, url.indexOf('@')); + final @NotNull String filteredUserInfo = + userInfo.contains(":") + ? (SENSITIVE_DATA_SUBSTITUTE + ":" + SENSITIVE_DATA_SUBSTITUTE) + : SENSITIVE_DATA_SUBSTITUTE; + return filteredUserInfo + url.substring(url.indexOf('@')); } public static final class UrlDetails { diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index af037b3344..ea40712654 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -142,7 +142,7 @@ class UrlUtilsTest { } @Test - fun `splits url without query or fragment and no authority`() { + fun `splits url without query or fragment and no user info`() { val urlDetails = UrlUtils.parse( "https://sentry.io" ) @@ -161,20 +161,41 @@ class UrlUtilsTest { assertEquals("top", urlDetails.fragment) } + // Fragment is allowed to contain '?' according to RFC 3986 @Test - fun `no details extracted with query after fragment`() { + fun `extracts details with question mark after fragment`() { val urlDetails = UrlUtils.parse( "https://user:password@sentry.io#fragment?q=1&s=2&token=secret" ) + assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `extracts details with question mark after fragment without user info`() { + val urlDetails = UrlUtils.parse( + "https://sentry.io#fragment?q=1&s=2&token=secret" + ) + assertEquals("https://sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `no details extracted from malformed url due to invalid protocol`() { + val urlDetails = UrlUtils.parse( + "htps://user@sentry.io#fragment?q=1&s=2&token=secret" + ) assertNull(urlDetails.url) assertNull(urlDetails.query) assertNull(urlDetails.fragment) } @Test - fun `no details extracted with query after fragment without authority`() { + fun `no details extracted from malformed url due to # symbol in fragment`() { val urlDetails = UrlUtils.parse( - "https://sentry.io#fragment?q=1&s=2&token=secret" + "https://example.com#hello#fragment" ) assertNull(urlDetails.url) assertNull(urlDetails.query) @@ -182,9 +203,137 @@ class UrlUtilsTest { } @Test - fun `no details extracted from malformed url`() { + fun `strips empty user info`() { val urlDetails = UrlUtils.parse( - "htps://user@sentry.io#fragment?q=1&s=2&token=secret" + "https://@sentry.io?query=a#fragment?q=1&s=2&token=secret" + ) + assertEquals("https://[Filtered]@sentry.io", urlDetails.url) + assertEquals("query=a", urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `extracts details from relative url with leading @ symbol`() { + val urlDetails = UrlUtils.parse( + "@@sentry.io/pages/10?query=a#fragment?q=1&s=2&token=secret" + ) + assertEquals("@@sentry.io/pages/10", urlDetails.url) + assertEquals("query=a", urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `extracts details from relative url with leading question mark`() { + val urlDetails = UrlUtils.parse( + "?query=a#fragment?q=1&s=2&token=secret" + ) + assertEquals("", urlDetails.url) + assertEquals("query=a", urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `does not filter email address in path`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com/api/v4/auth/password/reset/email@example.com" + )!! + assertEquals("https://staging.server.com/api/v4/auth/password/reset/email@example.com", urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) + } + + @Test + fun `does not filter email address in path with fragment`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com/api/v4/auth/password/reset/email@example.com#top" + )!! + assertEquals("https://staging.server.com/api/v4/auth/password/reset/email@example.com", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + + @Test + fun `does not filter email address in path with query and fragment`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com/api/v4/auth/password/reset/email@example.com?a=b&c=d#top" + )!! + assertEquals("https://staging.server.com/api/v4/auth/password/reset/email@example.com", urlDetails.url) + assertEquals("a=b&c=d", urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + + @Test + fun `does not filter email address in query`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com/?email=someone@example.com" + )!! + assertEquals("https://staging.server.com/", urlDetails.url) + assertEquals("email=someone@example.com", urlDetails.query) + } + + @Test + fun `does not filter email address in fragment`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com#email=someone@example.com" + )!! + assertEquals("https://staging.server.com", urlDetails.url) + assertEquals("email=someone@example.com", urlDetails.fragment) + } + + @Test + fun `does not filter email address in fragment with query`() { + val urlDetails = UrlUtils.parseNullable( + "https://staging.server.com?q=a&b=c#email=someone@example.com" + )!! + assertEquals("https://staging.server.com", urlDetails.url) + assertEquals("q=a&b=c", urlDetails.query) + assertEquals("email=someone@example.com", urlDetails.fragment) + } + + @Test + fun `extracts details from relative url with email in path`() { + val urlDetails = UrlUtils.parse( + "/emails/user@sentry.io?query=a&b=c#fragment?q=1&s=2&token=secret" + ) + assertEquals("/emails/user@sentry.io", urlDetails.url) + assertEquals("query=a&b=c", urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `extracts details from relative url with email in query`() { + val urlDetails = UrlUtils.parse( + "users/10?email=user@sentry.io&b=c#fragment?q=1&s=2&token=secret" + ) + assertEquals("users/10", urlDetails.url) + assertEquals("email=user@sentry.io&b=c", urlDetails.query) + assertEquals("fragment?q=1&s=2&token=secret", urlDetails.fragment) + } + + @Test + fun `extracts details from relative url with email in fragment`() { + val urlDetails = UrlUtils.parse( + "users/10?email=user@sentry.io&b=c#fragment?q=1&s=2&email=user@sentry.io" + ) + assertEquals("users/10", urlDetails.url) + assertEquals("email=user@sentry.io&b=c", urlDetails.query) + assertEquals("fragment?q=1&s=2&email=user@sentry.io", urlDetails.fragment) + } + + @Test + fun `extracts path from file url`() { + val urlDetails = UrlUtils.parse( + "file:///users/sentry/text.txt" + ) + assertEquals("file:///users/sentry/text.txt", urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) + } + + @Test + fun `does not extract details from websockets uri`() { + val urlDetails = UrlUtils.parse( + "wss://example.com/socket" ) assertNull(urlDetails.url) assertNull(urlDetails.query)