From d63e3d7e5b5d31fd485fc710c97d674904f13754 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 12 Apr 2021 12:21:45 +0200 Subject: Revert "Revert "Revert "Bjorncs/jdisc http2 preps [run-systemtest]""" --- cloud-tenant-base-dependencies-enforcer/pom.xml | 7 - container-core/abi-spec.json | 7 +- container-core/pom.xml | 15 +- .../java/com/yahoo/jdisc/http/HttpRequest.java | 3 +- .../http/server/jetty/AccessLogRequestLog.java | 2 +- .../jdisc/http/server/jetty/ConnectorFactory.java | 92 ++--------- .../jdisc/http/server/jetty/FilterResolver.java | 32 ++-- .../http/server/jetty/FilteringRequestHandler.java | 11 +- .../http/server/jetty/HealthCheckProxyHandler.java | 4 +- .../http/server/jetty/HttpRequestDispatch.java | 31 ++-- .../http/server/jetty/HttpRequestFactory.java | 9 +- .../http/server/jetty/HttpServletRequestUtils.java | 38 +++++ .../server/jetty/JDiscFilterInvokerFilter.java | 9 +- .../jdisc/http/server/jetty/JDiscHttpServlet.java | 9 +- .../jdisc/http/server/jetty/RequestUtils.java | 51 ------ .../http/server/jetty/SecuredRedirectHandler.java | 4 +- .../jetty/TlsClientAuthenticationEnforcer.java | 10 +- .../yahoo/jdisc/http/servlet/ServletRequest.java | 5 +- .../jdisc.http.jdisc.http.connector.def | 3 - .../http/filter/ServletFilterRequestTest.java | 27 ++-- .../http/server/jetty/AccessLogRequestLogTest.java | 89 +++++++---- .../jdisc/http/server/jetty/FilterTestCase.java | 2 +- .../http/server/jetty/HttpRequestFactoryTest.java | 30 +++- .../jdisc/http/server/jetty/HttpServerTest.java | 108 ++++--------- .../http/server/jetty/JDiscHttpServletTest.java | 23 +-- .../http/server/jetty/JettyMockRequestBuilder.java | 176 --------------------- .../server/jetty/JettyMockResponseBuilder.java | 29 ---- .../jdisc/http/server/jetty/SimpleHttpClient.java | 83 ++++------ .../yahoo/jdisc/http/server/jetty/TestDrivers.java | 5 +- container-dependency-versions/pom.xml | 20 --- container-disc/pom.xml | 6 - dist/vespa.spec | 2 - jdisc_jetty/pom.xml | 12 -- parent/pom.xml | 5 + 34 files changed, 312 insertions(+), 647 deletions(-) create mode 100644 container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java delete mode 100644 container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java delete mode 100644 container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java delete mode 100644 container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 25350957a34..8a504635f25 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -31,7 +31,6 @@ 3.1.0 2.3.0 9.4.38.v20210224 - 1.1.3.v20160715 5.7.0 1.7.0 1.7.1 @@ -254,12 +253,6 @@ org.apache.opennlp:opennlp-tools:1.8.4:jar:test org.apiguardian:apiguardian-api:1.1.0:jar:test org.codehaus.woodstox:stax2-api:3.1.4:jar:test - org.eclipse.jetty.alpn:alpn-api:[${jetty-alpn.version}]:jar:test - org.eclipse.jetty.http2:http2-common:[${jetty.version}]:jar:test - org.eclipse.jetty.http2:http2-hpack:[${jetty.version}]:jar:test - org.eclipse.jetty.http2:http2-server:[${jetty.version}]:jar:test - org.eclipse.jetty:jetty-alpn-server:[${jetty.version}]:jar:test - org.eclipse.jetty:jetty-alpn-java-server:[${jetty.version}]:jar:test org.eclipse.jetty:jetty-continuation:[${jetty.version}]:jar:test org.eclipse.jetty:jetty-jmx:[${jetty.version}]:jar:test org.eclipse.jetty:jetty-security:[${jetty.version}]:jar:test diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index e333621df0e..da8ed609dfc 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -907,7 +907,6 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder secureRedirect(com.yahoo.jdisc.http.ConnectorConfig$SecureRedirect$Builder)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder maxRequestsPerConnection(int)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder maxConnectionLife(double)", - "public com.yahoo.jdisc.http.ConnectorConfig$Builder http2Enabled(boolean)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -1229,8 +1228,7 @@ "public com.yahoo.jdisc.http.ConnectorConfig$ProxyProtocol proxyProtocol()", "public com.yahoo.jdisc.http.ConnectorConfig$SecureRedirect secureRedirect()", "public int maxRequestsPerConnection()", - "public double maxConnectionLife()", - "public boolean http2Enabled()" + "public double maxConnectionLife()" ], "fields": [ "public static final java.lang.String CONFIG_DEF_MD5", @@ -1465,8 +1463,7 @@ ], "fields": [ "public static final enum com.yahoo.jdisc.http.HttpRequest$Version HTTP_1_0", - "public static final enum com.yahoo.jdisc.http.HttpRequest$Version HTTP_1_1", - "public static final enum com.yahoo.jdisc.http.HttpRequest$Version HTTP_2_0" + "public static final enum com.yahoo.jdisc.http.HttpRequest$Version HTTP_1_1" ] }, "com.yahoo.jdisc.http.HttpRequest": { diff --git a/container-core/pom.xml b/container-core/pom.xml index 8334dcf1f60..d0722a081d1 100644 --- a/container-core/pom.xml +++ b/container-core/pom.xml @@ -254,6 +254,11 @@ jaxb-api provided + + org.eclipse.jetty + jetty-servlet + provided + @@ -314,15 +319,9 @@ test - org.apache.httpcomponents.client5 - httpclient5 + org.springframework + spring-test test - - - org.slf4j - slf4j-api - - diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java index ea01d215ca5..118c34245c0 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java @@ -47,8 +47,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { public enum Version { HTTP_1_0("HTTP/1.0"), - HTTP_1_1("HTTP/1.1"), - HTTP_2_0("HTTP/2.0"); + HTTP_1_1("HTTP/1.1"); private final String str; 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 11898381f0a..2f9fc0d07b2 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 @@ -24,7 +24,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.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; /** * This class is a bridge between Jetty's {@link org.eclipse.jetty.server.handler.RequestLogHandler} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java index bc358e2fb06..d7ad12a5c64 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java @@ -7,8 +7,6 @@ import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ssl.SslContextFactoryProvider; import com.yahoo.security.tls.MixedMode; import com.yahoo.security.tls.TransportSecurityUtils; -import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; -import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.DetectorConnectionFactory; import org.eclipse.jetty.server.HttpConfiguration; @@ -20,7 +18,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.ssl.SslContextFactory; -import java.util.Collection; import java.util.List; /** @@ -79,72 +76,41 @@ public class ConnectorFactory { } private List createConnectionFactories(Metric metric) { + HttpConnectionFactory httpFactory = newHttpConnectionFactory(); if (!isSslEffectivelyEnabled(connectorConfig)) { - return List.of(newHttp1ConnectionFactory()); + return List.of(httpFactory); } else if (connectorConfig.ssl().enabled()) { - return connectionFactoriesForHttps(metric); + return connectionFactoriesForHttps(metric, httpFactory); } else if (TransportSecurityUtils.isTransportSecurityEnabled()) { switch (TransportSecurityUtils.getInsecureMixedMode()) { case TLS_CLIENT_MIXED_SERVER: case PLAINTEXT_CLIENT_MIXED_SERVER: - return connectionFactoriesForHttpsMixedMode(metric); + return List.of(new DetectorConnectionFactory(newSslConnectionFactory(metric, httpFactory)), httpFactory); case DISABLED: - return connectionFactoriesForHttps(metric); + return connectionFactoriesForHttps(metric, httpFactory); default: throw new IllegalStateException(); } } else { - return List.of(newHttp1ConnectionFactory()); + return List.of(httpFactory); } } - private List connectionFactoriesForHttps(Metric metric) { + private List connectionFactoriesForHttps(Metric metric, HttpConnectionFactory httpFactory) { ConnectorConfig.ProxyProtocol proxyProtocolConfig = connectorConfig.proxyProtocol(); - HttpConnectionFactory http1Factory = newHttp1ConnectionFactory(); - if (connectorConfig.http2Enabled()) { - HTTP2ServerConnectionFactory http2Factory = newHttp2ConnectionFactory(); - ALPNServerConnectionFactory alpnFactory = newAlpnConnectionFactory(List.of(http1Factory, http2Factory), http1Factory); - SslConnectionFactory sslFactory = newSslConnectionFactory(metric, alpnFactory); - if (proxyProtocolConfig.enabled()) { - if (proxyProtocolConfig.mixedMode()) { - ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(alpnFactory); - DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory, proxyProtocolFactory); - return List.of(detectorFactory, proxyProtocolFactory, sslFactory, alpnFactory, http1Factory, http2Factory); - } else { - ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(alpnFactory); - return List.of(proxyProtocolFactory, sslFactory, alpnFactory, http1Factory, http2Factory); - } + SslConnectionFactory sslFactory = newSslConnectionFactory(metric, httpFactory); + if (proxyProtocolConfig.enabled()) { + if (proxyProtocolConfig.mixedMode()) { + return List.of(new DetectorConnectionFactory(sslFactory, new ProxyConnectionFactory(sslFactory.getProtocol())), sslFactory, httpFactory); } else { - return List.of(sslFactory, alpnFactory, http1Factory, http2Factory); + return List.of(new ProxyConnectionFactory(sslFactory.getProtocol()), sslFactory, httpFactory); } } else { - SslConnectionFactory sslFactory = newSslConnectionFactory(metric, http1Factory); - if (proxyProtocolConfig.enabled()) { - if (proxyProtocolConfig.mixedMode()) { - ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(sslFactory); - DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory, proxyProtocolFactory); - return List.of(detectorFactory, proxyProtocolFactory, sslFactory, http1Factory); - } else { - ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(sslFactory); - return List.of(proxyProtocolFactory, sslFactory, http1Factory); - } - } else { - return List.of(sslFactory, http1Factory); - } + return List.of(sslFactory, httpFactory); } } - private List connectionFactoriesForHttpsMixedMode(Metric metric) { - // No support for proxy-protocol/http2 when using HTTP with TLS mixed mode - HttpConnectionFactory httpFactory = newHttp1ConnectionFactory(); - SslConnectionFactory sslFactory = newSslConnectionFactory(metric, httpFactory); - // Detector connection factory with single alternative will fallback to next protocol in list (httpFactory in this case) - // Cannot specify HttpConnectionFactory as alternative it does not implement ConnectionFactory.Detecting - DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory); - return List.of(detectorFactory, httpFactory, sslFactory); - } - - private HttpConfiguration newHttpConfiguration() { + private HttpConnectionFactory newHttpConnectionFactory() { HttpConfiguration httpConfig = new HttpConfiguration(); httpConfig.setSendDateHeader(true); httpConfig.setSendServerVersion(false); @@ -156,40 +122,16 @@ public class ConnectorFactory { if (isSslEffectivelyEnabled(connectorConfig)) { httpConfig.addCustomizer(new SecureRequestCustomizer()); } - return httpConfig; - } - - private HttpConnectionFactory newHttp1ConnectionFactory() { - return new HttpConnectionFactory(newHttpConfiguration()); + return new HttpConnectionFactory(httpConfig); } - private HTTP2ServerConnectionFactory newHttp2ConnectionFactory() { - return new HTTP2ServerConnectionFactory(newHttpConfiguration()); - } - - private SslConnectionFactory newSslConnectionFactory(Metric metric, ConnectionFactory wrappedFactory) { + private SslConnectionFactory newSslConnectionFactory(Metric metric, HttpConnectionFactory httpFactory) { SslContextFactory ctxFactory = sslContextFactoryProvider.getInstance(connectorConfig.name(), connectorConfig.listenPort()); - SslConnectionFactory connectionFactory = new SslConnectionFactory(ctxFactory, wrappedFactory.getProtocol()); + SslConnectionFactory connectionFactory = new SslConnectionFactory(ctxFactory, httpFactory.getProtocol()); connectionFactory.addBean(new SslHandshakeFailedListener(metric, connectorConfig.name(), connectorConfig.listenPort())); return connectionFactory; } - private ALPNServerConnectionFactory newAlpnConnectionFactory(Collection alternatives, - ConnectionFactory defaultFactory) { - String[] protocols = alternatives.stream().map(ConnectionFactory::getProtocol).toArray(String[]::new); - ALPNServerConnectionFactory factory = new ALPNServerConnectionFactory(protocols); - factory.setDefaultProtocol(defaultFactory.getProtocol()); - return factory; - } - - private DetectorConnectionFactory newDetectorConnectionFactory(ConnectionFactory.Detecting... alternatives) { - return new DetectorConnectionFactory(alternatives); - } - - private ProxyConnectionFactory newProxyProtocolConnectionFactory(ConnectionFactory wrappedFactory) { - return new ProxyConnectionFactory(wrappedFactory.getProtocol()); - } - private static boolean isSslEffectivelyEnabled(ConnectorConfig config) { return config.ssl().enabled() || (config.implicitTlsEnabled() && TransportSecurityUtils.isTransportSecurityEnabled()); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java index a9639ba4da7..1e2686aa184 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java @@ -11,13 +11,13 @@ import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.filter.RequestFilter; import com.yahoo.jdisc.http.filter.ResponseFilter; import com.yahoo.jdisc.http.servlet.ServletRequest; -import org.eclipse.jetty.server.Request; +import javax.servlet.http.HttpServletRequest; import java.net.URI; import java.util.Map; import java.util.Optional; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; +import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; /** * Resolve request/response filter (chain) based on {@link FilterBindings}. @@ -36,38 +36,38 @@ class FilterResolver { this.strictFiltering = strictFiltering; } - Optional resolveRequestFilter(Request request, URI jdiscUri) { - Optional maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(request).listenPort()); + Optional resolveRequestFilter(HttpServletRequest servletRequest, URI jdiscUri) { + Optional maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(servletRequest).listenPort()); if (maybeFilterId.isPresent()) { - metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); - request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); + servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); } else if (!strictFiltering) { - metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(request, null)); + metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(servletRequest, null)); } else { String syntheticFilterId = RejectingRequestFilter.SYNTHETIC_FILTER_CHAIN_ID; - metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, syntheticFilterId)); - request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, syntheticFilterId)); + servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); return Optional.of(RejectingRequestFilter.INSTANCE); } return maybeFilterId.map(bindings::getRequestFilter); } - Optional resolveResponseFilter(Request request, URI jdiscUri) { - Optional maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(request).listenPort()); + Optional resolveResponseFilter(HttpServletRequest servletRequest, URI jdiscUri) { + Optional maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(servletRequest).listenPort()); if (maybeFilterId.isPresent()) { - metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); - request.setAttribute(ServletRequest.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); + metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); + servletRequest.setAttribute(ServletRequest.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); } else { - metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(request, null)); + metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(servletRequest, null)); } return maybeFilterId.map(bindings::getResponseFilter); } - private Metric.Context createMetricContext(Request request, String filterId) { + private Metric.Context createMetricContext(HttpServletRequest request, String filterId) { Map extraDimensions = filterId != null ? Map.of(MetricDefinitions.FILTER_CHAIN_ID_DIMENSION, filterId) : Map.of(); - return getConnector(request).createRequestMetricContext(request, extraDimensions); + return JDiscHttpServlet.getConnector(request).createRequestMetricContext(request, extraDimensions); } private static class RejectingRequestFilter extends NoopSharedResource implements RequestFilter { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilteringRequestHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilteringRequestHandler.java index a487b63ef10..de768f979a1 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilteringRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilteringRequestHandler.java @@ -15,6 +15,7 @@ import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.filter.RequestFilter; import com.yahoo.jdisc.http.filter.ResponseFilter; +import javax.servlet.http.HttpServletRequest; import java.nio.ByteBuffer; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -41,11 +42,11 @@ class FilteringRequestHandler extends AbstractRequestHandler { }; private final FilterResolver filterResolver; - private final org.eclipse.jetty.server.Request jettyRequest; + private final HttpServletRequest servletRequest; - public FilteringRequestHandler(FilterResolver filterResolver, org.eclipse.jetty.server.Request jettyRequest) { + public FilteringRequestHandler(FilterResolver filterResolver, HttpServletRequest servletRequest) { this.filterResolver = filterResolver; - this.jettyRequest = jettyRequest; + this.servletRequest = servletRequest; } @Override @@ -53,9 +54,9 @@ class FilteringRequestHandler extends AbstractRequestHandler { Preconditions.checkArgument(request instanceof HttpRequest, "Expected HttpRequest, got " + request); Objects.requireNonNull(originalResponseHandler, "responseHandler"); - RequestFilter requestFilter = filterResolver.resolveRequestFilter(jettyRequest, request.getUri()) + RequestFilter requestFilter = filterResolver.resolveRequestFilter(servletRequest, request.getUri()) .orElse(null); - ResponseFilter responseFilter = filterResolver.resolveResponseFilter(jettyRequest, request.getUri()) + ResponseFilter responseFilter = filterResolver.resolveResponseFilter(servletRequest, request.getUri()) .orElse(null); // Not using request.connect() here - it adds logic for error handling that we'd rather leave to the framework. diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java index 8b6192bb455..0f7ce77e4cd 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java @@ -40,7 +40,7 @@ import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.Logger; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; /** * A handler that proxies status.html health checks @@ -91,7 +91,7 @@ class HealthCheckProxyHandler extends HandlerWrapper { @Override public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException { - int localPort = getConnectorLocalPort(request); + int localPort = getConnectorLocalPort(servletRequest); ProxyTarget proxyTarget = portToProxyTargetMapping.get(localPort); if (proxyTarget != null) { AsyncContext asyncContext = servletRequest.startAsync(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index 7828751df5a..05715b13d10 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -14,6 +14,7 @@ import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpRequest; import org.eclipse.jetty.io.EofException; +import org.eclipse.jetty.server.HttpConnection; import org.eclipse.jetty.server.Request; import javax.servlet.AsyncContext; @@ -33,8 +34,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import static com.yahoo.jdisc.http.HttpHeaders.Values.APPLICATION_X_WWW_FORM_URLENCODED; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getHttp1Connection; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnection; +import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; import static com.yahoo.yolean.Exceptions.throwUnchecked; /** @@ -71,7 +72,7 @@ class HttpRequestDispatch { jDiscContext.janitor, metricReporter, jDiscContext.developerMode()); - markHttp1ConnectionAsNonPersistentIfThresholdReached(jettyRequest); + markConnectionAsNonPersistentIfThresholdReached(servletRequest); this.async = servletRequest.startAsync(); async.setTimeout(0); metricReporter.uriLength(jettyRequest.getOriginalURI().length()); @@ -138,24 +139,22 @@ class HttpRequestDispatch { }; } - private static void markHttp1ConnectionAsNonPersistentIfThresholdReached(Request request) { + private static void markConnectionAsNonPersistentIfThresholdReached(HttpServletRequest request) { ConnectorConfig connectorConfig = getConnector(request).connectorConfig(); int maxRequestsPerConnection = connectorConfig.maxRequestsPerConnection(); if (maxRequestsPerConnection > 0) { - getHttp1Connection(request).ifPresent(connection -> { - if (connection.getMessagesIn() >= maxRequestsPerConnection) { - connection.getGenerator().setPersistent(false); - } - }); + HttpConnection connection = getConnection(request); + if (connection.getMessagesIn() >= maxRequestsPerConnection) { + connection.getGenerator().setPersistent(false); + } } double maxConnectionLifeInSeconds = connectorConfig.maxConnectionLife(); if (maxConnectionLifeInSeconds > 0) { - getHttp1Connection(request).ifPresent(connection -> { - Instant expireAt = Instant.ofEpochMilli((long) (connection.getCreatedTimeStamp() + maxConnectionLifeInSeconds * 1000)); - if (Instant.now().isAfter(expireAt)) { - connection.getGenerator().setPersistent(false); - } - }); + HttpConnection connection = getConnection(request); + Instant expireAt = Instant.ofEpochMilli((long)(connection.getCreatedTimeStamp() + maxConnectionLifeInSeconds * 1000)); + if (Instant.now().isAfter(expireAt)) { + connection.getGenerator().setPersistent(false); + } } } @@ -213,7 +212,7 @@ class HttpRequestDispatch { AccessLogEntry accessLogEntry, HttpServletRequest servletRequest) { RequestHandler requestHandler = wrapHandlerIfFormPost( - new FilteringRequestHandler(context.filterResolver, (Request)servletRequest), + new FilteringRequestHandler(context.filterResolver, servletRequest), servletRequest, context.serverConfig.removeRawPostBodyForWwwUrlEncodedPost()); return new AccessLoggingRequestHandler(requestHandler, accessLogEntry); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java index 8b223c45827..e8d37cfadb5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java @@ -4,7 +4,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.servlet.ServletRequest; import com.yahoo.jdisc.service.CurrentContainer; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.Utf8Appendable; import javax.servlet.http.HttpServletRequest; @@ -14,8 +13,8 @@ import java.security.cert.X509Certificate; import java.util.Enumeration; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnection; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnection; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; /** * @author Simon Thoresen Hult @@ -31,7 +30,7 @@ class HttpRequestFactory { HttpRequest.Method.valueOf(servletRequest.getMethod()), HttpRequest.Version.fromString(servletRequest.getProtocol()), new InetSocketAddress(servletRequest.getRemoteAddr(), servletRequest.getRemotePort()), - getConnection((Request) servletRequest).getCreatedTimeStamp()); + getConnection(servletRequest).getCreatedTimeStamp()); httpRequest.context().put(ServletRequest.JDISC_REQUEST_X509CERT, getCertChain(servletRequest)); return httpRequest; } catch (Utf8Appendable.NotUtf8Exception e) { @@ -44,7 +43,7 @@ class HttpRequestFactory { try { String scheme = servletRequest.getScheme(); String host = servletRequest.getServerName(); - int port = getConnectorLocalPort((Request) servletRequest); + int port = getConnectorLocalPort(servletRequest); String path = servletRequest.getRequestURI(); String query = servletRequest.getQueryString(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java new file mode 100644 index 00000000000..e7b9f459d2e --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java @@ -0,0 +1,38 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty; + +import org.eclipse.jetty.server.HttpConnection; + +import javax.servlet.http.HttpServletRequest; + +/** + * @author bjorncs + */ +public class HttpServletRequestUtils { + private HttpServletRequestUtils() {} + + public static HttpConnection getConnection(HttpServletRequest request) { + return (HttpConnection)request.getAttribute("org.eclipse.jetty.server.HttpConnection"); + } + + /** + * Note: {@link HttpServletRequest#getLocalPort()} may return the local port of the load balancer / reverse proxy if proxy-protocol is enabled. + * @return the actual local port of the underlying Jetty connector + */ + public static int getConnectorLocalPort(HttpServletRequest request) { + JDiscServerConnector connector = (JDiscServerConnector) getConnection(request).getConnector(); + int actualLocalPort = connector.getLocalPort(); + int localPortIfConnectorUnopened = -1; + int localPortIfConnectorClosed = -2; + if (actualLocalPort == localPortIfConnectorUnopened || actualLocalPort == localPortIfConnectorClosed) { + int configuredLocalPort = connector.listenPort(); + int localPortEphemeralPort = 0; + if (configuredLocalPort == localPortEphemeralPort) { + throw new IllegalStateException("Unable to determine connector's listen port"); + } + return configuredLocalPort; + } + return actualLocalPort; + } + +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscFilterInvokerFilter.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscFilterInvokerFilter.java index bff300431ce..a89c115a1c2 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscFilterInvokerFilter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscFilterInvokerFilter.java @@ -4,7 +4,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.filter.RequestFilter; -import org.eclipse.jetty.server.Request; import javax.servlet.AsyncContext; import javax.servlet.AsyncListener; @@ -27,7 +26,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; +import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; import static com.yahoo.yolean.Exceptions.throwUnchecked; /** @@ -78,7 +77,7 @@ class JDiscFilterInvokerFilter implements Filter { private void runChainAndResponseFilters(URI uri, HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { Optional responseFilterInvoker = - jDiscContext.filterResolver.resolveResponseFilter((Request)request, uri) + jDiscContext.filterResolver.resolveResponseFilter(request, uri) .map(responseFilter -> new OneTimeRunnable(() -> filterInvoker.invokeResponseFilterChain(responseFilter, uri, request, response))); @@ -108,7 +107,7 @@ class JDiscFilterInvokerFilter implements Filter { private HttpServletRequest runRequestFilterWithMatchingBinding(AtomicReference responseReturned, URI uri, HttpServletRequest request, HttpServletResponse response) throws IOException { try { - RequestFilter requestFilter = jDiscContext.filterResolver.resolveRequestFilter((Request)request, uri).orElse(null); + RequestFilter requestFilter = jDiscContext.filterResolver.resolveRequestFilter(request, uri).orElse(null); if (requestFilter == null) return request; @@ -135,7 +134,7 @@ class JDiscFilterInvokerFilter implements Filter { final AccessLogEntry accessLogEntry = null; // Not used in this context. return new HttpRequestDispatch(jDiscContext, accessLogEntry, - getConnector((Request)request).createRequestMetricContext(request, Map.of()), + getConnector(request).createRequestMetricContext(request, Map.of()), request, response); } catch (IOException e) { throw throwUnchecked(e); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java index 7e1445ffa4f..41a1ffc2709 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java @@ -5,7 +5,6 @@ import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.handler.OverloadException; import com.yahoo.jdisc.http.HttpRequest.Method; -import org.eclipse.jetty.server.Request; import javax.servlet.ServletException; import javax.servlet.annotation.WebServlet; @@ -21,7 +20,7 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnection; /** * @author Simon Thoresen Hult @@ -86,7 +85,7 @@ class JDiscHttpServlet extends HttpServlet { @Override protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - request.setAttribute(JDiscServerConnector.REQUEST_ATTRIBUTE, getConnector((Request) request)); + request.setAttribute(JDiscServerConnector.REQUEST_ATTRIBUTE, getConnector(request)); Metric.Context metricContext = getMetricContext(request); context.metric.add(MetricDefinitions.NUM_REQUESTS, 1, metricContext); @@ -104,6 +103,10 @@ class JDiscHttpServlet extends HttpServlet { } } + static JDiscServerConnector getConnector(HttpServletRequest request) { + return (JDiscServerConnector)getConnection(request).getConnector(); + } + private void dispatchHttpRequest(HttpServletRequest request, HttpServletResponse response) throws IOException { AccessLogEntry accessLogEntry = new AccessLogEntry(); request.setAttribute(ATTRIBUTE_NAME_ACCESS_LOG_ENTRY, accessLogEntry); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java deleted file mode 100644 index 5fca7a8d778..00000000000 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.http.server.jetty; - -import org.eclipse.jetty.io.Connection; -import org.eclipse.jetty.server.HttpConnection; -import org.eclipse.jetty.server.Request; - -import javax.servlet.http.HttpServletRequest; -import java.util.Optional; - -/** - * @author bjorncs - */ -public class RequestUtils { - private RequestUtils() {} - - public static Connection getConnection(Request request) { - return request.getHttpChannel().getConnection(); - } - - public static Optional getHttp1Connection(Request request) { - Connection connection = getConnection(request); - if (connection instanceof HttpConnection) return Optional.of((HttpConnection) connection); - return Optional.empty(); - } - - public static JDiscServerConnector getConnector(Request request) { - return (JDiscServerConnector) request.getHttpChannel().getConnector(); - } - - /** - * Note: {@link HttpServletRequest#getLocalPort()} may return the local port of the load balancer / reverse proxy if proxy-protocol is enabled. - * @return the actual local port of the underlying Jetty connector - */ - public static int getConnectorLocalPort(Request request) { - JDiscServerConnector connector = getConnector(request); - int actualLocalPort = connector.getLocalPort(); - int localPortIfConnectorUnopened = -1; - int localPortIfConnectorClosed = -2; - if (actualLocalPort == localPortIfConnectorUnopened || actualLocalPort == localPortIfConnectorClosed) { - int configuredLocalPort = connector.listenPort(); - int localPortEphemeralPort = 0; - if (configuredLocalPort == localPortEphemeralPort) { - throw new IllegalStateException("Unable to determine connector's listen port"); - } - return configuredLocalPort; - } - return actualLocalPort; - } - -} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/SecuredRedirectHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/SecuredRedirectHandler.java index dad274ae520..e32c9d46deb 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/SecuredRedirectHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/SecuredRedirectHandler.java @@ -14,7 +14,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; /** * A secure redirect handler inspired by {@link org.eclipse.jetty.server.handler.SecuredRedirectHandler}. @@ -33,7 +33,7 @@ class SecuredRedirectHandler extends HandlerWrapper { @Override public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException { - int localPort = getConnectorLocalPort(request); + int localPort = getConnectorLocalPort(servletRequest); if (!redirectMap.containsKey(localPort)) { _handler.handle(target, request, servletRequest, servletResponse); return; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java index 7299ab4b500..10a6c4702b5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java @@ -16,7 +16,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; /** * A Jetty handler that enforces TLS client authentication with configurable white list. @@ -34,7 +34,7 @@ class TlsClientAuthenticationEnforcer extends HandlerWrapper { @Override public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException { if (isHttpsRequest(request) - && !isRequestToWhitelistedBinding(request) + && !isRequestToWhitelistedBinding(servletRequest) && !isClientAuthenticated(servletRequest)) { servletResponse.sendError( Response.Status.UNAUTHORIZED, @@ -60,14 +60,14 @@ class TlsClientAuthenticationEnforcer extends HandlerWrapper { return request.getDispatcherType() == DispatcherType.REQUEST && request.getScheme().equalsIgnoreCase("https"); } - private boolean isRequestToWhitelistedBinding(Request jettyRequest) { - int localPort = getConnectorLocalPort(jettyRequest); + private boolean isRequestToWhitelistedBinding(HttpServletRequest servletRequest) { + int localPort = getConnectorLocalPort(servletRequest); List whiteListedPaths = getWhitelistedPathsForPort(localPort); if (whiteListedPaths == null) { return true; // enforcer not enabled } // Note: Same path definition as HttpRequestFactory.getUri() - return whiteListedPaths.contains(jettyRequest.getRequestURI()); + return whiteListedPaths.contains(servletRequest.getRequestURI()); } private List getWhitelistedPathsForPort(int localPort) { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java index bb78511a17f..c945dc6d8b6 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java @@ -6,7 +6,6 @@ import com.yahoo.jdisc.HeaderFields; import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpRequest; -import org.eclipse.jetty.server.Request; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; @@ -25,7 +24,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnection; +import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnection; /** * Mutable wrapper to use a {@link javax.servlet.http.HttpServletRequest} @@ -69,7 +68,7 @@ public class ServletRequest extends HttpServletRequestWrapper implements Servlet remoteHostAddress = request.getRemoteAddr(); remoteHostName = request.getRemoteHost(); remotePort = request.getRemotePort(); - connectedAt = getConnection((Request) request).getCreatedTimeStamp(); + connectedAt = getConnection(request).getCreatedTimeStamp(); headerFields = new HeaderFields(); Enumeration parentHeaders = request.getHeaderNames(); 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 cb1e366f843..055e5ad62d2 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 @@ -125,6 +125,3 @@ maxRequestsPerConnection int default=0 # Maximum number of seconds a connection can live before it's marked as non-persistent. Set to '0' to disable. maxConnectionLife double default=0.0 - -# Enable HTTP/2 (in addition to HTTP/1.1 using ALPN) -http2Enabled bool default=false diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java index ed4c9b66068..3052902f174 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java @@ -3,11 +3,12 @@ package com.yahoo.jdisc.http.filter; import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpHeaders; -import com.yahoo.jdisc.http.server.jetty.JettyMockRequestBuilder; import com.yahoo.jdisc.http.servlet.ServletRequest; -import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.HttpConnection; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.mock.web.MockHttpServletRequest; import java.net.URI; import java.util.Arrays; @@ -17,6 +18,7 @@ import java.util.List; import static com.yahoo.jdisc.http.HttpRequest.Version; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; /** * Test the parts of the DiscFilterRequest API that are implemented @@ -24,6 +26,7 @@ import static org.junit.Assert.assertTrue; * {@link com.yahoo.jdisc.http.servlet.ServletRequest}. * * @author gjoranv + * @since 5.27 */ public class ServletFilterRequestTest { @@ -51,14 +54,18 @@ public class ServletFilterRequestTest { parentRequest = ((ServletFilterRequest)filterRequest).getServletRequest(); } - private ServletRequest newServletRequest() { - Request parent = JettyMockRequestBuilder.newBuilder() - .remote("1.2.3.4", host, port) - .header(headerName, List.of(headerValue)) - .parameter(paramName, List.of(paramValue)) - .parameter(listParamName, List.of(listParamValue)) - .attribute(attributeName, attributeValue) - .build(); + private ServletRequest newServletRequest() throws Exception { + MockHttpServletRequest parent = new MockHttpServletRequest("GET", uri.toString()); + parent.setProtocol(Version.HTTP_1_1.toString()); + parent.setRemoteHost(host); + parent.setRemotePort(port); + parent.setParameter(paramName, paramValue); + parent.setParameter(listParamName, listParamValue); + parent.addHeader(headerName, headerValue); + parent.setAttribute(attributeName, attributeValue); + HttpConnection connection = Mockito.mock(HttpConnection.class); + when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); + parent.setAttribute("org.eclipse.jetty.server.HttpConnection", connection); return new ServletRequest(parent, uri); } 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 c45d17a4ff8..e472f954afc 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,12 @@ 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.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.server.HttpChannel; +import org.eclipse.jetty.server.HttpConnection; +import org.eclipse.jetty.server.HttpInput; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.junit.Test; @@ -18,6 +23,8 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @author Oyvind Bakksjo @@ -26,9 +33,9 @@ import static org.junit.Assert.assertTrue; public class AccessLogRequestLogTest { @Test public void requireThatQueryWithUnquotedSpecialCharactersIsHandled() { - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, "/search/", "query=year:>2010") - .build(); + final Request jettyRequest = createRequestMock(); + when(jettyRequest.getRequestURI()).thenReturn("/search/"); + when(jettyRequest.getQueryString()).thenReturn("query=year:>2010"); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -40,11 +47,11 @@ public class AccessLogRequestLogTest { @Test public void requireThatDoubleQuotingIsNotPerformed() { - String path = "/search/"; - String query = "query=year%252010+%3B&customParameter=something"; - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, path, query) - .build(); + final Request jettyRequest = createRequestMock(); + final String path = "/search/"; + when(jettyRequest.getRequestURI()).thenReturn(path); + final String query = "query=year%252010+%3B&customParameter=something"; + when(jettyRequest.getQueryString()).thenReturn(query); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -57,11 +64,11 @@ public class AccessLogRequestLogTest { @Test public void raw_path_and_query_are_set_from_request() { + Request jettyRequest = createRequestMock(); String rawPath = "//search/"; + when(jettyRequest.getRequestURI()).thenReturn(rawPath); String rawQuery = "q=%%2"; - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, rawPath, rawQuery) - .build(); + when(jettyRequest.getQueryString()).thenReturn(rawQuery); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -74,11 +81,11 @@ public class AccessLogRequestLogTest { @Test public void verify_x_forwarded_for_precedence () { - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, "//search/", "q=%%2") - .header("x-forwarded-for", List.of("1.2.3.4")) - .header("y-ra", List.of("2.3.4.5")) - .build(); + Request jettyRequest = createRequestMock(); + when(jettyRequest.getRequestURI()).thenReturn("//search/"); + when(jettyRequest.getQueryString()).thenReturn("q=%%2"); + when(jettyRequest.getHeader("x-forwarded-for")).thenReturn("1.2.3.4"); + when(jettyRequest.getHeader("y-ra")).thenReturn("2.3.4.5"); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -88,11 +95,11 @@ public class AccessLogRequestLogTest { @Test public void verify_x_forwarded_port_precedence () { - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, "//search/", "q=%%2") - .header("X-Forwarded-Port", List.of("80")) - .header("y-rp", List.of("8080")) - .build(); + Request jettyRequest = createRequestMock(); + when(jettyRequest.getRequestURI()).thenReturn("//search/"); + when(jettyRequest.getQueryString()).thenReturn("q=%%2"); + when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("80"); + when(jettyRequest.getHeader("y-rp")).thenReturn("8080"); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -102,12 +109,10 @@ public class AccessLogRequestLogTest { @Test public void defaults_to_peer_port_if_remote_port_header_is_invalid() { - Request jettyRequest = createRequestBuilder() - .uri("http", "localhost", 12345, "/search/", null) - .header("X-Forwarded-Port", List.of("8o8o")) - .header("y-rp", List.of("8o8o")) - .remote("2.3.4.5", "localhost", 80) - .build(); + final Request jettyRequest = createRequestMock(); + when(jettyRequest.getRequestURI()).thenReturn("/search/"); + when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("8o8o"); + when(jettyRequest.getRemotePort()).thenReturn(80); InMemoryRequestLog requestLog = new InMemoryRequestLog(); doAccessLoggingOfRequest(requestLog, jettyRequest); @@ -124,14 +129,32 @@ public class AccessLogRequestLogTest { new AccessLogRequestLog(requestLog, config).log(jettyRequest, createResponseMock()); } - private static JettyMockRequestBuilder createRequestBuilder() { - return JettyMockRequestBuilder.newBuilder() - .attribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY, new AccessLogEntry()) - .remote("2.3.4.5", "localhost", 12345) - .localPort(1234); + private static Request createRequestMock() { + JDiscServerConnector serverConnector = mock(JDiscServerConnector.class); + int localPort = 1234; + when(serverConnector.connectorConfig()).thenReturn(new ConnectorConfig(new ConnectorConfig.Builder().listenPort(localPort))); + when(serverConnector.getLocalPort()).thenReturn(localPort); + HttpConnection httpConnection = mock(HttpConnection.class); + when(httpConnection.getConnector()).thenReturn(serverConnector); + Request request = mock(Request.class); + when(request.getMethod()).thenReturn("GET"); + when(request.getRemoteAddr()).thenReturn("localhost"); + when(request.getRemotePort()).thenReturn(12345); + when(request.getProtocol()).thenReturn("HTTP/1.1"); + when(request.getScheme()).thenReturn("http"); + when(request.getTimeStamp()).thenReturn(0L); + when(request.getAttribute(JDiscHttpServlet.ATTRIBUTE_NAME_ACCESS_LOG_ENTRY)).thenReturn(new AccessLogEntry()); + when(request.getAttribute("org.eclipse.jetty.server.HttpConnection")).thenReturn(httpConnection); + HttpInput httpInput = mock(HttpInput.class); + when(httpInput.getContentReceived()).thenReturn(2345L); + when(request.getHttpInput()).thenReturn(httpInput); + return request; } private Response createResponseMock() { - return JettyMockResponseBuilder.newBuilder().build(); + Response response = mock(Response.class); + when(response.getHttpChannel()).thenReturn(mock(HttpChannel.class)); + when(response.getCommittedMetaData()).thenReturn(mock(MetaData.Response.class)); + return response; } } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java index e117ef7f723..a67656dd5ca 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java @@ -99,7 +99,7 @@ public class FilterTestCase { final MyRequestHandler requestHandler = new MyRequestHandler(); final TestDriver testDriver = newDriver(requestHandler, filterBindings); - testDriver.client().get("/status.html"); + testDriver.client().get("status.html"); assertThat(requestHandler.awaitInvocation(), is(true)); assertThat(requestHandler.getHeaderMap().get("foo").get(0), is("bar")); diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java index fbbf3074839..9c1348004ee 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java @@ -7,8 +7,10 @@ import com.yahoo.jdisc.References; import com.yahoo.jdisc.ResourceReference; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.handler.RequestHandler; +import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.service.CurrentContainer; +import org.eclipse.jetty.server.HttpConnection; import org.junit.Test; import javax.servlet.http.HttpServletRequest; @@ -20,6 +22,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @author Steinar Knutsen @@ -137,15 +141,27 @@ public class HttpRequestFactoryTest { assertEquals(LOCAL_PORT, request.getUri().getPort()); } - private HttpServletRequest createMockRequest(String scheme, String host, String path, String query) { - return JettyMockRequestBuilder.newBuilder() - .uri(scheme, host, LOCAL_PORT, path, query) - .remote("127.0.0.1", "localhost", 1234) - .localPort(LOCAL_PORT) - .build(); + private static HttpServletRequest createMockRequest(String scheme, String serverName, String path, String queryString) { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpConnection connection = mock(HttpConnection.class); + JDiscServerConnector connector = mock(JDiscServerConnector.class); + when(connector.connectorConfig()).thenReturn(new ConnectorConfig(new ConnectorConfig.Builder().listenPort(LOCAL_PORT))); + when(connector.getLocalPort()).thenReturn(LOCAL_PORT); + when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); + when(connection.getConnector()).thenReturn(connector); + when(request.getAttribute("org.eclipse.jetty.server.HttpConnection")).thenReturn(connection); + when(request.getProtocol()).thenReturn("HTTP/1.1"); + when(request.getScheme()).thenReturn(scheme); + when(request.getServerName()).thenReturn(serverName); + when(request.getRemoteAddr()).thenReturn("127.0.0.1"); + when(request.getRemotePort()).thenReturn(1234); + when(request.getLocalPort()).thenReturn(LOCAL_PORT); + when(request.getMethod()).thenReturn("GET"); + when(request.getQueryString()).thenReturn(queryString); + when(request.getRequestURI()).thenReturn(path); + return request; } - private static final class MockContainer implements CurrentContainer { @Override diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index f5d77b53f12..d8e94d13813 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -35,19 +35,10 @@ import com.yahoo.security.SslContextBuilder; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; import com.yahoo.security.tls.TlsContext; -import org.apache.hc.client5.http.async.methods.SimpleHttpRequests; -import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; -import org.apache.hc.client5.http.entity.mime.FormBodyPart; -import org.apache.hc.client5.http.entity.mime.FormBodyPartBuilder; -import org.apache.hc.client5.http.entity.mime.StringBody; -import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; -import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder; -import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; -import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder; -import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; -import org.apache.hc.core5.http.ContentType; -import org.apache.hc.core5.http.nio.ssl.TlsStrategy; -import org.apache.hc.core5.http2.HttpVersionPolicy; +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.mime.FormBodyPart; +import org.apache.http.entity.mime.content.StringBody; import org.assertj.core.api.Assertions; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1; @@ -116,7 +107,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.anyOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @@ -410,8 +400,8 @@ public class HttpServerTest { final ResponseValidator response = driver.client().newPost("/status.html") .setMultipartContent( - newFileBody("start.txt", startTxtContent), - newFileBody("updater.conf", updaterConfContent)) + newFileBody("", "start.txt", startTxtContent), + newFileBody("", "updater.conf", updaterConfContent)) .execute(); response.expectStatusCode(is(OK)) .expectContent(containsString(startTxtContent)) @@ -514,28 +504,12 @@ public class HttpServerTest { assertTrue(driver.close()); } - @Test - public void requireThatServerCanRespondToHttp2Request() throws Exception { - Path privateKeyFile = tmpFolder.newFile().toPath(); - Path certificateFile = tmpFolder.newFile().toPath(); - generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - - TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); - try (CloseableHttpAsyncClient client = createHttp2Client(certificateFile, privateKeyFile)) { - String uri = "https://localhost:" + driver.server().getListenPort() + "/status.html"; - SimpleHttpResponse response = client.execute(SimpleHttpRequests.get(uri), null).get(); - assertNull(response.getBodyText()); - assertEquals(OK, response.getCode()); - } - assertTrue(driver.close()); - } - @Test public void requireThatTlsClientAuthenticationEnforcerRejectsRequestsForNonWhitelistedPaths() throws IOException { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - TestDriver driver = createSslWithTlsClientAuthenticationEnforcer(certificateFile, privateKeyFile); + TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); SSLContext trustStoreOnlyCtx = new SslContextBuilder() .withTrustStore(certificateFile) @@ -944,21 +918,6 @@ public class HttpServerTest { return client; } - private static CloseableHttpAsyncClient createHttp2Client(Path certificateFile, Path privateKeyFile) { - TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); - TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create() - .setSslContext(driver.newSslContext()) - .build(); - var client = HttpAsyncClientBuilder.create() - .setVersionPolicy(HttpVersionPolicy.FORCE_HTTP_2) - .disableConnectionState() - .disableAutomaticRetries() - .setConnectionManager(PoolingAsyncClientConnectionManagerBuilder.create().setTlsStrategy(tlsStrategy).build()) - .build(); - client.start(); - return client; - } - private static void assertLogEntryHasRemote(RequestLogEntry entry, String expectedAddress, int expectedPort) { assertEquals(expectedAddress, entry.peerAddress().get()); if (expectedPort > 0) { @@ -1010,25 +969,6 @@ public class HttpServerTest { }); } - private static TestDriver createSslWithTlsClientAuthenticationEnforcer(Path certificateFile, Path privateKeyFile) { - ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder() - .tlsClientAuthEnforcer( - new ConnectorConfig.TlsClientAuthEnforcer.Builder() - .enable(true) - .pathWhitelist("/status.html")) - .ssl(new ConnectorConfig.Ssl.Builder() - .enabled(true) - .clientAuth(ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH) - .privateKeyFile(privateKeyFile.toString()) - .certificateFile(certificateFile.toString()) - .caCertificateFile(certificateFile.toString())); - return TestDrivers.newConfiguredInstance( - new EchoRequestHandler(), - new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)), - connectorConfig, - binder -> {}); - } - private static TestDriver createSslTestDriver( Path serverCertificateFile, Path serverPrivateKeyFile, MetricConsumerMock metricConsumer, InMemoryConnectionLog connectionLog) throws IOException { Module extraModule = binder -> { @@ -1109,16 +1049,30 @@ public class HttpServerTest { new ConnectorConfig.Builder()); } - private static FormBodyPart newFileBody(final String fileName, final String fileContent) { - return FormBodyPartBuilder.create() - .setBody( - new StringBody(fileContent, ContentType.TEXT_PLAIN) { - @Override public String getFilename() { return fileName; } - @Override public String getMimeType() { return ""; } - @Override public String getCharset() { return null; } - }) - .setName(fileName) - .build(); + private static FormBodyPart newFileBody(final String parameterName, final String fileName, final String fileContent) { + return new FormBodyPart( + parameterName, + new StringBody(fileContent, ContentType.TEXT_PLAIN) { + @Override + public String getFilename() { + return fileName; + } + + @Override + public String getTransferEncoding() { + return "binary"; + } + + @Override + public String getMimeType() { + return ""; + } + + @Override + public String getCharset() { + return null; + } + }); } private static class ConnectedAtRequestHandler extends AbstractRequestHandler { diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java index 23c229e2ec5..230f59cbb34 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java @@ -7,15 +7,15 @@ import com.yahoo.jdisc.handler.AbstractRequestHandler; import com.yahoo.jdisc.handler.ContentChannel; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseHandler; -import org.apache.hc.client5.http.classic.methods.HttpDelete; -import org.apache.hc.client5.http.classic.methods.HttpGet; -import org.apache.hc.client5.http.classic.methods.HttpHead; -import org.apache.hc.client5.http.classic.methods.HttpOptions; -import org.apache.hc.client5.http.classic.methods.HttpPatch; -import org.apache.hc.client5.http.classic.methods.HttpPost; -import org.apache.hc.client5.http.classic.methods.HttpPut; -import org.apache.hc.client5.http.classic.methods.HttpTrace; -import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpHead; +import org.apache.http.client.methods.HttpOptions; +import org.apache.http.client.methods.HttpPatch; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.client.methods.HttpTrace; import org.junit.Test; import java.io.IOException; @@ -73,7 +73,8 @@ public class JDiscHttpServletTest { }; } - private static class UnknownMethodHttpRequest extends HttpUriRequestBase { - UnknownMethodHttpRequest(URI uri) { super("UNKNOWN_METHOD", uri); } + private static class UnknownMethodHttpRequest extends HttpRequestBase { + UnknownMethodHttpRequest(URI uri) { setURI(uri); } + @Override public String getMethod() { return "UNKNOWN_METHOD"; } } } 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 deleted file mode 100644 index 4bf6afeb3f1..00000000000 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockRequestBuilder.java +++ /dev/null @@ -1,176 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.http.server.jetty; - -import com.yahoo.jdisc.http.ConnectorConfig; -import org.eclipse.jetty.server.HttpChannel; -import org.eclipse.jetty.server.HttpConnection; -import org.eclipse.jetty.server.HttpInput; -import org.eclipse.jetty.server.Request; -import org.mockito.stubbing.Answer; - -import java.io.UnsupportedEncodingException; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * Builder for creating a mock instance of Jetty's {@link Request} type. - * - * @author bjorncs - */ -public class JettyMockRequestBuilder { - - private final Map> parameters = new HashMap<>(); - private final Map> headers = new HashMap<>(); - private final Map attributes = new HashMap<>(); - private Integer localPort; - private String uriScheme; - private String uriServerName; - private Integer uriPort; - private String uriPath; - private String uriQuery; - private String remoteAddress; - private String remoteHost; - private Integer remotePort; - - private JettyMockRequestBuilder() {} - - public static JettyMockRequestBuilder newBuilder() { return new JettyMockRequestBuilder(); } - - public JettyMockRequestBuilder localPort(int localPort) { this.localPort = localPort; return this; } - - public JettyMockRequestBuilder remote(String address, String host, int port) { - this.remoteAddress = address; - this.remoteHost = host; - this.remotePort = port; - return this; - } - - public JettyMockRequestBuilder uri(String scheme, String serverName, int port, String path, String query) { - this.uriScheme = scheme; - this.uriServerName = serverName; - this.uriPort = port; - this.uriPath = path; - this.uriQuery = query; - return this; - } - - public JettyMockRequestBuilder parameter(String name, List values) { this.parameters.put(name, List.copyOf(values)); return this; } - - public JettyMockRequestBuilder header(String name, List values) { this.headers.put(name, List.copyOf(values)); return this; } - - public JettyMockRequestBuilder attribute(String name, Object value) { this.attributes.put(name, value); return this; } - - public Request build() { - int localPort = this.localPort != null ? this.localPort : 8080; - String scheme = this.uriScheme != null ? this.uriScheme : "http"; - String serverName = this.uriServerName != null ? this.uriServerName : "localhost"; - int uriPort = this.uriPort != null ? this.uriPort : 8080; - String path = this.uriPath; - String query = this.uriQuery; - String remoteAddress = this.remoteAddress != null ? this.remoteAddress : "1.2.3.4"; - String remoteHost = this.remoteHost != null ? this.remoteHost : "remotehost"; - Integer remotePort = this.remotePort != null ? this.remotePort : 12345; - - 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.getLocalPort()).thenReturn(localPort); - when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); - when(connection.getConnector()).thenReturn(connector); - when(connection.getHttpChannel()).thenReturn(channel); - when(channel.getConnector()).thenReturn(connector); - when(channel.getConnection()).thenReturn(connection); - - HttpInput httpInput = mock(HttpInput.class); - when(httpInput.getContentReceived()).thenReturn(2345L); - - Request request = mock(Request.class); - when(request.getHttpChannel()).thenReturn(channel); - when(request.getHttpInput()).thenReturn(httpInput); - when(request.getProtocol()).thenReturn("HTTP/1.1"); - when(request.getScheme()).thenReturn(scheme); - when(request.getServerName()).thenReturn(serverName); - when(request.getRemoteAddr()).thenReturn(remoteAddress); - when(request.getRemotePort()).thenReturn(remotePort); - when(request.getRemoteHost()).thenReturn(remoteHost); - when(request.getLocalPort()).thenReturn(uriPort); - when(request.getMethod()).thenReturn("GET"); - when(request.getQueryString()).thenReturn(query); - when(request.getRequestURI()).thenReturn(path); - - mockCharacterEncodingHandling(request); - mockHeaderHandling(request); - mockParameterHandling(request); - mockAttributeHandling(request); - - return request; - } - - private void mockCharacterEncodingHandling(Request request) { - try { - AtomicReference characterEncoding = new AtomicReference<>(""); - when(request.getCharacterEncoding()).thenAnswer((Answer) ignored -> characterEncoding.get()); - doAnswer((Answer) invocation -> { - String value = invocation.getArgument(0); - characterEncoding.set(value); - return null; - }).when(request).setCharacterEncoding(anyString()); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } - } - - private void mockHeaderHandling(Request request) { - Map> headers = new ConcurrentHashMap<>(this.headers); - when(request.getHeaderNames()).thenReturn(Collections.enumeration(headers.keySet())); - when(request.getHeaders(anyString())).thenAnswer((Answer>) invocation -> { - String key = invocation.getArgument(0); - List values = headers.get(key); - return values != null ? Collections.enumeration(values) : Collections.enumeration(List.of()); - }); - when(request.getHeader(anyString())).thenAnswer((Answer) invocation -> { - String name = invocation.getArgument(0); - List values = headers.get(name); - if (values == null || values.isEmpty()) return null; - return values.get(0); - }); - } - - private void mockParameterHandling(Request request) { - Map parameters = new ConcurrentHashMap<>(); - this.parameters.forEach((key, values) -> parameters.put(key, values.toArray(String[]::new))); - when(request.getParameterMap()).thenReturn(parameters); - } - - private void mockAttributeHandling(Request request) { - Map attributes = new ConcurrentHashMap<>(this.attributes); - - when(request.getAttribute(any())).thenAnswer(invocation -> { - String attributeName = invocation.getArgument(0); - return attributes.get(attributeName); - }); - doAnswer((Answer) invocation -> { - String attributeName = invocation.getArgument(0); - Object attributeValue = invocation.getArgument(1); - attributes.put(attributeName, attributeValue); - return null; - }).when(request).setAttribute(anyString(), any()); - doAnswer((Answer) invocation -> { - String attributeName = invocation.getArgument(0); - attributes.remove(attributeName); - return null; - }).when(request).removeAttribute(anyString()); - } -} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java deleted file mode 100644 index 6addb966208..00000000000 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.http.server.jetty; - -import org.eclipse.jetty.http.MetaData; -import org.eclipse.jetty.server.HttpChannel; -import org.eclipse.jetty.server.Response; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * Builder for creating a mock instance of Jetty's {@link Response} type. - * - * @author bjorncs - */ -public class JettyMockResponseBuilder { - - private JettyMockResponseBuilder() {} - - public static JettyMockResponseBuilder newBuilder() { return new JettyMockResponseBuilder(); } - - public Response build() { - Response response = mock(Response.class); - when(response.getHttpChannel()).thenReturn(mock(HttpChannel.class)); - when(response.getCommittedMetaData()).thenReturn(mock(MetaData.Response.class)); - return response; - } - -} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java index 161f48d847d..eea8d7e3072 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java @@ -1,36 +1,33 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; -import org.apache.hc.client5.http.SystemDefaultDnsResolver; -import org.apache.hc.client5.http.classic.methods.HttpGet; -import org.apache.hc.client5.http.classic.methods.HttpPost; -import org.apache.hc.client5.http.classic.methods.HttpUriRequest; -import org.apache.hc.client5.http.entity.GzipCompressingEntity; -import org.apache.hc.client5.http.entity.mime.FormBodyPart; -import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder; -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; -import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; -import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; -import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; -import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; -import org.apache.hc.core5.http.ContentType; -import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpResponse; -import org.apache.hc.core5.http.ParseException; -import org.apache.hc.core5.http.io.entity.ByteArrayEntity; -import org.apache.hc.core5.http.io.entity.EntityUtils; -import org.apache.hc.core5.http.io.entity.StringEntity; +import org.apache.http.Header; +import org.apache.http.HttpEntity; +import org.apache.http.HttpResponse; +import org.apache.http.client.entity.GzipCompressingEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.conn.ssl.DefaultHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.entity.StringEntity; +import org.apache.http.entity.mime.FormBodyPart; +import org.apache.http.entity.mime.MultipartEntityBuilder; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.BasicHttpClientConnectionManager; +import org.apache.http.util.EntityUtils; import org.hamcrest.Matcher; import org.hamcrest.MatcherAssert; import javax.net.ssl.SSLContext; import java.io.IOException; -import java.net.InetAddress; import java.net.URI; -import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -58,9 +55,8 @@ public class SimpleHttpClient implements AutoCloseable { public SimpleHttpClient(SSLContext sslContext, List enabledProtocols, List enabledCiphers, int listenPort, boolean useCompression) { - HttpClientBuilder builder = HttpClientBuilder.create() - .disableAutomaticRetries() - .disableConnectionState(); // Reuse SSL connection when client authentication is enabled + HttpClientBuilder builder = HttpClientBuilder.create(); + builder.disableConnectionState(); // Reuse SSL connection when client authentication is enabled if (!useCompression) { builder.disableContentCompression(); } @@ -70,17 +66,12 @@ public class SimpleHttpClient implements AutoCloseable { toArray(enabledProtocols), toArray(enabledCiphers), new DefaultHostnameVerifier()); - PoolingHttpClientConnectionManager connManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslConnectionFactory) - .setDnsResolver(new SystemDefaultDnsResolver() { - @Override - public InetAddress[] resolve(String host) throws UnknownHostException { - // Returns single address instead of multiple (to avoid multiple connection attempts) - return new InetAddress[] { InetAddress.getByName(host) }; - } - }) + builder.setSSLSocketFactory(sslConnectionFactory); + + Registry registry = RegistryBuilder.create() + .register("https", sslConnectionFactory) .build(); - builder.setConnectionManager(connManager); + builder.setConnectionManager(new BasicHttpClientConnectionManager(registry)); scheme = "https"; } else { scheme = "http"; @@ -148,7 +139,7 @@ public class SimpleHttpClient implements AutoCloseable { } public RequestExecutor setBinaryContent(final byte[] content) { - this.entity = new ByteArrayEntity(content, ContentType.DEFAULT_BINARY); + this.entity = new ByteArrayEntity(content); return this; } @@ -161,7 +152,7 @@ public class SimpleHttpClient implements AutoCloseable { public ResponseValidator execute() throws IOException { if (entity != null) { - request.setEntity(entity); + ((HttpPost)request).setEntity(entity); } try (CloseableHttpResponse response = delegate.execute(request)){ return new ResponseValidator(response); @@ -174,19 +165,15 @@ public class SimpleHttpClient implements AutoCloseable { private final HttpResponse response; private final String content; - public ResponseValidator(CloseableHttpResponse response) throws IOException { - try { - this.response = response; + public ResponseValidator(HttpResponse response) throws IOException { + this.response = response; - HttpEntity entity = response.getEntity(); - this.content = entity == null ? null : EntityUtils.toString(entity, StandardCharsets.UTF_8); - } catch (ParseException e) { - throw new IOException(e); - } + HttpEntity entity = response.getEntity(); + this.content = entity == null ? null : EntityUtils.toString(entity, StandardCharsets.UTF_8); } public ResponseValidator expectStatusCode(Matcher matcher) { - MatcherAssert.assertThat(response.getCode(), matcher); + MatcherAssert.assertThat(response.getStatusLine().getStatusCode(), matcher); return this; } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java index 75fc0948da9..7d7530c32e0 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java @@ -55,7 +55,10 @@ public class TestDrivers { newConfigModule( new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)), new ConnectorConfig.Builder() - .http2Enabled(true) + .tlsClientAuthEnforcer( + new ConnectorConfig.TlsClientAuthEnforcer.Builder() + .enable(true) + .pathWhitelist("/status.html")) .ssl(new ConnectorConfig.Ssl.Builder() .enabled(true) .clientAuth(tlsClientAuth == TlsClientAuth.NEED diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index afcdf474723..04ce3405d83 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -387,25 +387,6 @@ guava-testlib ${guava.version} - - - - org.eclipse.jetty.alpn - alpn-api - ${jetty-alpn.version} - - - - org.eclipse.jetty.http2 - http2-server - ${jetty.version} - - - - org.eclipse.jetty - jetty-alpn-java-server - ${jetty.version} - org.eclipse.jetty @@ -478,7 +459,6 @@ 3.1.0 2.3.0 9.4.38.v20210224 - 1.1.3.v20160715 1.7.1 20090211 1.7.30 diff --git a/container-disc/pom.xml b/container-disc/pom.xml index 15eb0328284..a78657b7f09 100644 --- a/container-disc/pom.xml +++ b/container-disc/pom.xml @@ -188,12 +188,6 @@ zkfacade-jar-with-dependencies.jar, zookeeper-server-jar-with-dependencies.jar, - alpn-api-${jetty-alpn.version}.jar, - http2-server-${jetty.version}.jar, - http2-common-${jetty.version}.jar, - http2-hpack-${jetty.version}.jar, - jetty-alpn-java-server-${jetty.version}.jar, - jetty-alpn-server-${jetty.version}.jar, jetty-continuation-${jetty.version}.jar, jetty-http-${jetty.version}.jar, jetty-io-${jetty.version}.jar, diff --git a/dist/vespa.spec b/dist/vespa.spec index a14f3bb0037..3707d4afb9c 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -650,8 +650,6 @@ fi %{_prefix}/lib/jars/jdisc_core-jar-with-dependencies.jar %{_prefix}/lib/jars/jdisc-security-filters-jar-with-dependencies.jar %{_prefix}/lib/jars/jersey-*.jar -%{_prefix}/lib/jars/alpn-*.jar -%{_prefix}/lib/jars/http2-*.jar %{_prefix}/lib/jars/jetty-*.jar %{_prefix}/lib/jars/mimepull-*.jar %{_prefix}/lib/jars/model-evaluation-jar-with-dependencies.jar diff --git a/jdisc_jetty/pom.xml b/jdisc_jetty/pom.xml index fcbf0eed8b7..89e573651e9 100644 --- a/jdisc_jetty/pom.xml +++ b/jdisc_jetty/pom.xml @@ -15,18 +15,6 @@ 7-SNAPSHOT jar - - org.eclipse.jetty.alpn - alpn-api - - - org.eclipse.jetty.http2 - http2-server - - - org.eclipse.jetty - jetty-alpn-java-server - org.eclipse.jetty jetty-continuation diff --git a/parent/pom.xml b/parent/pom.xml index 3b5b0891e73..0f2ea616193 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -735,6 +735,11 @@ asm ${asm.version} + + org.springframework + spring-test + 4.0.6.RELEASE + org.tensorflow proto -- cgit v1.2.3 From 89066e062ffac158cc6f6ea3b904b70fd5cf1daa Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 12 Apr 2021 12:22:56 +0200 Subject: Add back connector config definition --- .../resources/configdefinitions/jdisc.http.jdisc.http.connector.def | 3 +++ 1 file changed, 3 insertions(+) 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 055e5ad62d2..cb1e366f843 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 @@ -125,3 +125,6 @@ maxRequestsPerConnection int default=0 # Maximum number of seconds a connection can live before it's marked as non-persistent. Set to '0' to disable. maxConnectionLife double default=0.0 + +# Enable HTTP/2 (in addition to HTTP/1.1 using ALPN) +http2Enabled bool default=false -- cgit v1.2.3