Factor in X-Forwarded-Host / Forwarded when capturing server.address …

…and server.port
open-telemetry · Oct 19, 2023 · 2732c3b · 2732c3b
1 parent e1efa2d
commit 2732c3b
Show file tree

Hide file tree

Showing 9 changed files with 328 additions and 55 deletions.
diff --git a/...ttp/ForwardedAddressAndPortExtractor.java → .../ForwardedForAddressAndPortExtractor.java b/...ttp/ForwardedAddressAndPortExtractor.java → .../ForwardedForAddressAndPortExtractor.java
@@ -5,14 +5,18 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.notFound;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
+
 import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
 import java.util.Locale;
 
-final class ForwardedAddressAndPortExtractor<REQUEST> implements AddressAndPortExtractor<REQUEST> {
+final class ForwardedForAddressAndPortExtractor<REQUEST>
+    implements AddressAndPortExtractor<REQUEST> {
 
   private final HttpServerAttributesGetter<REQUEST, ?> getter;
 
-  ForwardedAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
+  ForwardedForAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
     this.getter = getter;
   }
 
@@ -38,7 +42,7 @@ private static boolean extractFromForwardedHeader(AddressPortSink sink, String f
     if (start < 0) {
       return false;
     }
-    start += 4; // start is now the index after for=
+    start += "for=".length(); // start is now the index after for=
     if (start >= forwarded.length() - 1) { // the value after for= must not be empty
       return false;
     }
@@ -132,26 +136,11 @@ private static boolean extractClientInfo(
     return true;
   }
 
-  private static boolean notFound(int pos, int end) {
-    return pos < 0 || pos >= end;
-  }
-
   private static int findPortEnd(String header, int start, int end) {
     int numberEnd = start;
     while (numberEnd < end && Character.isDigit(header.charAt(numberEnd))) {
       ++numberEnd;
     }
     return numberEnd;
   }
-
-  private static void setPort(AddressPortSink sink, String header, int start, int end) {
-    if (start == end) {
-      return;
-    }
-    try {
-      sink.setPort(Integer.parseInt(header.substring(start, end)));
-    } catch (NumberFormatException ignored) {
-      // malformed port, ignoring
-    }
-  }
 }
diff --git a/...telemetry/instrumentation/api/instrumenter/http/ForwardedHostAddressAndPortExtractor.java b/...telemetry/instrumentation/api/instrumenter/http/ForwardedHostAddressAndPortExtractor.java
@@ -0,0 +1,90 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.notFound;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
+
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
+import java.util.Locale;
+
+final class ForwardedHostAddressAndPortExtractor<REQUEST>
+    implements AddressAndPortExtractor<REQUEST> {
+
+  private final HttpCommonAttributesGetter<REQUEST, ?> getter;
+
+  ForwardedHostAddressAndPortExtractor(HttpCommonAttributesGetter<REQUEST, ?> getter) {
+    this.getter = getter;
+  }
+
+  @Override
+  public void extract(AddressPortSink sink, REQUEST request) {
+    // try Forwarded
+    for (String forwarded : getter.getHttpRequestHeader(request, "forwarded")) {
+      if (extractFromForwardedHeader(sink, forwarded)) {
+        return;
+      }
+    }
+
+    // try X-Forwarded-Host
+    for (String forwardedHost : getter.getHttpRequestHeader(request, "x-forwarded-host")) {
+      if (extractHost(sink, forwardedHost, 0, forwardedHost.length())) {
+        return;
+      }
+    }
+
+    // try Host
+    for (String host : getter.getHttpRequestHeader(request, "host")) {
+      if (extractHost(sink, host, 0, host.length())) {
+        return;
+      }
+    }
+  }
+
+  private static boolean extractFromForwardedHeader(AddressPortSink sink, String forwarded) {
+    int start = forwarded.toLowerCase(Locale.ROOT).indexOf("host=");
+    if (start < 0) {
+      return false;
+    }
+    start += "host=".length(); // start is now the index after host=
+    if (start >= forwarded.length() - 1) { // the value after host= must not be empty
+      return false;
+    }
+    // find the end of the `host=<address>` section
+    int end = forwarded.indexOf(';', start);
+    if (end < 0) {
+      end = forwarded.length();
+    }
+    return extractHost(sink, forwarded, start, end);
+  }
+
+  private static boolean extractHost(AddressPortSink sink, String host, int start, int end) {
+    if (start >= end) {
+      return false;
+    }
+
+    // skip quotes
+    if (host.charAt(start) == '"') {
+      // try to find the end of the quote
+      int quoteEnd = host.indexOf('"', start + 1);
+      if (notFound(quoteEnd, end)) {
+        // malformed header value
+        return false;
+      }
+      return extractHost(sink, host, start + 1, quoteEnd);
+    }
+
+    int hostHeaderSeparator = host.indexOf(':', start);
+    if (notFound(hostHeaderSeparator, end)) {
+      sink.setAddress(host.substring(start, end));
+    } else {
+      sink.setAddress(host.substring(start, hostHeaderSeparator));
+      setPort(sink, host, hostHeaderSeparator + 1, end);
+    }
+
+    return true;
+  }
+}
diff --git a/...main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HeaderParsingHelper.java b/...main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HeaderParsingHelper.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor.AddressPortSink;
+
+final class HeaderParsingHelper {
+
+  static boolean notFound(int pos, int end) {
+    return pos < 0 || pos >= end;
+  }
+
+  static void setPort(AddressPortSink sink, String header, int start, int end) {
+    if (start == end) {
+      return;
+    }
+    try {
+      sink.setPort(Integer.parseInt(header.substring(start, end)));
+    } catch (NumberFormatException ignored) {
+      // malformed port, ignoring
+    }
+  }
+
+  private HeaderParsingHelper() {}
+}
diff --git a/...a/io/opentelemetry/instrumentation/api/instrumenter/http/HostAddressAndPortExtractor.java b/...a/io/opentelemetry/instrumentation/api/instrumenter/http/HostAddressAndPortExtractor.java
@@ -5,16 +5,13 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpCommonAttributesExtractor.firstHeaderValue;
-import static java.util.logging.Level.FINE;
 
 import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
-import java.util.logging.Logger;
 
 final class HostAddressAndPortExtractor<REQUEST> implements AddressAndPortExtractor<REQUEST> {
 
-  private static final Logger logger = Logger.getLogger(HttpCommonAttributesGetter.class.getName());
-
   private final HttpCommonAttributesGetter<REQUEST, ?> getter;
 
   HostAddressAndPortExtractor(HttpCommonAttributesGetter<REQUEST, ?> getter) {
@@ -31,14 +28,9 @@ public void extract(AddressPortSink sink, REQUEST request) {
     int hostHeaderSeparator = host.indexOf(':');
     if (hostHeaderSeparator == -1) {
       sink.setAddress(host);
-      return;
-    }
-
-    sink.setAddress(host.substring(0, hostHeaderSeparator));
-    try {
-      sink.setPort(Integer.parseInt(host.substring(hostHeaderSeparator + 1)));
-    } catch (NumberFormatException e) {
-      logger.log(FINE, e.getMessage(), e);
+    } else {
+      sink.setAddress(host.substring(0, hostHeaderSeparator));
+      setPort(sink, host, hostHeaderSeparator + 1, host.length());
     }
   }
 }
diff --git a/...telemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java b/...telemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java
@@ -54,10 +54,10 @@ public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {
 
     clientAddressPortExtractor =
         new ClientAddressAndPortExtractor<>(
-            netAttributesGetter, new ForwardedAddressAndPortExtractor<>(httpAttributesGetter));
+            netAttributesGetter, new ForwardedForAddressAndPortExtractor<>(httpAttributesGetter));
     serverAddressPortExtractor =
         new ServerAddressAndPortExtractor<>(
-            netAttributesGetter, new HostAddressAndPortExtractor<>(httpAttributesGetter));
+            netAttributesGetter, new ForwardedHostAddressAndPortExtractor<>(httpAttributesGetter));
   }
 
   /**

diff --git a/...va/io/opentelemetry/instrumentation/api/instrumenter/network/internal/AddressAndPort.java b/...va/io/opentelemetry/instrumentation/api/instrumenter/network/internal/AddressAndPort.java
@@ -30,4 +30,9 @@ public void setPort(Integer port) {
   public String getAddress() {
     return address;
   }
+
+  @Nullable
+  public Integer getPort() {
+    return port;
+  }
 }
diff --git a/...ForwardedAddressAndPortExtractorTest.java → ...wardedForAddressAndPortExtractorTest.java b/...ForwardedAddressAndPortExtractorTest.java → ...wardedForAddressAndPortExtractorTest.java
@@ -12,7 +12,7 @@
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.Mockito.doReturn;
 
-import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPort;
 import java.util.List;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
@@ -27,11 +27,11 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
-class ForwardedAddressAndPortExtractorTest {
+class ForwardedForAddressAndPortExtractorTest {
 
   @Mock HttpServerAttributesGetter<String, String> getter;
 
-  @InjectMocks ForwardedAddressAndPortExtractor<String> underTest;
+  @InjectMocks ForwardedForAddressAndPortExtractor<String> underTest;
 
   @ParameterizedTest
   @ArgumentsSource(ForwardedArgs.class)
@@ -42,8 +42,8 @@ void shouldParseForwarded(
     AddressAndPort sink = new AddressAndPort();
     underTest.extract(sink, "request");
 
-    assertThat(sink.address).isEqualTo(expectedAddress);
-    assertThat(sink.port).isEqualTo(expectedPort);
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
   }
 
   static final class ForwardedArgs implements ArgumentsProvider {
@@ -100,8 +100,8 @@ void shouldParseForwardedFor(
     AddressAndPort sink = new AddressAndPort();
     underTest.extract(sink, "request");
 
-    assertThat(sink.address).isEqualTo(expectedAddress);
-    assertThat(sink.port).isEqualTo(expectedPort);
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
   }
 
   static final class ForwardedForArgs implements ArgumentsProvider {
@@ -147,20 +147,4 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext extensionCo
           arguments(asList("1.2.3.4", "::1"), "1.2.3.4", null));
     }
   }
-
-  static final class AddressAndPort implements AddressAndPortExtractor.AddressPortSink {
-
-    @Nullable String address = null;
-    @Nullable Integer port = null;
-
-    @Override
-    public void setAddress(String address) {
-      this.address = address;
-    }
-
-    @Override
-    public void setPort(Integer port) {
-      this.port = port;
-    }
-  }
 }