From 9d003d758b89c8246bcec3598de37687f67c29dc Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 16 Nov 2022 08:32:49 +0100 Subject: [PATCH 01/10] Use split instead of regex to prevent infinite loops in case of weird URLs --- .../okhttp/SentryOkHttpInterceptorTest.kt | 4 ++ .../main/java/io/sentry/util/UrlUtils.java | 40 +++++++++++++++---- .../test/java/io/sentry/util/UrlUtilsTest.kt | 6 +-- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 9429a2112f..10a09163d5 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -263,6 +263,8 @@ class SentryOkHttpInterceptorTest { @SuppressWarnings("SwallowedException") @Test fun `adds breadcrumb when http calls results in exception`() { + // to setup mocks + fixture.getSut() val interceptor = SentryOkHttpInterceptor(fixture.hub) val chain = mock() whenever(chain.proceed(any())).thenThrow(IOException()) @@ -489,6 +491,8 @@ class SentryOkHttpInterceptorTest { @SuppressWarnings("SwallowedException") @Test fun `does not capture an error even if it throws`() { + // to setup mocks + fixture.getSut() val interceptor = SentryOkHttpInterceptor( fixture.hub, captureFailedRequests = true diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index b38099d57c..d84bc2e3c8 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -8,7 +8,6 @@ public final class UrlUtils { private static final @NotNull Pattern USER_INFO_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); - private static final @NotNull Pattern TOKEN_REGEX = Pattern.compile("(.*)([?&]token=[^&#]+)(.*)"); public static @Nullable String maybeStripSensitiveDataFromUrlNullable( final @Nullable String url, final boolean isSendDefaultPii) { @@ -27,19 +26,44 @@ public final class UrlUtils { @NotNull String modifiedUrl = url; - Matcher userInfoMatcher = USER_INFO_REGEX.matcher(modifiedUrl); + final @NotNull Matcher userInfoMatcher = USER_INFO_REGEX.matcher(modifiedUrl); if (userInfoMatcher.matches() && userInfoMatcher.groupCount() == 3) { final @NotNull String userInfoString = userInfoMatcher.group(2); final @NotNull String replacementString = userInfoString.contains(":") ? "%s:%s@" : "%s@"; modifiedUrl = userInfoMatcher.group(1) + replacementString + userInfoMatcher.group(3); } - Matcher tokenMatcher = TOKEN_REGEX.matcher(modifiedUrl); - if (tokenMatcher.matches() && tokenMatcher.groupCount() == 3) { - final @NotNull String tokenString = tokenMatcher.group(2); - final @NotNull String queryParamSeparator = tokenString.substring(0, 1); - modifiedUrl = - tokenMatcher.group(1) + queryParamSeparator + "token=%s" + tokenMatcher.group(3); + final int queryParamSeparatorIndex = modifiedUrl.indexOf("?"); + if (queryParamSeparatorIndex >= 0) { + final @NotNull String urlWithoutQuery = + modifiedUrl.substring(0, queryParamSeparatorIndex).trim(); + final @NotNull StringBuilder urlBuilder = new StringBuilder(urlWithoutQuery); + @NotNull String query = modifiedUrl.substring(queryParamSeparatorIndex).trim(); + @NotNull String anchorPart = ""; + + final int anchorInQueryIndex = query.indexOf("#"); + if (anchorInQueryIndex >= 0) { + anchorPart = query.substring(anchorInQueryIndex); + query = query.substring(0, anchorInQueryIndex); + } + + final @NotNull String[] queryParams = query.split("&", -1); + @NotNull String separator = ""; + for (final @NotNull String queryParam : queryParams) { + final @NotNull String[] queryParamParts = queryParam.split("=", -1); + urlBuilder.append(separator); + if (queryParamParts.length == 2) { + urlBuilder.append(queryParamParts[0]); + urlBuilder.append("=%s"); + } else { + urlBuilder.append(queryParam); + } + separator = "&"; + } + + urlBuilder.append(anchorPart); + + modifiedUrl = urlBuilder.toString(); } return modifiedUrl; diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index cca8523782..bc8869ad0d 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -43,7 +43,7 @@ class UrlUtilsTest { @Test fun `strips token from query params as later param`() { - assertEquals("https://sentry.io?q=1&s=2&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret", false)) + assertEquals("https://sentry.io?q=%s&s=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret", false)) } @Test @@ -53,7 +53,7 @@ class UrlUtilsTest { @Test fun `strips token from query params as later param and keeps anchor`() { - assertEquals("https://sentry.io?q=1&s=2&token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret#top", false)) + assertEquals("https://sentry.io?q=%s&s=%s&token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret#top", false)) } @Test @@ -63,6 +63,6 @@ class UrlUtilsTest { @Test fun `strips token from query params after anchor with &`() { - assertEquals("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=query", false)) + assertEquals("https://api.github.com/users/getsentry/repos/#fragment?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=query", false)) } } From 96173adf77525481d952b2d225bff8e049516ed3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 16 Nov 2022 08:42:59 +0100 Subject: [PATCH 02/10] Add changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78d682c40d..8342b19493 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,12 @@ # Changelog -## 6.7.0 +## Unreleased ### Fixes - Fix `Gpu.vendorId` should be a String ([#2343](https://github.com/getsentry/sentry-java/pull/2343)) - Don't set device name on Android if `sendDefaultPii` is disabled ([#2354](https://github.com/getsentry/sentry-java/pull/2354)) +- Remove sensitive data from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) ### Features From b9e68d69458ff0f2c131ed6e2e19b2a50fdef6e4 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 16 Nov 2022 11:21:25 +0100 Subject: [PATCH 03/10] Pass SentryOptions into UrlUtils instead of just isSendDefaultPii; filter request query params for okhttp --- .../android/okhttp/SentryOkHttpInterceptor.kt | 8 +-- .../okhttp/SentryOkHttpInterceptorTest.kt | 44 +++++++++++- .../apollo3/SentryApollo3HttpInterceptor.kt | 4 +- .../sentry/apollo/SentryApolloInterceptor.kt | 2 +- .../sentry/openfeign/SentryFeignClient.java | 5 +- ...entrySpanClientHttpRequestInterceptor.java | 4 +- ...entrySpanClientHttpRequestInterceptor.java | 6 +- sentry/api/sentry.api | 5 +- .../main/java/io/sentry/util/UrlUtils.java | 70 +++++++++++-------- .../test/java/io/sentry/util/UrlUtilsTest.kt | 35 ++++++---- 10 files changed, 122 insertions(+), 61 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 504d2d1a8d..5f84dad8a9 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -54,7 +54,7 @@ class SentryOkHttpInterceptor( override fun intercept(chain: Interceptor.Chain): Response { var request = chain.request() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options.isSendDefaultPii) + val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options) val method = request.method // read transaction from the bound scope @@ -97,7 +97,7 @@ class SentryOkHttpInterceptor( } finally { finishSpan(span, request, response) - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options.isSendDefaultPii) + val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options) val breadcrumb = Breadcrumb.http(url, request.method, code) request.body?.contentLength().ifHasValidLength { breadcrumb.setData("request_body_size", it) @@ -182,11 +182,11 @@ class SentryOkHttpInterceptor( hint.set(OKHTTP_RESPONSE, response) val sentryRequest = io.sentry.protocol.Request().apply { - url = UrlUtils.maybeStripSensitiveDataFromUrl(requestUrl, hub.options.isSendDefaultPii) + url = UrlUtils.maybeStripSensitiveDataFromUrl(requestUrl, hub.options) // Cookie is only sent if isSendDefaultPii is enabled cookies = if (hub.options.isSendDefaultPii) request.headers["Cookie"] else null method = request.method - queryString = query + queryString = query?.let { UrlUtils.maybeStripSensitiveDataFromQuery(it, hub.options) } headers = getHeaders(request.headers) fragment = urlFragment diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 10a09163d5..93f204c35d 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -410,7 +410,8 @@ class SentryOkHttpInterceptorTest { val sut = fixture.getSut( captureFailedRequests = true, httpStatusCode = statusCode, - responseBody = "fail" + responseBody = "fail", + sendDefaultPii = true ) val request = getRequest(url = "/hello?myQuery=myValue#myFragment") @@ -425,17 +426,56 @@ class SentryOkHttpInterceptorTest { assertEquals("GET", sentryRequest.method) // because of isSendDefaultPii - assertNull(sentryRequest.headers) + assertNotNull(sentryRequest.headers) assertNull(sentryRequest.cookies) val sentryResponse = it.contexts.response!! assertEquals(statusCode, sentryResponse.statusCode) assertEquals(response.body!!.contentLength(), sentryResponse.bodySize) + // because of isSendDefaultPii + assertNotNull(sentryResponse.headers) + assertNull(sentryResponse.cookies) + + assertTrue(it.throwable is SentryHttpClientException) + }, + any() + ) + } + + @Test + fun `stips sensitive information if not sendDefaultPii`() { + val statusCode = 500 + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = statusCode, + responseBody = "fail", + sendDefaultPii = false + ) + + val request = getRequest(url = "/hello?myQuery=myValue#myFragment") + val response = sut.newCall(request).execute() + + verify(fixture.hub).captureEvent( + check { + val sentryRequest = it.request!! + assertEquals("http://localhost:${fixture.server.port}/hello", sentryRequest.url) + assertEquals("myQuery=%s", sentryRequest.queryString) + assertEquals("myFragment", sentryRequest.fragment) + assertEquals("GET", sentryRequest.method) + // because of isSendDefaultPii assertNull(sentryRequest.headers) assertNull(sentryRequest.cookies) + val sentryResponse = it.contexts.response!! + assertEquals(statusCode, sentryResponse.statusCode) + assertEquals(response.body!!.contentLength(), sentryResponse.bodySize) + + // because of isSendDefaultPii + assertNull(sentryResponse.headers) + assertNull(sentryResponse.cookies) + assertTrue(it.throwable is SentryHttpClientException) }, any() diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 9dff5ec85c..1c72e67acf 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -81,7 +81,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH } private fun startChild(request: HttpRequest, activeSpan: ISpan): ISpan { - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options.isSendDefaultPii) + val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options) val method = request.method val operationName = operationNameFromHeaders(request) @@ -122,7 +122,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH } span.finish() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options.isSendDefaultPii) + val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options) val breadcrumb = Breadcrumb.http(url, request.method.name, statusCode) diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index 48316d0a67..018ad7f585 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -115,7 +115,7 @@ class SentryApolloInterceptor( val httpResponse = it.httpResponse.get() val httpRequest = httpResponse.request() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(httpRequest.url().toString(), hub.options.isSendDefaultPii) + val url = UrlUtils.maybeStripSensitiveDataFromUrl(httpRequest.url().toString(), hub.options) val breadcrumb = Breadcrumb.http(url, httpRequest.method(), httpResponse.code()) httpRequest.body()?.contentLength().ifHasValidLength { contentLength -> diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index c7cd5f4f15..cef1386a34 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -52,8 +52,7 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O ISpan span = activeSpan.startChild("http.client"); final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl( - request.url(), hub.getOptions().isSendDefaultPii()); + UrlUtils.maybeStripSensitiveDataFromUrl(request.url(), hub.getOptions()); span.setDescription(request.httpMethod().name() + " " + url); final RequestWrapper requestWrapper = new RequestWrapper(request); @@ -103,7 +102,7 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O private void addBreadcrumb(final @NotNull Request request, final @Nullable Response response) { final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl(request.url(), hub.getOptions().isSendDefaultPii()); + UrlUtils.maybeStripSensitiveDataFromUrl(request.url(), hub.getOptions()); final Breadcrumb breadcrumb = Breadcrumb.http( url, request.httpMethod().name(), response != null ? response.status() : null); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index f49f79507e..36d1c38f28 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -49,7 +49,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final ISpan span = activeSpan.startChild("http.client"); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions().isSendDefaultPii()); + final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); span.setDescription(methodName + " " + url); final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); @@ -90,7 +90,7 @@ private void addBreadcrumb( final @Nullable Integer responseStatusCode, final @Nullable ClientHttpResponse response) { final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions().isSendDefaultPii()); + final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); final Breadcrumb breadcrumb = Breadcrumb.http(url, methodName, responseStatusCode); diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index d0c7182d73..a61881926b 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -49,8 +49,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl( - request.getURI().toString(), hub.getOptions().isSendDefaultPii()); + UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); span.setDescription(methodName + " " + url); final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); @@ -93,8 +92,7 @@ private void addBreadcrumb( final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl( - request.getURI().toString(), hub.getOptions().isSendDefaultPii()); + UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); final Breadcrumb breadcrumb = Breadcrumb.http(url, methodName, responseStatusCode); breadcrumb.setData("request_body_size", body.length); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 66ec32d5a7..85d44a09e8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3381,8 +3381,9 @@ public final class io/sentry/util/StringUtils { public final class io/sentry/util/UrlUtils { public fun ()V - public static fun maybeStripSensitiveDataFromUrl (Ljava/lang/String;Z)Ljava/lang/String; - public static fun maybeStripSensitiveDataFromUrlNullable (Ljava/lang/String;Z)Ljava/lang/String; + public static fun maybeStripSensitiveDataFromQuery (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; + public static fun maybeStripSensitiveDataFromUrl (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; + public static fun maybeStripSensitiveDataFromUrlNullable (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; } public class io/sentry/vendor/Base64 { diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index d84bc2e3c8..294fbd2443 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -1,5 +1,6 @@ package io.sentry.util; +import io.sentry.SentryOptions; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.jetbrains.annotations.NotNull; @@ -10,17 +11,17 @@ public final class UrlUtils { private static final @NotNull Pattern USER_INFO_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); public static @Nullable String maybeStripSensitiveDataFromUrlNullable( - final @Nullable String url, final boolean isSendDefaultPii) { + final @Nullable String url, final @NotNull SentryOptions options) { if (url == null) { return null; } - return maybeStripSensitiveDataFromUrl(url, isSendDefaultPii); + return maybeStripSensitiveDataFromUrl(url, options); } public static @NotNull String maybeStripSensitiveDataFromUrl( - final @NotNull String url, final boolean isSendDefaultPii) { - if (isSendDefaultPii) { + final @NotNull String url, final @NotNull SentryOptions options) { + if (options.isSendDefaultPii()) { return url; } @@ -37,35 +38,46 @@ public final class UrlUtils { if (queryParamSeparatorIndex >= 0) { final @NotNull String urlWithoutQuery = modifiedUrl.substring(0, queryParamSeparatorIndex).trim(); - final @NotNull StringBuilder urlBuilder = new StringBuilder(urlWithoutQuery); - @NotNull String query = modifiedUrl.substring(queryParamSeparatorIndex).trim(); - @NotNull String anchorPart = ""; - - final int anchorInQueryIndex = query.indexOf("#"); - if (anchorInQueryIndex >= 0) { - anchorPart = query.substring(anchorInQueryIndex); - query = query.substring(0, anchorInQueryIndex); - } + final @NotNull String query = modifiedUrl.substring(queryParamSeparatorIndex).trim(); - final @NotNull String[] queryParams = query.split("&", -1); - @NotNull String separator = ""; - for (final @NotNull String queryParam : queryParams) { - final @NotNull String[] queryParamParts = queryParam.split("=", -1); - urlBuilder.append(separator); - if (queryParamParts.length == 2) { - urlBuilder.append(queryParamParts[0]); - urlBuilder.append("=%s"); - } else { - urlBuilder.append(queryParam); - } - separator = "&"; - } + modifiedUrl = urlWithoutQuery + maybeStripSensitiveDataFromQuery(query, options); + } - urlBuilder.append(anchorPart); + return modifiedUrl; + } - modifiedUrl = urlBuilder.toString(); + public static @NotNull String maybeStripSensitiveDataFromQuery( + @NotNull String query, final @NotNull SentryOptions options) { + if (options.isSendDefaultPii()) { + return query; } - return modifiedUrl; + final @NotNull StringBuilder queryBuilder = new StringBuilder(); + @NotNull String modifiedQuery = query; + @NotNull String anchorPart = ""; + + final int anchorInQueryIndex = modifiedQuery.indexOf("#"); + if (anchorInQueryIndex >= 0) { + anchorPart = query.substring(anchorInQueryIndex); + modifiedQuery = query.substring(0, anchorInQueryIndex); + } + + final @NotNull String[] queryParams = modifiedQuery.split("&", -1); + @NotNull String separator = ""; + for (final @NotNull String queryParam : queryParams) { + final @NotNull String[] queryParamParts = queryParam.split("=", -1); + queryBuilder.append(separator); + if (queryParamParts.length == 2) { + queryBuilder.append(queryParamParts[0]); + queryBuilder.append("=%s"); + } else { + queryBuilder.append(queryParam); + } + separator = "&"; + } + + queryBuilder.append(anchorPart); + + return queryBuilder.toString(); } } diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index bc8869ad0d..cc4936d9d9 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -1,5 +1,6 @@ package io.sentry.util +import io.sentry.SentryOptions import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -8,61 +9,71 @@ class UrlUtilsTest { @Test fun `returns null for null`() { - assertNull(UrlUtils.maybeStripSensitiveDataFromUrlNullable(null, false)) + assertNull(UrlUtils.maybeStripSensitiveDataFromUrlNullable(null, SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `keeps sensitive data if sendDefaultPii is true`() { - assertEquals("https://user:password@sentry.io?q=1&s=2&token=secret#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io?q=1&s=2&token=secret#top", true)) + assertEquals("https://user:password@sentry.io?q=1&s=2&token=secret#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io?q=1&s=2&token=secret#top", SentryOptions().also { it.isSendDefaultPii = true })) } @Test fun `strips user info with user and password`() { - assertEquals("http://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user:password@sentry.io", false)) + assertEquals("http://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user:password@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips user info with user and password from https`() { - assertEquals("https://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io", false)) + assertEquals("https://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips user info with user only`() { - assertEquals("http://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user@sentry.io", false)) + assertEquals("http://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips user info with user only from https`() { - assertEquals("https://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user@sentry.io", false)) + assertEquals("https://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params as first param`() { - assertEquals("https://sentry.io?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret", false)) + assertEquals("https://sentry.io?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params as later param`() { - assertEquals("https://sentry.io?q=%s&s=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret", false)) + assertEquals("https://sentry.io?q=%s&s=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params as first param and keeps anchor`() { - assertEquals("https://sentry.io?token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret#top", false)) + assertEquals("https://sentry.io?token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret#top", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params as later param and keeps anchor`() { - assertEquals("https://sentry.io?q=%s&s=%s&token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret#top", false)) + assertEquals("https://sentry.io?q=%s&s=%s&token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret#top", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params after anchor`() { - assertEquals("https://api.github.com/users/getsentry/repos/#fragment?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?token=query", false)) + assertEquals("https://api.github.com/users/getsentry/repos/#fragment?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?token=query", SentryOptions().also { it.isSendDefaultPii = false })) } @Test fun `strips token from query params after anchor with &`() { - assertEquals("https://api.github.com/users/getsentry/repos/#fragment?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=query", false)) + assertEquals("https://api.github.com/users/getsentry/repos/#fragment?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=query", SentryOptions().also { it.isSendDefaultPii = false })) + } + + @Test + fun `strips query params`() { + assertEquals("?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromQuery("?q=%s&token=query", SentryOptions().also { it.isSendDefaultPii = false })) + } + + @Test + fun `strips query params without ?`() { + assertEquals("q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromQuery("q=%s&token=query", SentryOptions().also { it.isSendDefaultPii = false })) } } From 4ff109629fe0a6a98d7575d912c30a6b6019049f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 16 Jan 2023 06:53:40 +0100 Subject: [PATCH 04/10] Update according to dev docs --- .../android/okhttp/SentryOkHttpInterceptor.kt | 25 +--- .../okhttp/SentryOkHttpInterceptorTest.kt | 39 ------ .../apollo3/SentryApollo3HttpInterceptor.kt | 10 +- .../sentry/apollo/SentryApolloInterceptor.kt | 4 +- .../sentry/openfeign/SentryFeignClient.java | 15 +- ...entrySpanClientHttpRequestInterceptor.java | 8 +- ...entrySpanClientHttpRequestInterceptor.java | 12 +- sentry/api/sentry.api | 15 +- .../src/main/java/io/sentry/Breadcrumb.java | 12 +- .../main/java/io/sentry/util/UrlUtils.java | 131 ++++++++++++------ .../test/java/io/sentry/util/UrlUtilsTest.kt | 114 +++++++++++---- 11 files changed, 224 insertions(+), 161 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 5f84dad8a9..306341b77f 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -54,11 +54,13 @@ class SentryOkHttpInterceptor( override fun intercept(chain: Interceptor.Chain): Response { var request = chain.request() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options) + val urlDetails = UrlUtils.convertUrl(request.url.toString()) + val url = urlDetails.urlOrFallback val method = request.method // read transaction from the bound scope val span = hub.span?.startChild("http.client", "$method $url") + urlDetails.applyToSpan(span) var response: Response? = null @@ -97,8 +99,7 @@ class SentryOkHttpInterceptor( } finally { finishSpan(span, request, response) - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url.toString(), hub.options) - val breadcrumb = Breadcrumb.http(url, request.method, code) + val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) request.body?.contentLength().ifHasValidLength { breadcrumb.setData("request_body_size", it) } @@ -151,20 +152,10 @@ class SentryOkHttpInterceptor( // url will be: https://api.github.com/users/getsentry/repos/ // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ // but that's not possible - var requestUrl = request.url.toString() - - val query = request.url.query - if (!query.isNullOrEmpty()) { - requestUrl = requestUrl.replace("?$query", "") - } - - val urlFragment = request.url.fragment - if (!urlFragment.isNullOrEmpty()) { - requestUrl = requestUrl.replace("#$urlFragment", "") - } + val urlDetails = UrlUtils.convertUrl(request.url.toString()) // return if its not a target match - if (!PropagationTargetsUtils.contain(failedRequestTargets, requestUrl)) { + if (!PropagationTargetsUtils.contain(failedRequestTargets, request.url.toString())) { return } @@ -182,13 +173,11 @@ class SentryOkHttpInterceptor( hint.set(OKHTTP_RESPONSE, response) val sentryRequest = io.sentry.protocol.Request().apply { - url = UrlUtils.maybeStripSensitiveDataFromUrl(requestUrl, hub.options) + urlDetails.applyToRequest(this) // Cookie is only sent if isSendDefaultPii is enabled cookies = if (hub.options.isSendDefaultPii) request.headers["Cookie"] else null method = request.method - queryString = query?.let { UrlUtils.maybeStripSensitiveDataFromQuery(it, hub.options) } headers = getHeaders(request.headers) - fragment = urlFragment request.body?.contentLength().ifHasValidLength { bodySize = it diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 93f204c35d..eed10ea8c6 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -443,45 +443,6 @@ class SentryOkHttpInterceptorTest { ) } - @Test - fun `stips sensitive information if not sendDefaultPii`() { - val statusCode = 500 - val sut = fixture.getSut( - captureFailedRequests = true, - httpStatusCode = statusCode, - responseBody = "fail", - sendDefaultPii = false - ) - - val request = getRequest(url = "/hello?myQuery=myValue#myFragment") - val response = sut.newCall(request).execute() - - verify(fixture.hub).captureEvent( - check { - val sentryRequest = it.request!! - assertEquals("http://localhost:${fixture.server.port}/hello", sentryRequest.url) - assertEquals("myQuery=%s", sentryRequest.queryString) - assertEquals("myFragment", sentryRequest.fragment) - assertEquals("GET", sentryRequest.method) - - // because of isSendDefaultPii - assertNull(sentryRequest.headers) - assertNull(sentryRequest.cookies) - - val sentryResponse = it.contexts.response!! - assertEquals(statusCode, sentryResponse.statusCode) - assertEquals(response.body!!.contentLength(), sentryResponse.bodySize) - - // because of isSendDefaultPii - assertNull(sentryResponse.headers) - assertNull(sentryResponse.cookies) - - assertTrue(it.throwable is SentryHttpClientException) - }, - any() - ) - } - @Test fun `captures an error event with request body size`() { val sut = fixture.getSut( diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 1c72e67acf..8b9d3b5808 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -81,7 +81,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH } private fun startChild(request: HttpRequest, activeSpan: ISpan): ISpan { - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options) + val urlDetails = UrlUtils.convertUrl(request.url) val method = request.method val operationName = operationNameFromHeaders(request) @@ -89,9 +89,11 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH val operationType = request.valueForHeader(SENTRY_APOLLO_3_OPERATION_TYPE) ?: method val operationId = request.valueForHeader("X-APOLLO-OPERATION-ID") val variables = request.valueForHeader(SENTRY_APOLLO_3_VARIABLES) - val description = "$operationType ${operationName ?: url}" + val description = "$operationType ${operationName ?: urlDetails.urlOrFallback}" return activeSpan.startChild(operation, description).apply { + urlDetails.applyToSpan(this) + operationId?.let { setData("operationId", it) } @@ -122,9 +124,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH } span.finish() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(request.url, hub.options) - val breadcrumb = - Breadcrumb.http(url, request.method.name, statusCode) + val breadcrumb = Breadcrumb.http(request.url, request.method.name, statusCode) request.body?.contentLength.ifHasValidLength { contentLength -> breadcrumb.setData("request_body_size", contentLength) diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index 018ad7f585..cd61d5367b 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -21,7 +21,6 @@ import io.sentry.SentryLevel import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE -import io.sentry.util.UrlUtils import java.util.concurrent.Executor class SentryApolloInterceptor( @@ -115,8 +114,7 @@ class SentryApolloInterceptor( val httpResponse = it.httpResponse.get() val httpRequest = httpResponse.request() - val url = UrlUtils.maybeStripSensitiveDataFromUrl(httpRequest.url().toString(), hub.options) - val breadcrumb = Breadcrumb.http(url, httpRequest.method(), httpResponse.code()) + val breadcrumb = Breadcrumb.http(httpRequest.url().toString(), httpRequest.method(), httpResponse.code()) httpRequest.body()?.contentLength().ifHasValidLength { contentLength -> breadcrumb.setData("request_body_size", contentLength) diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index cef1386a34..938280b6ff 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -51,13 +51,14 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O } ISpan span = activeSpan.startChild("http.client"); - final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl(request.url(), hub.getOptions()); - span.setDescription(request.httpMethod().name() + " " + url); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.url()); + span.setDescription(request.httpMethod().name() + " " + urlDetails.getUrlOrFallback()); + urlDetails.applyToSpan(span); final RequestWrapper requestWrapper = new RequestWrapper(request); - if (PropagationTargetsUtils.contain(hub.getOptions().getTracePropagationTargets(), url)) { + if (PropagationTargetsUtils.contain( + hub.getOptions().getTracePropagationTargets(), request.url())) { final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); final @Nullable Collection requestBaggageHeader = request.headers().get(BaggageHeader.BAGGAGE_HEADER); @@ -101,11 +102,11 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O } private void addBreadcrumb(final @NotNull Request request, final @Nullable Response response) { - final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl(request.url(), hub.getOptions()); final Breadcrumb breadcrumb = Breadcrumb.http( - url, request.httpMethod().name(), response != null ? response.status() : null); + request.url(), + request.httpMethod().name(), + response != null ? response.status() : null); breadcrumb.setData("request_body_size", request.body() != null ? request.body().length : 0); if (response != null && response.body() != null && response.body().length() != null) { breadcrumb.setData("response_body_size", response.body().length()); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index 36d1c38f28..8f5851a489 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -49,8 +49,9 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final ISpan span = activeSpan.startChild("http.client"); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); - span.setDescription(methodName + " " + url); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.getURI().toString()); + span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); + urlDetails.applyToSpan(span); final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); @@ -90,10 +91,9 @@ private void addBreadcrumb( final @Nullable Integer responseStatusCode, final @Nullable ClientHttpResponse response) { final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); final Breadcrumb breadcrumb = - Breadcrumb.http(url, methodName, responseStatusCode); + Breadcrumb.http(request.getURI().toString(), methodName, responseStatusCode); breadcrumb.setData("request_body_size", body.length); final Hint hint = new Hint(); diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index a61881926b..8aef63394c 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -48,9 +48,10 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final ISpan span = activeSpan.startChild("http.client"); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); - span.setDescription(methodName + " " + url); + final @NotNull UrlUtils.UrlDetails urlDetails = + UrlUtils.convertUrl(request.getURI().toString()); + urlDetails.applyToSpan(span); + span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); @@ -91,9 +92,8 @@ private void addBreadcrumb( final @Nullable ClientHttpResponse response) { final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull String url = - UrlUtils.maybeStripSensitiveDataFromUrl(request.getURI().toString(), hub.getOptions()); - final Breadcrumb breadcrumb = Breadcrumb.http(url, methodName, responseStatusCode); + final Breadcrumb breadcrumb = + Breadcrumb.http(request.getURI().toString(), methodName, responseStatusCode); breadcrumb.setData("request_body_size", body.length); final Hint hint = new Hint(); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 85d44a09e8..c7ebf187dc 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3381,9 +3381,18 @@ public final class io/sentry/util/StringUtils { public final class io/sentry/util/UrlUtils { public fun ()V - public static fun maybeStripSensitiveDataFromQuery (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; - public static fun maybeStripSensitiveDataFromUrl (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; - public static fun maybeStripSensitiveDataFromUrlNullable (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; + public static fun convertUrl (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; + public static fun convertUrlNullable (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; +} + +public final class io/sentry/util/UrlUtils$UrlDetails { + public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V + public fun applyToRequest (Lio/sentry/protocol/Request;)V + public fun applyToSpan (Lio/sentry/ISpan;)V + public fun getFragment ()Ljava/lang/String; + public fun getQuery ()Ljava/lang/String; + public fun getUrl ()Ljava/lang/String; + public fun getUrlOrFallback ()Ljava/lang/String; } public class io/sentry/vendor/Base64 { diff --git a/sentry/src/main/java/io/sentry/Breadcrumb.java b/sentry/src/main/java/io/sentry/Breadcrumb.java index 9d2231eeea..586ba38524 100644 --- a/sentry/src/main/java/io/sentry/Breadcrumb.java +++ b/sentry/src/main/java/io/sentry/Breadcrumb.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.util.CollectionUtils; +import io.sentry.util.UrlUtils; import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; import java.util.Collections; @@ -67,10 +68,19 @@ public Breadcrumb(final @NotNull Date timestamp) { */ public static @NotNull Breadcrumb http(final @NotNull String url, final @NotNull String method) { final Breadcrumb breadcrumb = new Breadcrumb(); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(url); breadcrumb.setType("http"); breadcrumb.setCategory("http"); - breadcrumb.setData("url", url); + if (urlDetails.getUrl() != null) { + breadcrumb.setData("url", urlDetails.getUrl()); + } breadcrumb.setData("method", method.toUpperCase(Locale.ROOT)); + if (urlDetails.getQuery() != null) { + breadcrumb.setData("http.query", urlDetails.getQuery()); + } + if (urlDetails.getFragment() != null) { + breadcrumb.setData("http.fragment", urlDetails.getFragment()); + } return breadcrumb; } diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index 294fbd2443..8a71b228d5 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -1,6 +1,9 @@ package io.sentry.util; -import io.sentry.SentryOptions; +import io.sentry.ISpan; +import io.sentry.protocol.Request; +import java.net.MalformedURLException; +import java.net.URL; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.jetbrains.annotations.NotNull; @@ -8,76 +11,114 @@ public final class UrlUtils { - private static final @NotNull Pattern USER_INFO_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); + private static final @NotNull Pattern AUTH_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); - public static @Nullable String maybeStripSensitiveDataFromUrlNullable( - final @Nullable String url, final @NotNull SentryOptions options) { + public static @Nullable UrlDetails convertUrlNullable(final @Nullable String url) { if (url == null) { return null; } - return maybeStripSensitiveDataFromUrl(url, options); + return convertUrl(url); } - public static @NotNull String maybeStripSensitiveDataFromUrl( - final @NotNull String url, final @NotNull SentryOptions options) { - if (options.isSendDefaultPii()) { - return url; + public static @NotNull UrlDetails convertUrl(final @NotNull String url) { + final @NotNull String filteredUrl = urlWithAuthRemoved(url); + try { + 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); } + } - @NotNull String modifiedUrl = url; - - final @NotNull Matcher userInfoMatcher = USER_INFO_REGEX.matcher(modifiedUrl); + 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(":") ? "%s:%s@" : "%s@"; - modifiedUrl = userInfoMatcher.group(1) + replacementString + userInfoMatcher.group(3); + final @NotNull String replacementString = + userInfoString.contains(":") ? "[Filtered]:[Filtered]@" : "[Filtered]@"; + return userInfoMatcher.group(1) + replacementString + userInfoMatcher.group(3); + } else { + return url; } + } + + private static @NotNull String baseUrlOnly(final @NotNull String url) { + final int queryParamSeparatorIndex = url.indexOf("?"); - final int queryParamSeparatorIndex = modifiedUrl.indexOf("?"); if (queryParamSeparatorIndex >= 0) { - final @NotNull String urlWithoutQuery = - modifiedUrl.substring(0, queryParamSeparatorIndex).trim(); - final @NotNull String query = modifiedUrl.substring(queryParamSeparatorIndex).trim(); + return url.substring(0, queryParamSeparatorIndex).trim(); + } else { + final int fragmentSeparatorIndex = url.indexOf("#"); + if (fragmentSeparatorIndex >= 0) { + return url.substring(0, fragmentSeparatorIndex).trim(); + } else { + return url; + } + } + } + + public static final class UrlDetails { + private final @Nullable String url; + private final @Nullable String query; + private final @Nullable String fragment; - modifiedUrl = urlWithoutQuery + maybeStripSensitiveDataFromQuery(query, options); + public UrlDetails( + final @Nullable String url, final @Nullable String query, final @Nullable String fragment) { + this.url = url; + this.query = query; + this.fragment = fragment; } - return modifiedUrl; - } + public @Nullable String getUrl() { + return url; + } - public static @NotNull String maybeStripSensitiveDataFromQuery( - @NotNull String query, final @NotNull SentryOptions options) { - if (options.isSendDefaultPii()) { - return query; + public @NotNull String getUrlOrFallback() { + if (url == null) { + return "unknown"; + } else { + return url; + } } - final @NotNull StringBuilder queryBuilder = new StringBuilder(); - @NotNull String modifiedQuery = query; - @NotNull String anchorPart = ""; + public @Nullable String getQuery() { + return query; + } - final int anchorInQueryIndex = modifiedQuery.indexOf("#"); - if (anchorInQueryIndex >= 0) { - anchorPart = query.substring(anchorInQueryIndex); - modifiedQuery = query.substring(0, anchorInQueryIndex); + public @Nullable String getFragment() { + return fragment; } - final @NotNull String[] queryParams = modifiedQuery.split("&", -1); - @NotNull String separator = ""; - for (final @NotNull String queryParam : queryParams) { - final @NotNull String[] queryParamParts = queryParam.split("=", -1); - queryBuilder.append(separator); - if (queryParamParts.length == 2) { - queryBuilder.append(queryParamParts[0]); - queryBuilder.append("=%s"); - } else { - queryBuilder.append(queryParam); + public void applyToRequest(final @Nullable Request request) { + if (request == null) { + return; } - separator = "&"; + + request.setUrl(url); + request.setQueryString(query); + request.setFragment(fragment); } - queryBuilder.append(anchorPart); + public void applyToSpan(final @Nullable ISpan span) { + if (span == null) { + return; + } - return queryBuilder.toString(); + if (query != null) { + span.setData("http.query", query); + } + if (fragment != null) { + span.setData("http.fragment", fragment); + } + } } } diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index cc4936d9d9..adacf0d623 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -1,6 +1,5 @@ package io.sentry.util -import io.sentry.SentryOptions import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -9,71 +8,126 @@ class UrlUtilsTest { @Test fun `returns null for null`() { - assertNull(UrlUtils.maybeStripSensitiveDataFromUrlNullable(null, SentryOptions().also { it.isSendDefaultPii = false })) + assertNull(UrlUtils.convertUrlNullable(null)) } @Test - fun `keeps sensitive data if sendDefaultPii is true`() { - assertEquals("https://user:password@sentry.io?q=1&s=2&token=secret#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io?q=1&s=2&token=secret#top", SentryOptions().also { it.isSendDefaultPii = true })) - } - - @Test - fun `strips user info with user and password`() { - assertEquals("http://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user:password@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) + fun `strips user info with user and password from http`() { + val urlDetails = UrlUtils.convertUrl( + "http://user:password@sentry.io?q=1&s=2&token=secret#top" + ) + assertEquals("http://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test fun `strips user info with user and password from https`() { - assertEquals("https://%s:%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user:password@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) + val urlDetails = UrlUtils.convertUrl( + "https://user:password@sentry.io?q=1&s=2&token=secret#top" + ) + assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test - fun `strips user info with user only`() { - assertEquals("http://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("http://user@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) + fun `splits url`() { + val urlDetails = UrlUtils.convertUrl( + "https://sentry.io?q=1&s=2&token=secret#top" + ) + assertEquals("https://sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test - fun `strips user info with user only from https`() { - assertEquals("https://%s@sentry.io", UrlUtils.maybeStripSensitiveDataFromUrl("https://user@sentry.io", SentryOptions().also { it.isSendDefaultPii = false })) + fun `strips user info with user and password without query`() { + val urlDetails = UrlUtils.convertUrl( + "https://user:password@sentry.io#top" + ) + assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test - fun `strips token from query params as first param`() { - assertEquals("https://sentry.io?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret", SentryOptions().also { it.isSendDefaultPii = false })) + fun `splits without query`() { + val urlDetails = UrlUtils.convertUrl( + "https://sentry.io#top" + ) + assertEquals("https://sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test - fun `strips token from query params as later param`() { - assertEquals("https://sentry.io?q=%s&s=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret", SentryOptions().also { it.isSendDefaultPii = false })) + fun `strips user info with user and password without fragment`() { + val urlDetails = UrlUtils.convertUrl( + "https://user:password@sentry.io?q=1&s=2&token=secret" + ) + assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertNull(urlDetails.fragment) } @Test - fun `strips token from query params as first param and keeps anchor`() { - assertEquals("https://sentry.io?token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?token=secret#top", SentryOptions().also { it.isSendDefaultPii = false })) + fun `strips user info with user and password without query or fragment`() { + val urlDetails = UrlUtils.convertUrl( + "https://user:password@sentry.io" + ) + assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) } @Test - fun `strips token from query params as later param and keeps anchor`() { - assertEquals("https://sentry.io?q=%s&s=%s&token=%s#top", UrlUtils.maybeStripSensitiveDataFromUrl("https://sentry.io?q=1&s=2&token=secret#top", SentryOptions().also { it.isSendDefaultPii = false })) + fun `splits url without query or fragment and no authority`() { + val urlDetails = UrlUtils.convertUrl( + "https://sentry.io" + ) + assertEquals("https://sentry.io", urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) } @Test - fun `strips token from query params after anchor`() { - assertEquals("https://api.github.com/users/getsentry/repos/#fragment?token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?token=query", SentryOptions().also { it.isSendDefaultPii = false })) + fun `strips user info with user only`() { + val urlDetails = UrlUtils.convertUrl( + "https://user@sentry.io?q=1&s=2&token=secret#top" + ) + assertEquals("https://[Filtered]@sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test - fun `strips token from query params after anchor with &`() { - assertEquals("https://api.github.com/users/getsentry/repos/#fragment?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromUrl("https://api.github.com/users/getsentry/repos/#fragment?q=1&token=query", SentryOptions().also { it.isSendDefaultPii = false })) + fun `no details extracted with query after fragment`() { + val urlDetails = UrlUtils.convertUrl( + "https://user:password@sentry.io#fragment?q=1&s=2&token=secret" + ) + assertNull(urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) } @Test - fun `strips query params`() { - assertEquals("?q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromQuery("?q=%s&token=query", SentryOptions().also { it.isSendDefaultPii = false })) + fun `no details extracted with query after fragment without authority`() { + val urlDetails = UrlUtils.convertUrl( + "https://sentry.io#fragment?q=1&s=2&token=secret" + ) + assertNull(urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) } @Test - fun `strips query params without ?`() { - assertEquals("q=%s&token=%s", UrlUtils.maybeStripSensitiveDataFromQuery("q=%s&token=query", SentryOptions().also { it.isSendDefaultPii = false })) + fun `no details extracted from malformed url`() { + val urlDetails = UrlUtils.convertUrl( + "htps://user@sentry.io#fragment?q=1&s=2&token=secret" + ) + assertNull(urlDetails.url) + assertNull(urlDetails.query) + assertNull(urlDetails.fragment) } } From 2ac6f7a02f173ba526fd104f21482e4713ffb900 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 18 Jan 2023 09:51:11 +0100 Subject: [PATCH 05/10] Handle relative URLs as well --- .../main/java/io/sentry/util/UrlUtils.java | 64 ++++++++++++++++++- .../test/java/io/sentry/util/UrlUtilsTest.kt | 50 +++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index 8a71b228d5..793aff177f 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -22,8 +22,70 @@ public final class UrlUtils { } public static @NotNull UrlDetails convertUrl(final @NotNull String url) { - final @NotNull String filteredUrl = urlWithAuthRemoved(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); + + return new UrlDetails(baseUrl, query, fragment); + } + + 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; + } + } + + 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; + } + } + + private static @NotNull UrlDetails splitAbsoluteUrl(final @NotNull String url) { try { + final @NotNull String filteredUrl = urlWithAuthRemoved(url); final @NotNull URL urlObj = new URL(url); final @NotNull String baseUrl = baseUrlOnly(filteredUrl); if (baseUrl.contains("#")) { diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index adacf0d623..fb01c3e921 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -41,6 +41,56 @@ class UrlUtilsTest { assertEquals("top", urlDetails.fragment) } + @Test + fun `splits relative url`() { + val urlDetails = UrlUtils.convertUrl( + "/users/1?q=1&s=2&token=secret#top" + ) + assertEquals("/users/1", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + + @Test + fun `splits relative root url`() { + val urlDetails = UrlUtils.convertUrl( + "/?q=1&s=2&token=secret#top" + ) + assertEquals("/", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + + @Test + fun `splits url with just query and fragment`() { + val urlDetails = UrlUtils.convertUrl( + "/?q=1&s=2&token=secret#top" + ) + assertEquals("/", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + + @Test + fun `splits relative url with query only`() { + val urlDetails = UrlUtils.convertUrl( + "/users/1?q=1&s=2&token=secret" + ) + assertEquals("/users/1", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertNull(urlDetails.fragment) + } + + @Test + fun `splits relative url with fragment only`() { + val urlDetails = UrlUtils.convertUrl( + "/users/1#top" + ) + assertEquals("/users/1", urlDetails.url) + assertNull(urlDetails.query) + assertEquals("top", urlDetails.fragment) + } + @Test fun `strips user info with user and password without query`() { val urlDetails = UrlUtils.convertUrl( From 042369dcbae4b459d3b308ac940c1333334a7082 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 23 Jan 2023 07:36:01 +0100 Subject: [PATCH 06/10] Filter more URLs --- .../jakarta/SentryRequestHttpServletRequestProcessor.java | 5 ++++- .../servlet/SentryRequestHttpServletRequestProcessor.java | 5 ++++- .../io/sentry/spring/jakarta/SentryRequestResolver.java | 5 ++++- .../spring/jakarta/webflux/SentryRequestResolver.java | 8 ++++++-- .../jakarta/webflux/SentryWebfluxIntegrationTest.kt | 6 ++++-- .../main/java/io/sentry/spring/SentryRequestResolver.java | 5 ++++- .../spring/tracing/SentrySpanClientWebRequestFilter.java | 5 ++++- .../io/sentry/spring/webflux/SentryRequestResolver.java | 7 +++++-- .../sentry/spring/webflux/SentryWebfluxIntegrationTest.kt | 6 ++++-- 9 files changed, 39 insertions(+), 13 deletions(-) diff --git a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java index 20c97a0710..59e2086c5d 100644 --- a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java @@ -6,6 +6,7 @@ import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; +import io.sentry.util.UrlUtils; import jakarta.servlet.http.HttpServletRequest; import java.util.Collections; import java.util.Enumeration; @@ -30,8 +31,10 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http public @NotNull SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); + final @NotNull UrlUtils.UrlDetails urlDetails = + UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); - sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); event.setRequest(sentryRequest); diff --git a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java index b11f17e739..039b1126e5 100644 --- a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java @@ -6,6 +6,7 @@ import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; +import io.sentry.util.UrlUtils; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; @@ -30,8 +31,10 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http public @NotNull SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); + final @NotNull UrlUtils.UrlDetails urlDetails = + UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); - sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); event.setRequest(sentryRequest); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java index e4674609a6..6e7643363a 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java @@ -9,6 +9,8 @@ import java.util.Enumeration; import java.util.HashMap; import java.util.Map; + +import io.sentry.util.UrlUtils; import jakarta.servlet.http.HttpServletRequest; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -26,8 +28,9 @@ public SentryRequestResolver(final @NotNull IHub hub) { public @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); - sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); if (hub.getOptions().isSendDefaultPii()) { diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java index d0590b159d..91b505e6e0 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java @@ -5,6 +5,9 @@ import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; +import io.sentry.util.UrlUtils; + +import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -28,8 +31,9 @@ public SentryRequestResolver(final @NotNull IHub hub) { final String methodName = httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown"; sentryRequest.setMethod(methodName); - sentryRequest.setQueryString(httpRequest.getURI().getQuery()); - sentryRequest.setUrl(httpRequest.getURI().toString()); + final @NotNull URI uri = httpRequest.getURI(); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(uri.toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders())); if (hub.getOptions().isSendDefaultPii()) { diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt index 8b9710060b..2abb45b913 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt @@ -38,6 +38,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull @RunWith(SpringRunner::class) @SpringBootTest( @@ -63,7 +64,7 @@ class SentryWebfluxIntegrationTest { @Test fun `attaches request information to SentryEvents`() { testClient.get() - .uri("http://localhost:$port/hello?param=value") + .uri("http://localhost:$port/hello?param=value#top") .exchange() .expectStatus() .isOk @@ -71,9 +72,10 @@ class SentryWebfluxIntegrationTest { verify(transport).send( checkEvent { event -> assertNotNull(event.request) { - assertEquals("http://localhost:$port/hello?param=value", it.url) + assertEquals("http://localhost:$port/hello", it.url) assertEquals("GET", it.method) assertEquals("param=value", it.queryString) + assertNull(it.fragment) } }, anyOrNull() diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java index 18aa9bcded..02ca5dc5b2 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java @@ -5,6 +5,7 @@ import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; +import io.sentry.util.UrlUtils; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; @@ -26,8 +27,10 @@ public SentryRequestResolver(final @NotNull IHub hub) { public @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); + final @NotNull UrlUtils.UrlDetails urlDetails = + UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); - sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); if (hub.getOptions().isSendDefaultPii()) { diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 399e7044c9..ebade83853 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -13,6 +13,7 @@ import io.sentry.SpanStatus; import io.sentry.util.Objects; import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.UrlUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.web.reactive.function.client.ClientRequest; @@ -39,7 +40,9 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { } final ISpan span = activeSpan.startChild("http.client"); - span.setDescription(request.method().name() + " " + request.url()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.url().toString()); + span.setDescription(request.method().name() + " " + urlDetails.getUrlOrFallback()); + urlDetails.applyToSpan(span); final ClientRequest.Builder requestBuilder = ClientRequest.from(request); diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java index 3c66dae6ea..1c85cdab16 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java @@ -5,6 +5,8 @@ import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; +import io.sentry.util.UrlUtils; +import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -28,8 +30,9 @@ public SentryRequestResolver(final @NotNull IHub hub) { final String methodName = httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown"; sentryRequest.setMethod(methodName); - sentryRequest.setQueryString(httpRequest.getURI().getQuery()); - sentryRequest.setUrl(httpRequest.getURI().toString()); + final @NotNull URI uri = httpRequest.getURI(); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(uri.toString()); + urlDetails.applyToRequest(sentryRequest); sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders())); if (hub.getOptions().isSendDefaultPii()) { diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt index 4e125e97c8..1ef8cc0f2a 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt @@ -38,6 +38,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull @RunWith(SpringRunner::class) @SpringBootTest( @@ -63,7 +64,7 @@ class SentryWebfluxIntegrationTest { @Test fun `attaches request information to SentryEvents`() { testClient.get() - .uri("http://localhost:$port/hello?param=value") + .uri("http://localhost:$port/hello?param=value#top") .exchange() .expectStatus() .isOk @@ -71,9 +72,10 @@ class SentryWebfluxIntegrationTest { verify(transport).send( checkEvent { event -> assertNotNull(event.request) { - assertEquals("http://localhost:$port/hello?param=value", it.url) + assertEquals("http://localhost:$port/hello", it.url) assertEquals("GET", it.method) assertEquals("param=value", it.queryString) + assertNull(it.fragment) } }, anyOrNull() From 3117892662fb8e33f28523a83fac2834adc1db54 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 23 Jan 2023 07:53:33 +0100 Subject: [PATCH 07/10] Change wording in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99f7313299..6dad30fdab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ ### Fixes -- Remove sensitive data from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) +- Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) ## 6.12.1 From 349930e7ee5577057dd09109ba5c03746e8b8408 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 27 Jan 2023 11:59:02 +0100 Subject: [PATCH 08/10] make UrlUtils internal and rename to parse --- .../android/okhttp/SentryOkHttpInterceptor.kt | 4 +- .../apollo3/SentryApollo3HttpInterceptor.kt | 2 +- .../sentry/openfeign/SentryFeignClient.java | 2 +- ...tryRequestHttpServletRequestProcessor.java | 2 +- ...tryRequestHttpServletRequestProcessor.java | 2 +- .../spring/jakarta/SentryRequestResolver.java | 2 +- ...entrySpanClientHttpRequestInterceptor.java | 2 +- .../webflux/SentryRequestResolver.java | 2 +- .../sentry/spring/SentryRequestResolver.java | 2 +- ...entrySpanClientHttpRequestInterceptor.java | 3 +- .../SentrySpanClientWebRequestFilter.java | 2 +- .../spring/webflux/SentryRequestResolver.java | 2 +- sentry/api/sentry.api | 4 +- .../src/main/java/io/sentry/Breadcrumb.java | 2 +- .../main/java/io/sentry/util/UrlUtils.java | 8 ++-- .../test/java/io/sentry/util/UrlUtilsTest.kt | 46 +++++++++++-------- 16 files changed, 49 insertions(+), 38 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 6e10d3ba02..0e28e6f460 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -54,7 +54,7 @@ class SentryOkHttpInterceptor( override fun intercept(chain: Interceptor.Chain): Response { var request = chain.request() - val urlDetails = UrlUtils.convertUrl(request.url.toString()) + val urlDetails = UrlUtils.parse(request.url.toString()) val url = urlDetails.urlOrFallback val method = request.method @@ -152,7 +152,7 @@ class SentryOkHttpInterceptor( // url will be: https://api.github.com/users/getsentry/repos/ // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ // but that's not possible - val urlDetails = UrlUtils.convertUrl(request.url.toString()) + val urlDetails = UrlUtils.parse(request.url.toString()) // return if its not a target match if (!PropagationTargetsUtils.contain(failedRequestTargets, request.url.toString())) { diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 08e4309135..71b8c5adcb 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -81,7 +81,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH } private fun startChild(request: HttpRequest, activeSpan: ISpan): ISpan { - val urlDetails = UrlUtils.convertUrl(request.url) + val urlDetails = UrlUtils.parse(request.url) val method = request.method val operationName = operationNameFromHeaders(request) diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index 5b788b3a3b..ead779c3e8 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -51,7 +51,7 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O } ISpan span = activeSpan.startChild("http.client"); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.url()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.url()); span.setDescription(request.httpMethod().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); diff --git a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java index 59e2086c5d..17ac102090 100644 --- a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java @@ -32,7 +32,7 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); final @NotNull UrlUtils.UrlDetails urlDetails = - UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + UrlUtils.parse(httpRequest.getRequestURL().toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); diff --git a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java index 039b1126e5..b24c0446b0 100644 --- a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java @@ -32,7 +32,7 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); final @NotNull UrlUtils.UrlDetails urlDetails = - UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + UrlUtils.parse(httpRequest.getRequestURL().toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java index 6e7643363a..eda5d2e537 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java @@ -28,7 +28,7 @@ public SentryRequestResolver(final @NotNull IHub hub) { public @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(httpRequest.getRequestURL().toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index c8f28f9201..e51c3e0b20 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -49,7 +49,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final ISpan span = activeSpan.startChild("http.client"); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.getURI().toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.getURI().toString()); span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java index 91b505e6e0..02701ab957 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java @@ -32,7 +32,7 @@ public SentryRequestResolver(final @NotNull IHub hub) { httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown"; sentryRequest.setMethod(methodName); final @NotNull URI uri = httpRequest.getURI(); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(uri.toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(uri.toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders())); diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java index 02ca5dc5b2..59dc4def54 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java @@ -28,7 +28,7 @@ public SentryRequestResolver(final @NotNull IHub hub) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); final @NotNull UrlUtils.UrlDetails urlDetails = - UrlUtils.convertUrl(httpRequest.getRequestURL().toString()); + UrlUtils.parse(httpRequest.getRequestURL().toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index b249d8ab89..09a4bcc8a0 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -48,8 +48,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final ISpan span = activeSpan.startChild("http.client"); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - final @NotNull UrlUtils.UrlDetails urlDetails = - UrlUtils.convertUrl(request.getURI().toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.getURI().toString()); urlDetails.applyToSpan(span); span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index ebade83853..1c82865ed7 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -40,7 +40,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { } final ISpan span = activeSpan.startChild("http.client"); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(request.url().toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.url().toString()); span.setDescription(request.method().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java index 1c85cdab16..dd2006466a 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java @@ -31,7 +31,7 @@ public SentryRequestResolver(final @NotNull IHub hub) { httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown"; sentryRequest.setMethod(methodName); final @NotNull URI uri = httpRequest.getURI(); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(uri.toString()); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(uri.toString()); urlDetails.applyToRequest(sentryRequest); sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders())); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8f3b89b1d7..9235cbc5b4 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3712,8 +3712,8 @@ public final class io/sentry/util/StringUtils { public final class io/sentry/util/UrlUtils { public fun ()V - public static fun convertUrl (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; - public static fun convertUrlNullable (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; + public static fun parse (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; + public static fun parseNullable (Ljava/lang/String;)Lio/sentry/util/UrlUtils$UrlDetails; } public final class io/sentry/util/UrlUtils$UrlDetails { diff --git a/sentry/src/main/java/io/sentry/Breadcrumb.java b/sentry/src/main/java/io/sentry/Breadcrumb.java index ee712af8ff..868360c664 100644 --- a/sentry/src/main/java/io/sentry/Breadcrumb.java +++ b/sentry/src/main/java/io/sentry/Breadcrumb.java @@ -68,7 +68,7 @@ public Breadcrumb(final @NotNull Date timestamp) { */ public static @NotNull Breadcrumb http(final @NotNull String url, final @NotNull String method) { final Breadcrumb breadcrumb = new Breadcrumb(); - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.convertUrl(url); + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(url); breadcrumb.setType("http"); breadcrumb.setCategory("http"); if (urlDetails.getUrl() != null) { diff --git a/sentry/src/main/java/io/sentry/util/UrlUtils.java b/sentry/src/main/java/io/sentry/util/UrlUtils.java index 793aff177f..6cdc4c2d6a 100644 --- a/sentry/src/main/java/io/sentry/util/UrlUtils.java +++ b/sentry/src/main/java/io/sentry/util/UrlUtils.java @@ -6,22 +6,24 @@ import java.net.URL; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +@ApiStatus.Internal public final class UrlUtils { private static final @NotNull Pattern AUTH_REGEX = Pattern.compile("(.+://)(.*@)(.*)"); - public static @Nullable UrlDetails convertUrlNullable(final @Nullable String url) { + public static @Nullable UrlDetails parseNullable(final @Nullable String url) { if (url == null) { return null; } - return convertUrl(url); + return parse(url); } - public static @NotNull UrlDetails convertUrl(final @NotNull String url) { + public static @NotNull UrlDetails parse(final @NotNull String url) { if (isAbsoluteUrl(url)) { return splitAbsoluteUrl(url); } else { diff --git a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt index fb01c3e921..af037b3344 100644 --- a/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/UrlUtilsTest.kt @@ -8,12 +8,22 @@ class UrlUtilsTest { @Test fun `returns null for null`() { - assertNull(UrlUtils.convertUrlNullable(null)) + assertNull(UrlUtils.parseNullable(null)) + } + + @Test + fun `strips user info with user and password from http nullable`() { + val urlDetails = UrlUtils.parseNullable( + "http://user:password@sentry.io?q=1&s=2&token=secret#top" + )!! + assertEquals("http://[Filtered]:[Filtered]@sentry.io", urlDetails.url) + assertEquals("q=1&s=2&token=secret", urlDetails.query) + assertEquals("top", urlDetails.fragment) } @Test fun `strips user info with user and password from http`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "http://user:password@sentry.io?q=1&s=2&token=secret#top" ) assertEquals("http://[Filtered]:[Filtered]@sentry.io", urlDetails.url) @@ -23,7 +33,7 @@ class UrlUtilsTest { @Test fun `strips user info with user and password from https`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user:password@sentry.io?q=1&s=2&token=secret#top" ) assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) @@ -33,7 +43,7 @@ class UrlUtilsTest { @Test fun `splits url`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://sentry.io?q=1&s=2&token=secret#top" ) assertEquals("https://sentry.io", urlDetails.url) @@ -43,7 +53,7 @@ class UrlUtilsTest { @Test fun `splits relative url`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "/users/1?q=1&s=2&token=secret#top" ) assertEquals("/users/1", urlDetails.url) @@ -53,7 +63,7 @@ class UrlUtilsTest { @Test fun `splits relative root url`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "/?q=1&s=2&token=secret#top" ) assertEquals("/", urlDetails.url) @@ -63,7 +73,7 @@ class UrlUtilsTest { @Test fun `splits url with just query and fragment`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "/?q=1&s=2&token=secret#top" ) assertEquals("/", urlDetails.url) @@ -73,7 +83,7 @@ class UrlUtilsTest { @Test fun `splits relative url with query only`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "/users/1?q=1&s=2&token=secret" ) assertEquals("/users/1", urlDetails.url) @@ -83,7 +93,7 @@ class UrlUtilsTest { @Test fun `splits relative url with fragment only`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "/users/1#top" ) assertEquals("/users/1", urlDetails.url) @@ -93,7 +103,7 @@ class UrlUtilsTest { @Test fun `strips user info with user and password without query`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user:password@sentry.io#top" ) assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) @@ -103,7 +113,7 @@ class UrlUtilsTest { @Test fun `splits without query`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://sentry.io#top" ) assertEquals("https://sentry.io", urlDetails.url) @@ -113,7 +123,7 @@ class UrlUtilsTest { @Test fun `strips user info with user and password without fragment`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user:password@sentry.io?q=1&s=2&token=secret" ) assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) @@ -123,7 +133,7 @@ class UrlUtilsTest { @Test fun `strips user info with user and password without query or fragment`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user:password@sentry.io" ) assertEquals("https://[Filtered]:[Filtered]@sentry.io", urlDetails.url) @@ -133,7 +143,7 @@ class UrlUtilsTest { @Test fun `splits url without query or fragment and no authority`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://sentry.io" ) assertEquals("https://sentry.io", urlDetails.url) @@ -143,7 +153,7 @@ class UrlUtilsTest { @Test fun `strips user info with user only`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user@sentry.io?q=1&s=2&token=secret#top" ) assertEquals("https://[Filtered]@sentry.io", urlDetails.url) @@ -153,7 +163,7 @@ class UrlUtilsTest { @Test fun `no details extracted with query after fragment`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://user:password@sentry.io#fragment?q=1&s=2&token=secret" ) assertNull(urlDetails.url) @@ -163,7 +173,7 @@ class UrlUtilsTest { @Test fun `no details extracted with query after fragment without authority`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "https://sentry.io#fragment?q=1&s=2&token=secret" ) assertNull(urlDetails.url) @@ -173,7 +183,7 @@ class UrlUtilsTest { @Test fun `no details extracted from malformed url`() { - val urlDetails = UrlUtils.convertUrl( + val urlDetails = UrlUtils.parse( "htps://user@sentry.io#fragment?q=1&s=2&token=secret" ) assertNull(urlDetails.url) From 0c58acc0aea7c0f4ded583752343d1c2be83d04d Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 30 Jan 2023 09:30:10 +0100 Subject: [PATCH 09/10] Add test for url details apply methods --- .../src/test/java/io/sentry/UrlDetailsTest.kt | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 sentry/src/test/java/io/sentry/UrlDetailsTest.kt diff --git a/sentry/src/test/java/io/sentry/UrlDetailsTest.kt b/sentry/src/test/java/io/sentry/UrlDetailsTest.kt new file mode 100644 index 0000000000..bad6df1a46 --- /dev/null +++ b/sentry/src/test/java/io/sentry/UrlDetailsTest.kt @@ -0,0 +1,83 @@ +package io.sentry + +import io.sentry.protocol.Request +import io.sentry.util.UrlUtils +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class UrlDetailsTest { + + @Test + fun `does not crash on null span`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", "q=1", "top") + urlDetails.applyToSpan(null) + } + + @Test + fun `applies query and fragment to span`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", "q=1", "top") + val span = mock() + urlDetails.applyToSpan(span) + + verify(span).setData("http.query", "q=1") + verify(span).setData("http.fragment", "top") + } + + @Test + fun `applies query to span`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", "q=1", null) + val span = mock() + urlDetails.applyToSpan(span) + + verify(span).setData("http.query", "q=1") + verifyNoMoreInteractions(span) + } + + @Test + fun `applies fragment to span`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", null, "top") + val span = mock() + urlDetails.applyToSpan(span) + + verify(span).setData("http.fragment", "top") + verifyNoMoreInteractions(span) + } + + @Test + fun `does not crash on null request`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", "q=1", "top") + urlDetails.applyToRequest(null) + } + + @Test + fun `applies details to request`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", "q=1", "top") + val request = Request() + urlDetails.applyToRequest(request) + + assertEquals("https://sentry.io/api", request.url) + assertEquals("q=1", request.queryString) + assertEquals("top", request.fragment) + } + + @Test + fun `applies details without fragment and url to request`() { + val urlDetails = UrlUtils.UrlDetails("https://sentry.io/api", null, null) + val request = Request() + urlDetails.applyToRequest(request) + + assertEquals("https://sentry.io/api", request.url) + assertNull(request.queryString) + assertNull(request.fragment) + } + + @Test + fun `returns fallback for null URL`() { + val urlDetails = UrlUtils.UrlDetails(null, null, null) + assertEquals("unknown", urlDetails.urlOrFallback) + } +} From 5254e472c6d20ab832d246b7addde6e15250d3f5 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 30 Jan 2023 11:23:19 +0100 Subject: [PATCH 10/10] Cover http breadcrumb with test --- sentry/src/test/java/io/sentry/BreadcrumbTest.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt index e6b417cbc0..a710e54ec6 100644 --- a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt +++ b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt @@ -101,8 +101,10 @@ class BreadcrumbTest { @Test fun `creates HTTP breadcrumb`() { - val breadcrumb = Breadcrumb.http("http://example.com", "POST") - assertEquals("http://example.com", breadcrumb.data["url"]) + val breadcrumb = Breadcrumb.http("http://example.com/api?q=1#top", "POST") + assertEquals("http://example.com/api", breadcrumb.data["url"]) + assertEquals("q=1", breadcrumb.data["http.query"]) + assertEquals("top", breadcrumb.data["http.fragment"]) assertEquals("POST", breadcrumb.data["method"]) assertEquals("http", breadcrumb.type) assertEquals("http", breadcrumb.category)