From 4ba4e71543e101ef4f619029ee711e95f9a5fc4f Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Wed, 3 Aug 2022 16:10:35 +0200 Subject: [PATCH 01/18] ISSUE-2674 Strip sensitive data from the url when used in the clientAttributesExtractor --- .../http/HttpClientAttributesExtractor.java | 10 +++++++++- .../http/HttpClientAttributesExtractorTest.java | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index ae49e32d048b..b13c3c72d554 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -54,7 +54,15 @@ public static HttpClientAttributesExtractorBuilder request = new HashMap<>(); + request.put("url", "https://user1:secret@github.com"); + + HttpClientAttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter()).build(); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), request); + + assertThat(attributes.build()) + .containsOnly(entry(SemanticAttributes.HTTP_URL, "https://username:password@github.com")); + } + @Test void invalidStatusCode() { Map request = new HashMap<>(); From bc1ff06c4a78809b70e06b4d46faa19ed865cd88 Mon Sep 17 00:00:00 2001 From: Malte Date: Fri, 5 Aug 2022 15:01:43 +0200 Subject: [PATCH 02/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Mateusz Rzeszutek --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index b13c3c72d554..4161f606b4cf 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -58,7 +58,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST } private static String stripSensitiveData(String url) { - if (url.isEmpty() || url == null) { + if (url == null || url.isEmpty()) { return url; } // replace username & password From 89699d19f50c14a01a3b216e0976facb60ec4b6c Mon Sep 17 00:00:00 2001 From: Malte Date: Fri, 5 Aug 2022 15:09:10 +0200 Subject: [PATCH 03/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Mateusz Rzeszutek --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 4161f606b4cf..49342af342de 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -57,7 +57,8 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST internalSet(attributes, SemanticAttributes.HTTP_URL, stripSensitiveData(getter.url(request))); } - private static String stripSensitiveData(String url) { + @Nullable + private static String stripSensitiveData(@Nullable String url) { if (url == null || url.isEmpty()) { return url; } From 4da196e426754c6358effaa809078120768545a0 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Mon, 22 Aug 2022 22:06:27 +0200 Subject: [PATCH 04/18] Issue-2674 Replace regular expression with index of in order to avoid potential performance issues. --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 3 ++- .../instrumenter/http/HttpClientAttributesExtractorTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 49342af342de..84a807c93044 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -62,8 +62,9 @@ private static String stripSensitiveData(@Nullable String url) { if (url == null || url.isEmpty()) { return url; } + // replace username & password - return url.replaceFirst("(?<=\\/\\/)(.+)(?=@)", "username:password"); + return url.substring(0, url.indexOf("//")+2) + url.substring(url.indexOf("@")+1); } @Override diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index c94b8bdc2c17..76220c706485 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -152,7 +152,7 @@ public void stripBasicAuthTest() { extractor.onStart(attributes, Context.root(), request); assertThat(attributes.build()) - .containsOnly(entry(SemanticAttributes.HTTP_URL, "https://username:password@github.com")); + .containsOnly(entry(SemanticAttributes.HTTP_URL, "https://github.com")); } @Test From 76e9bf8c7ed2ef158747f9a8916deb7fc9d1de53 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Tue, 23 Aug 2022 07:54:53 +0200 Subject: [PATCH 05/18] Issue-2674 Add check to only do replacement logic in case the address contains an at character. --- .../instrumenter/http/HttpClientAttributesExtractor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 84a807c93044..ed37f35f88a1 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -64,7 +64,11 @@ private static String stripSensitiveData(@Nullable String url) { } // replace username & password - return url.substring(0, url.indexOf("//")+2) + url.substring(url.indexOf("@")+1); + if(url.contains("@")) { + return url.substring(0, url.indexOf("//")+2) + url.substring(url.indexOf("@")+1); + } else { + return url; + } } @Override From 6adfacd2e098f461cb6aaae5179436b1bea50041 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Tue, 23 Aug 2022 14:41:24 +0200 Subject: [PATCH 06/18] Issue-2674 Fix issues reported by checkstyle, apply suggestion not to run indexOf twice --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index ed37f35f88a1..06f8fd1d736e 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -63,9 +63,10 @@ private static String stripSensitiveData(@Nullable String url) { return url; } + int atIndex = url.indexOf("@"); // replace username & password - if(url.contains("@")) { - return url.substring(0, url.indexOf("//")+2) + url.substring(url.indexOf("@")+1); + if (atIndex != -1) { + return url.substring(0, url.indexOf("//") + 2) + url.substring(atIndex + 1); } else { return url; } From 5d90ea722a6fddd77ef292dadc3cdcb2953da589 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Mon, 19 Sep 2022 10:56:06 +0200 Subject: [PATCH 07/18] ISSUE-2674 Add check for the scheme and a few more test cases --- .../http/HttpClientAttributesExtractor.java | 17 ++++++++++---- .../HttpClientAttributesExtractorTest.java | 23 +++++++++++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 06f8fd1d736e..c5f80b6b02d5 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -64,12 +64,21 @@ private static String stripSensitiveData(@Nullable String url) { } int atIndex = url.indexOf("@"); - // replace username & password - if (atIndex != -1) { - return url.substring(0, url.indexOf("//") + 2) + url.substring(atIndex + 1); - } else { + int questionMarkIndex = url.indexOf("?"); + + // '@' char is present and is not a query param + if (atIndex == -1 || (questionMarkIndex != -1 && atIndex > questionMarkIndex)) { return url; } + + int doubleSlash = url.indexOf("//"); + + // No scheme present + if (doubleSlash == -1) { + return url.substring(atIndex + 1); + } else { + return url.substring(0, doubleSlash + 2) + url.substring(atIndex + 1); + } } @Override diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index 76220c706485..73bfd569ed01 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -145,14 +145,33 @@ public void stripBasicAuthTest() { Map request = new HashMap<>(); request.put("url", "https://user1:secret@github.com"); + stripRequestTest(request, "https://github.com"); + } + + @Test + public void stripBasicAuthWithQueryParamTest() { + Map request = new HashMap<>(); + request.put("url", "https://user1:secret@github.com?foo=b@r"); + + stripRequestTest(request, "https://github.com?foo=b@r"); + } + + @Test + public void stripBasicAuthWithoutSchemeTest() { + Map request = new HashMap<>(); + request.put("url", "user1:secret@github.com"); + + stripRequestTest(request, "github.com"); + } + + private static void stripRequestTest(Map request, String expected) { HttpClientAttributesExtractor, Map> extractor = HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter()).build(); AttributesBuilder attributes = Attributes.builder(); extractor.onStart(attributes, Context.root(), request); - assertThat(attributes.build()) - .containsOnly(entry(SemanticAttributes.HTTP_URL, "https://github.com")); + assertThat(attributes.build()).containsOnly(entry(SemanticAttributes.HTTP_URL, expected)); } @Test From c9dcfb939ddbad25c486aa315bf016274aa0dc4c Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Thu, 22 Sep 2022 19:42:02 +0200 Subject: [PATCH 08/18] ISSUE-2674 Change to parameterized test --- .../HttpClientAttributesExtractorTest.java | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index 73bfd569ed01..51b82e112cb3 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -10,6 +10,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.entry; +import static org.junit.jupiter.params.provider.Arguments.arguments; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; @@ -19,7 +20,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class HttpClientAttributesExtractorTest { @@ -139,15 +144,22 @@ void normal() { AttributeKey.stringArrayKey("http.response.header.custom_response_header"), asList("654", "321"))); } - - @Test - public void stripBasicAuthTest() { + @ParameterizedTest + @MethodSource("stripUrlArguments") + public void stripBasicAuthTest(String url, String expectedResult) { Map request = new HashMap<>(); - request.put("url", "https://user1:secret@github.com"); + request.put("url", url); - stripRequestTest(request, "https://github.com"); + stripRequestTest(request, expectedResult); } + private static Stream stripUrlArguments() { + return Stream.of( + arguments("https://user1:secret@github.com", "https://github.com"), + arguments("https://user1:secret@github.com?foo=b@r", "https://github.com?foo=b@r"), + arguments("user1:secret@github.com", "github.com") + ); + } @Test public void stripBasicAuthWithQueryParamTest() { Map request = new HashMap<>(); From 8904b5692bf4e521361ee065cbd778cbfbd5d8dd Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Fri, 23 Sep 2022 08:22:41 +0200 Subject: [PATCH 09/18] ISSUE-2674 Add few more test cases --- .../HttpClientAttributesExtractorTest.java | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index f56253efde76..7ce12eb74e65 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -20,8 +20,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import javax.annotation.Nullable; import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -116,9 +116,10 @@ void normal() { AttributeKey.stringArrayKey("http.response.header.custom_response_header"), asList("654", "321"))); } + @ParameterizedTest @MethodSource("stripUrlArguments") - public void stripBasicAuthTest(String url, String expectedResult) { + void stripBasicAuthTest(String url, String expectedResult) { Map request = new HashMap<>(); request.put("url", url); @@ -128,24 +129,12 @@ public void stripBasicAuthTest(String url, String expectedResult) { private static Stream stripUrlArguments() { return Stream.of( arguments("https://user1:secret@github.com", "https://github.com"), + arguments("https://user1:secret@github.com/path/", "https://github.com/path/"), + arguments("https://user1:secret@github.com#test.html", "https://github.com#test.html"), arguments("https://user1:secret@github.com?foo=b@r", "https://github.com?foo=b@r"), - arguments("user1:secret@github.com", "github.com") - ); - } - @Test - public void stripBasicAuthWithQueryParamTest() { - Map request = new HashMap<>(); - request.put("url", "https://user1:secret@github.com?foo=b@r"); - - stripRequestTest(request, "https://github.com?foo=b@r"); - } - - @Test - public void stripBasicAuthWithoutSchemeTest() { - Map request = new HashMap<>(); - request.put("url", "user1:secret@github.com"); - - stripRequestTest(request, "github.com"); + arguments( + "https://user1:secret@github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), + arguments("user1:secret@github.com", "github.com")); } private static void stripRequestTest(Map request, String expected) { From bb307855b341be70a2679931d9a5895a0b665da8 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Fri, 23 Sep 2022 16:39:15 +0200 Subject: [PATCH 10/18] ISSUE-2674 Check for the end of the host by making use of code from URLParser. --- .../http/HttpClientAttributesExtractor.java | 38 ++++++++++++++----- .../HttpClientAttributesExtractorTest.java | 4 +- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index c5f80b6b02d5..a5809d853135 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -63,22 +63,40 @@ private static String stripSensitiveData(@Nullable String url) { return url; } + int schemeEndIndex = url.indexOf(':'); + + if (schemeEndIndex == -1) { + // not a valid url + return null; + } + + int len = url.length(); + if (len <= schemeEndIndex + 2 + || url.charAt(schemeEndIndex + 1) != '/' + || url.charAt(schemeEndIndex + 2) != '/') { + // has no authority component + return null; + } + + // look for the end of the host: + // ':' ==> start of port, or + // '/', '?', '#' ==> start of path + int index; + for (index = schemeEndIndex + 3; index < len; index++) { + char c = url.charAt(index); + if (c == '/' || c == '?' || c == '#') { + break; + } + } + int atIndex = url.indexOf("@"); int questionMarkIndex = url.indexOf("?"); // '@' char is present and is not a query param - if (atIndex == -1 || (questionMarkIndex != -1 && atIndex > questionMarkIndex)) { + if (atIndex == -1 || (questionMarkIndex != -1 && atIndex > index)) { return url; } - - int doubleSlash = url.indexOf("//"); - - // No scheme present - if (doubleSlash == -1) { - return url.substring(atIndex + 1); - } else { - return url.substring(0, doubleSlash + 2) + url.substring(atIndex + 1); - } + return url.substring(0, schemeEndIndex + 3) + url.substring(atIndex + 1); } @Override diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index 7ce12eb74e65..f9273411be00 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -134,7 +134,9 @@ private static Stream stripUrlArguments() { arguments("https://user1:secret@github.com?foo=b@r", "https://github.com?foo=b@r"), arguments( "https://user1:secret@github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), - arguments("user1:secret@github.com", "github.com")); + arguments("https://github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), + arguments("https://github.com#t@st.html", "https://github.com#t@st.html"), + arguments("user1:secret@github.com", null)); } private static void stripRequestTest(Map request, String expected) { From 1101c163610faeae0e833ee3349f06eae079f442 Mon Sep 17 00:00:00 2001 From: Malte Date: Tue, 27 Sep 2022 15:49:00 +0200 Subject: [PATCH 11/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Trask Stalnaker --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index a5809d853135..09a1f62a6e21 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -67,7 +67,7 @@ private static String stripSensitiveData(@Nullable String url) { if (schemeEndIndex == -1) { // not a valid url - return null; + return url; } int len = url.length(); From 92ee6623c7f5e620eb4b15ad9522231139cd29fa Mon Sep 17 00:00:00 2001 From: Malte Date: Tue, 27 Sep 2022 15:49:08 +0200 Subject: [PATCH 12/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Trask Stalnaker --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 09a1f62a6e21..9e96a44669cc 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -75,7 +75,7 @@ private static String stripSensitiveData(@Nullable String url) { || url.charAt(schemeEndIndex + 1) != '/' || url.charAt(schemeEndIndex + 2) != '/') { // has no authority component - return null; + return url; } // look for the end of the host: From 8434511434a0b5e50d6933807f7b1796a44d228a Mon Sep 17 00:00:00 2001 From: Malte Date: Tue, 27 Sep 2022 15:49:15 +0200 Subject: [PATCH 13/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Trask Stalnaker --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 9e96a44669cc..8529337b6576 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -89,7 +89,7 @@ private static String stripSensitiveData(@Nullable String url) { } } - int atIndex = url.indexOf("@"); + int atIndex = url.lastIndexOf("@", index - 1); int questionMarkIndex = url.indexOf("?"); // '@' char is present and is not a query param From 1f41637e2c57b71814699c6c51c74274f37e808b Mon Sep 17 00:00:00 2001 From: Malte Date: Tue, 27 Sep 2022 15:49:30 +0200 Subject: [PATCH 14/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Trask Stalnaker --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 8529337b6576..2cfa62ed8aa1 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -78,8 +78,7 @@ private static String stripSensitiveData(@Nullable String url) { return url; } - // look for the end of the host: - // ':' ==> start of port, or + // look for the end of the authority component: // '/', '?', '#' ==> start of path int index; for (index = schemeEndIndex + 3; index < len; index++) { From ffdea7db575106f726a5da144f484555caed39a8 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Wed, 28 Sep 2022 15:02:16 +0200 Subject: [PATCH 15/18] ISSUE-2674 Fix unit tests --- .../instrumenter/http/HttpClientAttributesExtractorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index f9273411be00..36140cc0a3b7 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -136,7 +136,7 @@ private static Stream stripUrlArguments() { "https://user1:secret@github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), arguments("https://github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), arguments("https://github.com#t@st.html", "https://github.com#t@st.html"), - arguments("user1:secret@github.com", null)); + arguments("user1:secret@github.com", "user1:secret@github.com")); } private static void stripRequestTest(Map request, String expected) { From a25875ea68a44b292c2dc46299c3ed70b53b2504 Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Wed, 5 Oct 2022 11:15:07 +0200 Subject: [PATCH 16/18] ISSUE-2674 drop questionmarkindex variable and check if at character is last value of string instead. --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 4 +--- .../instrumenter/http/HttpClientAttributesExtractorTest.java | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 2cfa62ed8aa1..eb7b26effbf6 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -89,10 +89,8 @@ private static String stripSensitiveData(@Nullable String url) { } int atIndex = url.lastIndexOf("@", index - 1); - int questionMarkIndex = url.indexOf("?"); - // '@' char is present and is not a query param - if (atIndex == -1 || (questionMarkIndex != -1 && atIndex > index)) { + if (atIndex == -1 || atIndex == url.length() - 1) { return url; } return url.substring(0, schemeEndIndex + 3) + url.substring(atIndex + 1); diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index 36140cc0a3b7..d99052a589e3 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -136,7 +136,8 @@ private static Stream stripUrlArguments() { "https://user1:secret@github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), arguments("https://github.com/p@th?foo=b@r", "https://github.com/p@th?foo=b@r"), arguments("https://github.com#t@st.html", "https://github.com#t@st.html"), - arguments("user1:secret@github.com", "user1:secret@github.com")); + arguments("user1:secret@github.com", "user1:secret@github.com"), + arguments("https://github.com@", "https://github.com@")); } private static void stripRequestTest(Map request, String expected) { From 6b971960940362b2676319b3ecc38d9a03bc14cc Mon Sep 17 00:00:00 2001 From: Malte Date: Wed, 5 Oct 2022 13:53:17 +0200 Subject: [PATCH 17/18] Update instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java Co-authored-by: Mateusz Rzeszutek --- .../api/instrumenter/http/HttpClientAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index eb7b26effbf6..706984b9a189 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -90,7 +90,7 @@ private static String stripSensitiveData(@Nullable String url) { int atIndex = url.lastIndexOf("@", index - 1); - if (atIndex == -1 || atIndex == url.length() - 1) { + if (atIndex == -1 || atIndex == len - 1) { return url; } return url.substring(0, schemeEndIndex + 3) + url.substring(atIndex + 1); From 85f4c4f4572f92d78aedd8288263f31b386269ae Mon Sep 17 00:00:00 2001 From: Malte Pickhan Date: Thu, 6 Oct 2022 20:16:25 +0200 Subject: [PATCH 18/18] ISSUE-2674 avoid looking up at character twice --- .../instrumenter/http/HttpClientAttributesExtractor.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index eb7b26effbf6..5a74e41b2cdd 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -81,15 +81,19 @@ private static String stripSensitiveData(@Nullable String url) { // look for the end of the authority component: // '/', '?', '#' ==> start of path int index; + int atIndex = -1; for (index = schemeEndIndex + 3; index < len; index++) { char c = url.charAt(index); + + if (c == '@') { + atIndex = index; + } + if (c == '/' || c == '?' || c == '#') { break; } } - int atIndex = url.lastIndexOf("@", index - 1); - if (atIndex == -1 || atIndex == url.length() - 1) { return url; }