From 6dad9426c16cc5a2e95247d8e170fab07baa862e Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Tue, 18 Jul 2023 13:18:37 +0200 Subject: Make headers controlling client address connector specific --- container-core/abi-spec.json | 90 +++++++++++----------- .../http/server/jetty/AccessLogRequestLog.java | 17 ++-- .../jdisc/http/server/jetty/JettyHttpServer.java | 2 +- .../jdisc.http.jdisc.http.connector.def | 6 ++ .../jdisc.http.jdisc.http.server.def | 6 -- .../http/server/jetty/AccessLogRequestLogTest.java | 7 +- .../http/server/jetty/JettyMockRequestBuilder.java | 6 +- 7 files changed, 64 insertions(+), 70 deletions(-) (limited to 'container-core') diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index 757afeb64e2..6d7e3c86351 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -1027,6 +1027,45 @@ ], "fields" : [ ] }, + "com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder" : { + "superClass" : "java.lang.Object", + "interfaces" : [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes" : [ + "public", + "final" + ], + "methods" : [ + "public void ()", + "public void (com.yahoo.jdisc.http.ConnectorConfig$AccessLog)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remoteAddressHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remoteAddressHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remotePortHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder remotePortHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog build()" + ], + "fields" : [ + "public java.util.List remoteAddressHeaders", + "public java.util.List remotePortHeaders" + ] + }, + "com.yahoo.jdisc.http.ConnectorConfig$AccessLog" : { + "superClass" : "com.yahoo.config.InnerNode", + "interfaces" : [ ], + "attributes" : [ + "public", + "final" + ], + "methods" : [ + "public void (com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder)", + "public java.util.List remoteAddressHeaders()", + "public java.lang.String remoteAddressHeaders(int)", + "public java.util.List remotePortHeaders()", + "public java.lang.String remotePortHeaders(int)" + ], + "fields" : [ ] + }, "com.yahoo.jdisc.http.ConnectorConfig$Builder" : { "superClass" : "java.lang.Object", "interfaces" : [ @@ -1069,6 +1108,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder http2(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder serverName(com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder serverName(java.util.function.Consumer)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder accessLog(com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder accessLog(java.util.function.Consumer)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -1084,7 +1125,8 @@ "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder healthCheckProxy", "public com.yahoo.jdisc.http.ConnectorConfig$ProxyProtocol$Builder proxyProtocol", "public com.yahoo.jdisc.http.ConnectorConfig$Http2$Builder http2", - "public com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder serverName" + "public com.yahoo.jdisc.http.ConnectorConfig$ServerName$Builder serverName", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog$Builder accessLog" ] }, "com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder" : { @@ -1438,7 +1480,8 @@ "public double maxConnectionLife()", "public boolean http2Enabled()", "public com.yahoo.jdisc.http.ConnectorConfig$Http2 http2()", - "public com.yahoo.jdisc.http.ConnectorConfig$ServerName serverName()" + "public com.yahoo.jdisc.http.ConnectorConfig$ServerName serverName()", + "public com.yahoo.jdisc.http.ConnectorConfig$AccessLog accessLog()" ], "fields" : [ "public static final java.lang.String CONFIG_DEF_MD5", @@ -1771,45 +1814,6 @@ ], "fields" : [ ] }, - "com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder" : { - "superClass" : "java.lang.Object", - "interfaces" : [ - "com.yahoo.config.ConfigBuilder" - ], - "attributes" : [ - "public", - "final" - ], - "methods" : [ - "public void ()", - "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.lang.String)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.util.Collection)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.lang.String)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.util.Collection)", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog build()" - ], - "fields" : [ - "public java.util.List remoteAddressHeaders", - "public java.util.List remotePortHeaders" - ] - }, - "com.yahoo.jdisc.http.ServerConfig$AccessLog" : { - "superClass" : "com.yahoo.config.InnerNode", - "interfaces" : [ ], - "attributes" : [ - "public", - "final" - ], - "methods" : [ - "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", - "public java.util.List remoteAddressHeaders()", - "public java.lang.String remoteAddressHeaders(int)", - "public java.util.List remotePortHeaders()", - "public java.lang.String remotePortHeaders(int)" - ], - "fields" : [ ] - }, "com.yahoo.jdisc.http.ServerConfig$Builder" : { "superClass" : "java.lang.Object", "interfaces" : [ @@ -1839,8 +1843,6 @@ "public com.yahoo.jdisc.http.ServerConfig$Builder jmx(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ServerConfig$Builder metric(com.yahoo.jdisc.http.ServerConfig$Metric$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder metric(java.util.function.Consumer)", - "public com.yahoo.jdisc.http.ServerConfig$Builder accessLog(com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", - "public com.yahoo.jdisc.http.ServerConfig$Builder accessLog(java.util.function.Consumer)", "public com.yahoo.jdisc.http.ServerConfig$Builder connectionLog(com.yahoo.jdisc.http.ServerConfig$ConnectionLog$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder connectionLog(java.util.function.Consumer)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", @@ -1856,7 +1858,6 @@ "public java.util.List defaultFilters", "public com.yahoo.jdisc.http.ServerConfig$Jmx$Builder jmx", "public com.yahoo.jdisc.http.ServerConfig$Metric$Builder metric", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder accessLog", "public com.yahoo.jdisc.http.ServerConfig$ConnectionLog$Builder connectionLog" ] }, @@ -2070,7 +2071,6 @@ "public double stopTimeout()", "public com.yahoo.jdisc.http.ServerConfig$Jmx jmx()", "public com.yahoo.jdisc.http.ServerConfig$Metric metric()", - "public com.yahoo.jdisc.http.ServerConfig$AccessLog accessLog()", "public com.yahoo.jdisc.http.ServerConfig$ConnectionLog connectionLog()" ], "fields" : [ diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index 5b51eeee7d6..7a305c23ba3 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -7,8 +7,6 @@ import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; import com.yahoo.jdisc.http.HttpRequest; -import com.yahoo.jdisc.http.ServerConfig; -import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.server.HttpTransportOverHTTP2; import org.eclipse.jetty.server.HttpChannel; @@ -27,6 +25,7 @@ import java.util.function.BiConsumer; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; /** @@ -44,13 +43,9 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty private static final List LOGGED_REQUEST_HEADERS = List.of("Vespa-Client-Version"); private final RequestLog requestLog; - private final List remoteAddressHeaders; - private final List remotePortHeaders; - AccessLogRequestLog(RequestLog requestLog, ServerConfig.AccessLog config) { + AccessLogRequestLog(RequestLog requestLog) { this.requestLog = requestLog; - this.remoteAddressHeaders = config.remoteAddressHeaders(); - this.remotePortHeaders = config.remotePortHeaders(); } @Override @@ -144,16 +139,16 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty } } - private String getRemoteAddress(HttpServletRequest request) { - for (String header : remoteAddressHeaders) { + private String getRemoteAddress(Request request) { + for (String header : getConnector(request).connectorConfig().accessLog().remoteAddressHeaders()) { String value = request.getHeader(header); if (value != null) return value; } return request.getRemoteAddr(); } - private int getRemotePort(HttpServletRequest request) { - for (String header : remotePortHeaders) { + private int getRemotePort(Request request) { + for (String header : getConnector(request).connectorConfig().accessLog().remotePortHeaders()) { String value = request.getHeader(header); if (value != null) { OptionalInt maybePort = parsePort(value); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 3ebb65e7979..7d84ee6f8a3 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -68,7 +68,7 @@ public class JettyHttpServer extends AbstractServerProvider { server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); - server.setRequestLog(new AccessLogRequestLog(requestLog, serverConfig.accessLog())); + server.setRequestLog(new AccessLogRequestLog(requestLog)); setupJmx(server, serverConfig); configureJettyThreadpool(server, serverConfig); JettyConnectionLogger connectionLogger = new JettyConnectionLogger(serverConfig.connectionLog(), connectionLog); diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def index 3c01012fd9e..5a2bad63682 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def @@ -138,3 +138,9 @@ serverName.fallback string default="" # The list of accepted server names. Empty list to accept any. Elements follows format of 'serverName.default'. serverName.allowed[] string + +# HTTP request headers that contain remote address +accessLog.remoteAddressHeaders[] string + +# HTTP request headers that contain remote port +accessLog.remotePortHeaders[] string diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index c15cb6b2cc4..a85641f61e9 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -52,11 +52,5 @@ metric.searchHandlerPaths[] string # User-agent names to ignore wrt statistics (crawlers etc) metric.ignoredUserAgents[] string -# HTTP request headers that contain remote address -accessLog.remoteAddressHeaders[] string - -# HTTP request headers that contain remote port -accessLog.remotePortHeaders[] string - # Whether to enable jdisc connection log connectionLog.enabled bool default=false diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java index 766c7918882..122db0f765d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java @@ -4,7 +4,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; -import com.yahoo.jdisc.http.ServerConfig; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.junit.jupiter.api.Test; @@ -117,11 +116,7 @@ public class AccessLogRequestLogTest { } private void doAccessLoggingOfRequest(RequestLog requestLog, Request jettyRequest) { - ServerConfig.AccessLog config = new ServerConfig.AccessLog( - new ServerConfig.AccessLog.Builder() - .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra")) - .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))); - new AccessLogRequestLog(requestLog, config).log(jettyRequest, createResponseMock()); + new AccessLogRequestLog(requestLog).log(jettyRequest, createResponseMock()); } private static JettyMockRequestBuilder createRequestBuilder() { diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java index e62825fc2a8..8b13f30bcd7 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java @@ -85,7 +85,11 @@ public class JettyMockRequestBuilder { HttpChannel channel = mock(HttpChannel.class); HttpConnection connection = mock(HttpConnection.class); JDiscServerConnector connector = mock(JDiscServerConnector.class); - when(connector.connectorConfig()).thenReturn(new ConnectorConfig(new ConnectorConfig.Builder().listenPort(localPort))); + when(connector.connectorConfig()).thenReturn(new ConnectorConfig( + new ConnectorConfig.Builder().listenPort(localPort) + .accessLog(new ConnectorConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra")) + .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))))); when(connector.getLocalPort()).thenReturn(localPort); when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); when(connection.getConnector()).thenReturn(connector); -- cgit v1.2.3