From 66d58aa0a6d4dad2a077ade5a6e8ba19e0076bd2 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 6 Oct 2021 01:08:17 +0200 Subject: [PATCH] Move clientIp extraction to HttpServerAttributesExtractor (#4301) Co-authored-by: Trask Stalnaker --- .../api/instrumenter/ServerInstrumenter.java | 127 +-------- .../http/ForwarderHeaderParser.java | 71 +++++ .../http/HttpCommonAttributesExtractor.java | 8 +- .../http/HttpServerAttributesExtractor.java | 28 +- .../api/instrumenter/InstrumenterTest.java | 260 +----------------- .../http/ForwarderHeaderParserTest.java | 232 ++++++++++++++++ .../HttpServerAttributesExtractorTest.java | 43 ++- 7 files changed, 373 insertions(+), 396 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParser.java create mode 100644 instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParserTest.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java index 43c1e34db3..f26a2e7e66 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java @@ -5,14 +5,10 @@ package io.opentelemetry.instrumentation.api.instrumenter; -import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.context.propagation.TextMapGetter; -import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor; import io.opentelemetry.instrumentation.api.internal.ContextPropagationDebug; -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; -import org.checkerframework.checker.nullness.qual.Nullable; final class ServerInstrumenter extends Instrumenter { @@ -21,7 +17,7 @@ final class ServerInstrumenter extends Instrumenter builder, TextMapGetter getter) { - super(addClientIpExtractor(builder, getter)); + super(builder); this.propagators = builder.openTelemetry.getPropagators(); this.getter = getter; } @@ -33,125 +29,4 @@ final class ServerInstrumenter extends Instrumenter InstrumenterBuilder addClientIpExtractor( - InstrumenterBuilder builder, TextMapGetter getter) { - HttpServerAttributesExtractor httpAttributesExtractor = null; - for (AttributesExtractor extractor : - builder.attributesExtractors) { - if (extractor instanceof HttpServerAttributesExtractor) { - httpAttributesExtractor = (HttpServerAttributesExtractor) extractor; - } - } - if (httpAttributesExtractor == null) { - // Don't add HTTP_CLIENT_IP if there are no HTTP attributes registered. - return builder; - } - builder.addAttributesExtractor(new HttpClientIpExtractor<>(getter)); - return builder; - } - - private static class HttpClientIpExtractor - extends AttributesExtractor { - - private final TextMapGetter getter; - - HttpClientIpExtractor(TextMapGetter getter) { - this.getter = getter; - } - - @Override - protected void onStart(AttributesBuilder attributes, REQUEST request) {} - - @Override - protected void onEnd( - AttributesBuilder attributes, - REQUEST request, - @Nullable RESPONSE response, - @Nullable Throwable error) { - String clientIp = getForwardedClientIp(request); - set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp); - } - - @Nullable - // Visible for testing - String getForwardedClientIp(REQUEST request) { - // try Forwarded - String forwarded = getter.get(request, "Forwarded"); - if (forwarded != null) { - forwarded = extractForwarded(forwarded); - if (forwarded != null) { - return forwarded; - } - } - - // try X-Forwarded-For - forwarded = getter.get(request, "X-Forwarded-For"); - if (forwarded != null) { - return extractForwardedFor(forwarded); - } - - return null; - } - } - - // VisibleForTesting - @Nullable - static String extractForwarded(String forwarded) { - int start = forwarded.toLowerCase().indexOf("for="); - if (start < 0) { - return null; - } - start += 4; // start is now the index after for= - if (start >= forwarded.length() - 1) { // the value after for= must not be empty - return null; - } - return extractIpAddress(forwarded, start); - } - - // VisibleForTesting - @Nullable - static String extractForwardedFor(String forwarded) { - return extractIpAddress(forwarded, 0); - } - - // from https://www.rfc-editor.org/rfc/rfc7239 - // "Note that IPv6 addresses may not be quoted in - // X-Forwarded-For and may not be enclosed by square brackets, but they - // are quoted and enclosed in square brackets in Forwarded" - // and also (applying to Forwarded but not X-Forwarded-For) - // "It is important to note that an IPv6 address and any nodename with - // node-port specified MUST be quoted, since ':' is not an allowed - // character in 'token'." - @Nullable - private static String extractIpAddress(String forwarded, int start) { - if (forwarded.length() == start) { - return null; - } - if (forwarded.charAt(start) == '"') { - return extractIpAddress(forwarded, start + 1); - } - if (forwarded.charAt(start) == '[') { - int end = forwarded.indexOf(']', start + 1); - if (end == -1) { - return null; - } - return forwarded.substring(start + 1, end); - } - boolean inIpv4 = false; - for (int i = start; i < forwarded.length() - 1; i++) { - char c = forwarded.charAt(i); - if (c == '.') { - inIpv4 = true; - } else if (c == ',' || c == ';' || c == '"' || (inIpv4 && c == ':')) { - if (i == start) { // empty string - return null; - } - return forwarded.substring(start, i); - } - } - return forwarded.substring(start); - } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParser.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParser.java new file mode 100644 index 0000000000..e3f57ec787 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParser.java @@ -0,0 +1,71 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.http; + +import org.checkerframework.checker.nullness.qual.Nullable; + +final class ForwarderHeaderParser { + + // VisibleForTesting + @Nullable + static String extractForwarded(String forwarded) { + int start = forwarded.toLowerCase().indexOf("for="); + if (start < 0) { + return null; + } + start += 4; // start is now the index after for= + if (start >= forwarded.length() - 1) { // the value after for= must not be empty + return null; + } + return extractIpAddress(forwarded, start); + } + + // VisibleForTesting + @Nullable + static String extractForwardedFor(String forwarded) { + return extractIpAddress(forwarded, 0); + } + + // from https://www.rfc-editor.org/rfc/rfc7239 + // "Note that IPv6 addresses may not be quoted in + // X-Forwarded-For and may not be enclosed by square brackets, but they + // are quoted and enclosed in square brackets in Forwarded" + // and also (applying to Forwarded but not X-Forwarded-For) + // "It is important to note that an IPv6 address and any nodename with + // node-port specified MUST be quoted, since ':' is not an allowed + // character in 'token'." + @Nullable + private static String extractIpAddress(String forwarded, int start) { + if (forwarded.length() == start) { + return null; + } + if (forwarded.charAt(start) == '"') { + return extractIpAddress(forwarded, start + 1); + } + if (forwarded.charAt(start) == '[') { + int end = forwarded.indexOf(']', start + 1); + if (end == -1) { + return null; + } + return forwarded.substring(start + 1, end); + } + boolean inIpv4 = false; + for (int i = start; i < forwarded.length() - 1; i++) { + char c = forwarded.charAt(i); + if (c == '.') { + inIpv4 = true; + } else if (c == ',' || c == ';' || c == '"' || (inIpv4 && c == ':')) { + if (i == start) { // empty string + return null; + } + return forwarded.substring(start, i); + } + } + return forwarded.substring(start); + } + + private ForwarderHeaderParser() {} +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index 589b0e7744..7abbe08cba 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -89,8 +89,7 @@ public abstract class HttpCommonAttributesExtractor @Nullable private String userAgent(REQUEST request) { - List values = requestHeader(request, "user-agent"); - return values.isEmpty() ? null : values.get(0); + return firstHeaderValue(requestHeader(request, "user-agent")); } /** @@ -161,4 +160,9 @@ public abstract class HttpCommonAttributesExtractor * returned instead. */ protected abstract List responseHeader(REQUEST request, RESPONSE response, String name); + + @Nullable + static String firstHeaderValue(List values) { + return values.isEmpty() ? null : values.get(0); + } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java index 104d01baac..7a409a597d 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java @@ -5,11 +5,13 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; +import static io.opentelemetry.instrumentation.api.instrumenter.http.ForwarderHeaderParser.extractForwarded; +import static io.opentelemetry.instrumentation.api.instrumenter.http.ForwarderHeaderParser.extractForwardedFor; + import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; -import java.util.List; import org.checkerframework.checker.nullness.qual.Nullable; /** @@ -43,6 +45,7 @@ public abstract class HttpServerAttributesExtractor set(attributes, SemanticAttributes.HTTP_HOST, host(request)); set(attributes, SemanticAttributes.HTTP_TARGET, target(request)); set(attributes, SemanticAttributes.HTTP_ROUTE, route(request)); + set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request)); } @Override @@ -66,8 +69,7 @@ public abstract class HttpServerAttributesExtractor @Nullable private String host(REQUEST request) { - List values = requestHeader(request, "host"); - return values.isEmpty() ? null : values.get(0); + return firstHeaderValue(requestHeader(request, "host")); } @Nullable @@ -76,6 +78,26 @@ public abstract class HttpServerAttributesExtractor @Nullable protected abstract String scheme(REQUEST request); + @Nullable + private String clientIp(REQUEST request) { + // try Forwarded + String forwarded = firstHeaderValue(requestHeader(request, "forwarded")); + if (forwarded != null) { + forwarded = extractForwarded(forwarded); + if (forwarded != null) { + return forwarded; + } + } + + // try X-Forwarded-For + forwarded = firstHeaderValue(requestHeader(request, "x-forwarded-for")); + if (forwarded != null) { + return extractForwardedFor(forwarded); + } + + return null; + } + // Attributes which are not always available when the request is ready. /** diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java index 5d432fd474..788b4b375c 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java @@ -60,8 +60,7 @@ class InstrumenterTest { entry("req2_2", "req2_2_value"), entry("req3", "req3_value"), entry("linkTraceId", LINK_TRACE_ID), - entry("linkSpanId", LINK_SPAN_ID), - entry("Forwarded", "for=1.1.1.1")) + entry("linkSpanId", LINK_SPAN_ID)) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); private static final Map RESPONSE = @@ -297,49 +296,8 @@ class InstrumenterTest { .hasAttributesSatisfying( attributes -> assertThat(attributes) - .containsEntry(SemanticAttributes.NET_PEER_IP, "2.2.2.2") .containsEntry( - SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1")))); - } - - @Test - void server_http_xForwardedFor() { - Instrumenter, Map> instrumenter = - Instrumenter., Map>newBuilder( - otelTesting.getOpenTelemetry(), "test", unused -> "span") - .addAttributesExtractors( - mockHttpServerAttributes, - new ConstantNetPeerIpExtractor<>("2.2.2.2"), - new AttributesExtractor1(), - new AttributesExtractor2()) - .addSpanLinksExtractor(new LinksExtractor()) - .newServerInstrumenter(new MapGetter()); - - Map request = new HashMap<>(REQUEST); - request.remove("Forwarded"); - request.put("X-Forwarded-For", "1.1.1.1"); - - Context context = instrumenter.start(Context.root(), request); - SpanContext spanContext = Span.fromContext(context).getSpanContext(); - - assertThat(spanContext.isValid()).isTrue(); - assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext); - - instrumenter.end(context, request, RESPONSE, null); - - otelTesting - .assertTraces() - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - span -> - span.hasName("span") - .hasAttributesSatisfying( - attributes -> - assertThat(attributes) - .containsEntry(SemanticAttributes.NET_PEER_IP, "2.2.2.2") - .containsEntry( - SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1")))); + SemanticAttributes.NET_PEER_IP, "2.2.2.2")))); } @Test @@ -517,220 +475,6 @@ class InstrumenterTest { assertThat(context.get(testKey)).isEqualTo("testVal"); } - @Test - void extractForwarded() { - assertThat(ServerInstrumenter.extractForwarded("for=1.1.1.1")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedIpv6() { - assertThat( - ServerInstrumenter.extractForwarded( - "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\"")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedWithPort() { - assertThat(ServerInstrumenter.extractForwarded("for=\"1.1.1.1:2222\"")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedIpv6WithPort() { - assertThat( - ServerInstrumenter.extractForwarded( - "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\"")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedCaps() { - assertThat(ServerInstrumenter.extractForwarded("For=1.1.1.1")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedMalformed() { - assertThat(ServerInstrumenter.extractForwarded("for=;for=1.1.1.1")).isNull(); - } - - @Test - void extractForwardedEmpty() { - assertThat(ServerInstrumenter.extractForwarded("")).isNull(); - } - - @Test - void extractForwardedEmptyValue() { - assertThat(ServerInstrumenter.extractForwarded("for=")).isNull(); - } - - @Test - void extractForwardedEmptyValueWithSemicolon() { - assertThat(ServerInstrumenter.extractForwarded("for=;")).isNull(); - } - - @Test - void extractForwardedNoFor() { - assertThat(ServerInstrumenter.extractForwarded("by=1.1.1.1;test=1.1.1.1")).isNull(); - } - - @Test - void extractForwardedMultiple() { - assertThat(ServerInstrumenter.extractForwarded("for=1.1.1.1;for=1.2.3.4")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedMultipleIpV6() { - assertThat( - ServerInstrumenter.extractForwarded( - "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedMultipleWithPort() { - assertThat(ServerInstrumenter.extractForwarded("for=\"1.1.1.1:2222\";for=1.2.3.4")) - .isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedMultipleIpV6WithPort() { - assertThat( - ServerInstrumenter.extractForwarded( - "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedMixedSplitter() { - assertThat( - ServerInstrumenter.extractForwarded("test=abcd; by=1.2.3.4, for=1.1.1.1;for=1.2.3.4")) - .isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedMixedSplitterIpv6() { - assertThat( - ServerInstrumenter.extractForwarded( - "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedMixedSplitterWithPort() { - assertThat( - ServerInstrumenter.extractForwarded( - "test=abcd; by=1.2.3.4, for=\"1.1.1.1:2222\";for=1.2.3.4")) - .isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedMixedSplitterIpv6WithPort() { - assertThat( - ServerInstrumenter.extractForwarded( - "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedFor() { - assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedForIpv6() { - assertThat( - ServerInstrumenter.extractForwardedFor("\"[1111:1111:1111:1111:1111:1111:1111:1111]\"")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForIpv6Unquoted() { - assertThat(ServerInstrumenter.extractForwardedFor("[1111:1111:1111:1111:1111:1111:1111:1111]")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForIpv6Unbracketed() { - assertThat(ServerInstrumenter.extractForwardedFor("1111:1111:1111:1111:1111:1111:1111:1111")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForWithPort() { - assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1:2222")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedForIpv6WithPort() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\"")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForIpv6UnquotedWithPort() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "[1111:1111:1111:1111:1111:1111:1111:1111]:2222")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForEmpty() { - assertThat(ServerInstrumenter.extractForwardedFor("")).isNull(); - } - - @Test - void extractForwardedForMultiple() { - assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1,1.2.3.4")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedForMultipleIpv6() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "\"[1111:1111:1111:1111:1111:1111:1111:1111]\",1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForMultipleIpv6Unquoted() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "[1111:1111:1111:1111:1111:1111:1111:1111],1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForMultipleIpv6Unbracketed() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "1111:1111:1111:1111:1111:1111:1111:1111,1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForMultipleWithPort() { - assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1:2222,1.2.3.4")).isEqualTo("1.1.1.1"); - } - - @Test - void extractForwardedForMultipleIpv6WithPort() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\",1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - - @Test - void extractForwardedForMultipleIpv6UnquotedWithPort() { - assertThat( - ServerInstrumenter.extractForwardedFor( - "[1111:1111:1111:1111:1111:1111:1111:1111]:2222,1.2.3.4")) - .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); - } - @Test void clientNestedSpansSuppressed_whenInstrumentationTypeDisabled() { // this test depends on default config option for InstrumentationType diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParserTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParserTest.java new file mode 100644 index 0000000000..3d37637bac --- /dev/null +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParserTest.java @@ -0,0 +1,232 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.http; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class ForwarderHeaderParserTest { + + @Test + void extractForwarded() { + assertThat(ForwarderHeaderParser.extractForwarded("for=1.1.1.1")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedIpv6() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\"")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedWithPort() { + assertThat(ForwarderHeaderParser.extractForwarded("for=\"1.1.1.1:2222\"")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedIpv6WithPort() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\"")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedCaps() { + assertThat(ForwarderHeaderParser.extractForwarded("For=1.1.1.1")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedMalformed() { + assertThat(ForwarderHeaderParser.extractForwarded("for=;for=1.1.1.1")).isNull(); + } + + @Test + void extractForwardedEmpty() { + assertThat(ForwarderHeaderParser.extractForwarded("")).isNull(); + } + + @Test + void extractForwardedEmptyValue() { + assertThat(ForwarderHeaderParser.extractForwarded("for=")).isNull(); + } + + @Test + void extractForwardedEmptyValueWithSemicolon() { + assertThat(ForwarderHeaderParser.extractForwarded("for=;")).isNull(); + } + + @Test + void extractForwardedNoFor() { + assertThat(ForwarderHeaderParser.extractForwarded("by=1.1.1.1;test=1.1.1.1")).isNull(); + } + + @Test + void extractForwardedMultiple() { + assertThat(ForwarderHeaderParser.extractForwarded("for=1.1.1.1;for=1.2.3.4")) + .isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedMultipleIpV6() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedMultipleWithPort() { + assertThat(ForwarderHeaderParser.extractForwarded("for=\"1.1.1.1:2222\";for=1.2.3.4")) + .isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedMultipleIpV6WithPort() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedMixedSplitter() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "test=abcd; by=1.2.3.4, for=1.1.1.1;for=1.2.3.4")) + .isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedMixedSplitterIpv6() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedMixedSplitterWithPort() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "test=abcd; by=1.2.3.4, for=\"1.1.1.1:2222\";for=1.2.3.4")) + .isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedMixedSplitterIpv6WithPort() { + assertThat( + ForwarderHeaderParser.extractForwarded( + "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedFor() { + assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedForIpv6() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "\"[1111:1111:1111:1111:1111:1111:1111:1111]\"")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForIpv6Unquoted() { + assertThat( + ForwarderHeaderParser.extractForwardedFor("[1111:1111:1111:1111:1111:1111:1111:1111]")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForIpv6Unbracketed() { + assertThat(ForwarderHeaderParser.extractForwardedFor("1111:1111:1111:1111:1111:1111:1111:1111")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForWithPort() { + assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1:2222")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedForIpv6WithPort() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\"")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForIpv6UnquotedWithPort() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "[1111:1111:1111:1111:1111:1111:1111:1111]:2222")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForEmpty() { + assertThat(ForwarderHeaderParser.extractForwardedFor("")).isNull(); + } + + @Test + void extractForwardedForMultiple() { + assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1,1.2.3.4")).isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedForMultipleIpv6() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "\"[1111:1111:1111:1111:1111:1111:1111:1111]\",1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForMultipleIpv6Unquoted() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "[1111:1111:1111:1111:1111:1111:1111:1111],1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForMultipleIpv6Unbracketed() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "1111:1111:1111:1111:1111:1111:1111:1111,1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForMultipleWithPort() { + assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1:2222,1.2.3.4")) + .isEqualTo("1.1.1.1"); + } + + @Test + void extractForwardedForMultipleIpv6WithPort() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\",1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } + + @Test + void extractForwardedForMultipleIpv6UnquotedWithPort() { + assertThat( + ForwarderHeaderParser.extractForwardedFor( + "[1111:1111:1111:1111:1111:1111:1111:1111]:2222,1.2.3.4")) + .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111"); + } +} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java index 7b9ab8ecbc..77039ed647 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java @@ -7,6 +7,7 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.entry; @@ -55,23 +56,27 @@ class HttpServerAttributesExtractorTest { @Override protected List requestHeader(Map request, String name) { - return asList(request.get("header." + name).split(",")); + String values = request.get("header." + name); + return values == null ? emptyList() : asList(values.split(",")); } @Override protected Long requestContentLength(Map request, Map response) { - return Long.parseLong(request.get("requestContentLength")); + String value = request.get("requestContentLength"); + return value == null ? null : Long.parseLong(value); } @Override protected Long requestContentLengthUncompressed( Map request, Map response) { - return Long.parseLong(request.get("requestContentLengthUncompressed")); + String value = request.get("requestContentLengthUncompressed"); + return value == null ? null : Long.parseLong(value); } @Override protected Integer statusCode(Map request, Map response) { - return Integer.parseInt(response.get("statusCode")); + String value = response.get("statusCode"); + return value == null ? null : Integer.parseInt(value); } @Override @@ -82,19 +87,22 @@ class HttpServerAttributesExtractorTest { @Override protected Long responseContentLength( Map request, Map response) { - return Long.parseLong(response.get("responseContentLength")); + String value = response.get("responseContentLength"); + return value == null ? null : Long.parseLong(value); } @Override protected Long responseContentLengthUncompressed( Map request, Map response) { - return Long.parseLong(response.get("responseContentLengthUncompressed")); + String value = response.get("responseContentLengthUncompressed"); + return value == null ? null : Long.parseLong(value); } @Override protected List responseHeader( Map request, Map response, String name) { - return asList(response.get("header." + name).split(",")); + String values = response.get("header." + name); + return values == null ? emptyList() : asList(values.split(",")); } } @@ -112,6 +120,7 @@ class HttpServerAttributesExtractorTest { request.put("serverName", "server"); request.put("header.user-agent", "okhttp 3.x"); request.put("header.host", "github.com"); + request.put("header.forwarded", "for=1.1.1.1"); request.put("header.custom-request-header", "123,456"); Map response = new HashMap<>(); @@ -136,6 +145,7 @@ class HttpServerAttributesExtractorTest { entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"), entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"), entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"), + entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"), entry( AttributeKey.stringArrayKey("http.request.header.custom_request_header"), asList("123", "456"))); @@ -149,6 +159,7 @@ class HttpServerAttributesExtractorTest { entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"), entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"), entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"), + entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"), entry( AttributeKey.stringArrayKey("http.request.header.custom_request_header"), asList("123", "456")), @@ -163,4 +174,22 @@ class HttpServerAttributesExtractorTest { AttributeKey.stringArrayKey("http.response.header.custom_response_header"), asList("654", "321"))); } + + @Test + void extractClientIpFromX_Forwarded_For() { + Map request = new HashMap<>(); + request.put("header.x-forwarded-for", "1.1.1.1"); + + TestHttpServerAttributesExtractor extractor = + new TestHttpServerAttributesExtractor(CapturedHttpHeaders.empty()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, request); + assertThat(attributes.build()) + .containsOnly(entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1")); + + extractor.onEnd(attributes, request, null, null); + assertThat(attributes.build()) + .containsOnly(entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1")); + } }