From dcf5a1b725facbaff14ba7659254ea4b72895dbb Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 12 Apr 2021 09:47:31 +0200 Subject: Revert "Revert "Bjorncs/jdisc http2 preps [run-systemtest]"" --- .../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 +-- 13 files changed, 186 insertions(+), 116 deletions(-) delete mode 100644 container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java create mode 100644 container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java (limited to 'container-core/src/main/java/com/yahoo/jdisc/http/server/jetty') 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 2f9fc0d07b2..11898381f0a 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.HttpServletRequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.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 d7ad12a5c64..bc358e2fb06 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,6 +7,8 @@ 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; @@ -18,6 +20,7 @@ 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; /** @@ -76,41 +79,72 @@ public class ConnectorFactory { } private List createConnectionFactories(Metric metric) { - HttpConnectionFactory httpFactory = newHttpConnectionFactory(); if (!isSslEffectivelyEnabled(connectorConfig)) { - return List.of(httpFactory); + return List.of(newHttp1ConnectionFactory()); } else if (connectorConfig.ssl().enabled()) { - return connectionFactoriesForHttps(metric, httpFactory); + return connectionFactoriesForHttps(metric); } else if (TransportSecurityUtils.isTransportSecurityEnabled()) { switch (TransportSecurityUtils.getInsecureMixedMode()) { case TLS_CLIENT_MIXED_SERVER: case PLAINTEXT_CLIENT_MIXED_SERVER: - return List.of(new DetectorConnectionFactory(newSslConnectionFactory(metric, httpFactory)), httpFactory); + return connectionFactoriesForHttpsMixedMode(metric); case DISABLED: - return connectionFactoriesForHttps(metric, httpFactory); + return connectionFactoriesForHttps(metric); default: throw new IllegalStateException(); } } else { - return List.of(httpFactory); + return List.of(newHttp1ConnectionFactory()); } } - private List connectionFactoriesForHttps(Metric metric, HttpConnectionFactory httpFactory) { + private List connectionFactoriesForHttps(Metric metric) { ConnectorConfig.ProxyProtocol proxyProtocolConfig = connectorConfig.proxyProtocol(); - SslConnectionFactory sslFactory = newSslConnectionFactory(metric, httpFactory); - if (proxyProtocolConfig.enabled()) { - if (proxyProtocolConfig.mixedMode()) { - return List.of(new DetectorConnectionFactory(sslFactory, new ProxyConnectionFactory(sslFactory.getProtocol())), sslFactory, httpFactory); + 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); + } } else { - return List.of(new ProxyConnectionFactory(sslFactory.getProtocol()), sslFactory, httpFactory); + return List.of(sslFactory, alpnFactory, http1Factory, http2Factory); } } else { - return List.of(sslFactory, httpFactory); + 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); + } } } - private HttpConnectionFactory newHttpConnectionFactory() { + 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() { HttpConfiguration httpConfig = new HttpConfiguration(); httpConfig.setSendDateHeader(true); httpConfig.setSendServerVersion(false); @@ -122,16 +156,40 @@ public class ConnectorFactory { if (isSslEffectivelyEnabled(connectorConfig)) { httpConfig.addCustomizer(new SecureRequestCustomizer()); } - return new HttpConnectionFactory(httpConfig); + return httpConfig; + } + + private HttpConnectionFactory newHttp1ConnectionFactory() { + return new HttpConnectionFactory(newHttpConfiguration()); } - private SslConnectionFactory newSslConnectionFactory(Metric metric, HttpConnectionFactory httpFactory) { + private HTTP2ServerConnectionFactory newHttp2ConnectionFactory() { + return new HTTP2ServerConnectionFactory(newHttpConfiguration()); + } + + private SslConnectionFactory newSslConnectionFactory(Metric metric, ConnectionFactory wrappedFactory) { SslContextFactory ctxFactory = sslContextFactoryProvider.getInstance(connectorConfig.name(), connectorConfig.listenPort()); - SslConnectionFactory connectionFactory = new SslConnectionFactory(ctxFactory, httpFactory.getProtocol()); + SslConnectionFactory connectionFactory = new SslConnectionFactory(ctxFactory, wrappedFactory.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 1e2686aa184..a9639ba4da7 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.JDiscHttpServlet.getConnector; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; /** * Resolve request/response filter (chain) based on {@link FilterBindings}. @@ -36,38 +36,38 @@ class FilterResolver { this.strictFiltering = strictFiltering; } - Optional resolveRequestFilter(HttpServletRequest servletRequest, URI jdiscUri) { - Optional maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(servletRequest).listenPort()); + Optional resolveRequestFilter(Request request, URI jdiscUri) { + Optional maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(request).listenPort()); if (maybeFilterId.isPresent()) { - metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); - servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); + request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); } else if (!strictFiltering) { - metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(servletRequest, null)); + metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(request, null)); } else { String syntheticFilterId = RejectingRequestFilter.SYNTHETIC_FILTER_CHAIN_ID; - metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, syntheticFilterId)); - servletRequest.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, syntheticFilterId)); + request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); return Optional.of(RejectingRequestFilter.INSTANCE); } return maybeFilterId.map(bindings::getRequestFilter); } - Optional resolveResponseFilter(HttpServletRequest servletRequest, URI jdiscUri) { - Optional maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(servletRequest).listenPort()); + Optional resolveResponseFilter(Request request, URI jdiscUri) { + Optional maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(request).listenPort()); if (maybeFilterId.isPresent()) { - metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); - servletRequest.setAttribute(ServletRequest.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); + metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); + request.setAttribute(ServletRequest.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); } else { - metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(servletRequest, null)); + metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(request, null)); } return maybeFilterId.map(bindings::getResponseFilter); } - private Metric.Context createMetricContext(HttpServletRequest request, String filterId) { + private Metric.Context createMetricContext(Request request, String filterId) { Map extraDimensions = filterId != null ? Map.of(MetricDefinitions.FILTER_CHAIN_ID_DIMENSION, filterId) : Map.of(); - return JDiscHttpServlet.getConnector(request).createRequestMetricContext(request, extraDimensions); + return 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 de768f979a1..a487b63ef10 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,7 +15,6 @@ 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; @@ -42,11 +41,11 @@ class FilteringRequestHandler extends AbstractRequestHandler { }; private final FilterResolver filterResolver; - private final HttpServletRequest servletRequest; + private final org.eclipse.jetty.server.Request jettyRequest; - public FilteringRequestHandler(FilterResolver filterResolver, HttpServletRequest servletRequest) { + public FilteringRequestHandler(FilterResolver filterResolver, org.eclipse.jetty.server.Request jettyRequest) { this.filterResolver = filterResolver; - this.servletRequest = servletRequest; + this.jettyRequest = jettyRequest; } @Override @@ -54,9 +53,9 @@ class FilteringRequestHandler extends AbstractRequestHandler { Preconditions.checkArgument(request instanceof HttpRequest, "Expected HttpRequest, got " + request); Objects.requireNonNull(originalResponseHandler, "responseHandler"); - RequestFilter requestFilter = filterResolver.resolveRequestFilter(servletRequest, request.getUri()) + RequestFilter requestFilter = filterResolver.resolveRequestFilter(jettyRequest, request.getUri()) .orElse(null); - ResponseFilter responseFilter = filterResolver.resolveResponseFilter(servletRequest, request.getUri()) + ResponseFilter responseFilter = filterResolver.resolveResponseFilter(jettyRequest, 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 0f7ce77e4cd..8b6192bb455 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.HttpServletRequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.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(servletRequest); + int localPort = getConnectorLocalPort(request); 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 05715b13d10..7828751df5a 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,7 +14,6 @@ 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; @@ -34,8 +33,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.HttpServletRequestUtils.getConnection; -import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getHttp1Connection; import static com.yahoo.yolean.Exceptions.throwUnchecked; /** @@ -72,7 +71,7 @@ class HttpRequestDispatch { jDiscContext.janitor, metricReporter, jDiscContext.developerMode()); - markConnectionAsNonPersistentIfThresholdReached(servletRequest); + markHttp1ConnectionAsNonPersistentIfThresholdReached(jettyRequest); this.async = servletRequest.startAsync(); async.setTimeout(0); metricReporter.uriLength(jettyRequest.getOriginalURI().length()); @@ -139,22 +138,24 @@ class HttpRequestDispatch { }; } - private static void markConnectionAsNonPersistentIfThresholdReached(HttpServletRequest request) { + private static void markHttp1ConnectionAsNonPersistentIfThresholdReached(Request request) { ConnectorConfig connectorConfig = getConnector(request).connectorConfig(); int maxRequestsPerConnection = connectorConfig.maxRequestsPerConnection(); if (maxRequestsPerConnection > 0) { - HttpConnection connection = getConnection(request); - if (connection.getMessagesIn() >= maxRequestsPerConnection) { - connection.getGenerator().setPersistent(false); - } + getHttp1Connection(request).ifPresent(connection -> { + if (connection.getMessagesIn() >= maxRequestsPerConnection) { + connection.getGenerator().setPersistent(false); + } + }); } double maxConnectionLifeInSeconds = connectorConfig.maxConnectionLife(); if (maxConnectionLifeInSeconds > 0) { - HttpConnection connection = getConnection(request); - Instant expireAt = Instant.ofEpochMilli((long)(connection.getCreatedTimeStamp() + maxConnectionLifeInSeconds * 1000)); - if (Instant.now().isAfter(expireAt)) { - connection.getGenerator().setPersistent(false); - } + getHttp1Connection(request).ifPresent(connection -> { + Instant expireAt = Instant.ofEpochMilli((long) (connection.getCreatedTimeStamp() + maxConnectionLifeInSeconds * 1000)); + if (Instant.now().isAfter(expireAt)) { + connection.getGenerator().setPersistent(false); + } + }); } } @@ -212,7 +213,7 @@ class HttpRequestDispatch { AccessLogEntry accessLogEntry, HttpServletRequest servletRequest) { RequestHandler requestHandler = wrapHandlerIfFormPost( - new FilteringRequestHandler(context.filterResolver, servletRequest), + new FilteringRequestHandler(context.filterResolver, (Request)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 e8d37cfadb5..8b223c45827 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,6 +4,7 @@ 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; @@ -13,8 +14,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.HttpServletRequestUtils.getConnection; -import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnection; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnectorLocalPort; /** * @author Simon Thoresen Hult @@ -30,7 +31,7 @@ class HttpRequestFactory { HttpRequest.Method.valueOf(servletRequest.getMethod()), HttpRequest.Version.fromString(servletRequest.getProtocol()), new InetSocketAddress(servletRequest.getRemoteAddr(), servletRequest.getRemotePort()), - getConnection(servletRequest).getCreatedTimeStamp()); + getConnection((Request) servletRequest).getCreatedTimeStamp()); httpRequest.context().put(ServletRequest.JDISC_REQUEST_X509CERT, getCertChain(servletRequest)); return httpRequest; } catch (Utf8Appendable.NotUtf8Exception e) { @@ -43,7 +44,7 @@ class HttpRequestFactory { try { String scheme = servletRequest.getScheme(); String host = servletRequest.getServerName(); - int port = getConnectorLocalPort(servletRequest); + int port = getConnectorLocalPort((Request) 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 deleted file mode 100644 index e7b9f459d2e..00000000000 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpServletRequestUtils.java +++ /dev/null @@ -1,38 +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.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 a89c115a1c2..bff300431ce 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,6 +4,7 @@ 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; @@ -26,7 +27,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; -import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; import static com.yahoo.yolean.Exceptions.throwUnchecked; /** @@ -77,7 +78,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, uri) + jDiscContext.filterResolver.resolveResponseFilter((Request)request, uri) .map(responseFilter -> new OneTimeRunnable(() -> filterInvoker.invokeResponseFilterChain(responseFilter, uri, request, response))); @@ -107,7 +108,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, uri).orElse(null); + RequestFilter requestFilter = jDiscContext.filterResolver.resolveRequestFilter((Request)request, uri).orElse(null); if (requestFilter == null) return request; @@ -134,7 +135,7 @@ class JDiscFilterInvokerFilter implements Filter { final AccessLogEntry accessLogEntry = null; // Not used in this context. return new HttpRequestDispatch(jDiscContext, accessLogEntry, - getConnector(request).createRequestMetricContext(request, Map.of()), + getConnector((Request)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 41a1ffc2709..7e1445ffa4f 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,6 +5,7 @@ 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; @@ -20,7 +21,7 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.jdisc.http.server.jetty.HttpServletRequestUtils.getConnection; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; /** * @author Simon Thoresen Hult @@ -85,7 +86,7 @@ class JDiscHttpServlet extends HttpServlet { @Override protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - request.setAttribute(JDiscServerConnector.REQUEST_ATTRIBUTE, getConnector(request)); + request.setAttribute(JDiscServerConnector.REQUEST_ATTRIBUTE, getConnector((Request) request)); Metric.Context metricContext = getMetricContext(request); context.metric.add(MetricDefinitions.NUM_REQUESTS, 1, metricContext); @@ -103,10 +104,6 @@ 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 new file mode 100644 index 00000000000..5fca7a8d778 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java @@ -0,0 +1,51 @@ +// 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 e32c9d46deb..dad274ae520 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.HttpServletRequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.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(servletRequest); + int localPort = getConnectorLocalPort(request); 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 10a6c4702b5..7299ab4b500 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.HttpServletRequestUtils.getConnectorLocalPort; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.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(servletRequest) + && !isRequestToWhitelistedBinding(request) && !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(HttpServletRequest servletRequest) { - int localPort = getConnectorLocalPort(servletRequest); + private boolean isRequestToWhitelistedBinding(Request jettyRequest) { + int localPort = getConnectorLocalPort(jettyRequest); List whiteListedPaths = getWhitelistedPathsForPort(localPort); if (whiteListedPaths == null) { return true; // enforcer not enabled } // Note: Same path definition as HttpRequestFactory.getUri() - return whiteListedPaths.contains(servletRequest.getRequestURI()); + return whiteListedPaths.contains(jettyRequest.getRequestURI()); } private List getWhitelistedPathsForPort(int localPort) { -- cgit v1.2.3