diff options
| author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2021-12-03 08:11:22 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-12-03 08:11:22 +0100 |
| commit | 258c5987675c7b757c8c574a59e1793d1f68ea72 (patch) | |
| tree | 8bd0929c62fbd2cf4ad761101ffb5df6bcce79fa | |
| parent | 6e2ddaa1b917dadfda06f882a08c69a3c6b56558 (diff) | |
Revert "Remove Servlet integration from container-core [run-systemtest]"
45 files changed, 2449 insertions, 62 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiHandler.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiHandler.java index fdb598639a7..de9ae889e2d 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiHandler.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiHandler.java @@ -6,8 +6,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.container.jdisc.secretstore.SecretStore; -import com.yahoo.jdisc.http.server.jetty.RequestUtils; - +import com.yahoo.jdisc.http.servlet.ServletRequest; import java.util.logging.Level; import com.yahoo.restapi.ErrorResponse; import com.yahoo.restapi.Path; @@ -169,7 +168,7 @@ public class CertificateAuthorityApiHandler extends LoggingRequestHandler { } private List<X509Certificate> getRequestCertificateChain(HttpRequest request) { - return Optional.ofNullable(request.getJDiscRequest().context().get(RequestUtils.JDISC_REQUEST_X509CERT)) + return Optional.ofNullable(request.getJDiscRequest().context().get(ServletRequest.JDISC_REQUEST_X509CERT)) .map(X509Certificate[].class::cast) .map(Arrays::asList) .orElse(Collections.emptyList()); diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java index 7bfc4ad41a4..03ff057fa11 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.ca.restapi; import com.yahoo.application.container.handler.Request; -import com.yahoo.jdisc.http.server.jetty.RequestUtils; +import com.yahoo.jdisc.http.servlet.ServletRequest; import com.yahoo.security.KeyAlgorithm; import com.yahoo.security.KeyUtils; import com.yahoo.security.Pkcs10Csr; @@ -95,7 +95,7 @@ public class CertificateAuthorityApiTest extends ContainerTester { instanceRefreshJson(csr), Request.Method.POST, principal); - request.getAttributes().put(RequestUtils.JDISC_REQUEST_X509CERT, new X509Certificate[]{certificate}); + request.getAttributes().put(ServletRequest.JDISC_REQUEST_X509CERT, new X509Certificate[]{certificate}); assertIdentityResponse(request); // POST instance refresh with ZTS client @@ -136,7 +136,7 @@ public class CertificateAuthorityApiTest extends ContainerTester { instanceRefreshJson(csr), Request.Method.POST, principal); - request.getAttributes().put(RequestUtils.JDISC_REQUEST_X509CERT, new X509Certificate[]{cert}); + request.getAttributes().put(ServletRequest.JDISC_REQUEST_X509CERT, new X509Certificate[]{cert}); assertResponse( 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"POST http://localhost:12345/ca/v1/instance/vespa.external.provider_prod_us-north-1/vespa.external/tenant/foobar failed: Mismatch between instance ID in URL path and instance ID in CSR [instanceId=foobar,instanceIdFromCsr=1.cluster1.default.app1.tenant1.us-north-1.prod.node]\"}", diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/PrincipalFromHeaderFilter.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/PrincipalFromHeaderFilter.java index df98ba75dd2..9ed8102190c 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/PrincipalFromHeaderFilter.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/PrincipalFromHeaderFilter.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.ca.restapi.mock; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; -import com.yahoo.jdisc.http.server.jetty.RequestUtils; +import com.yahoo.jdisc.http.servlet.ServletRequest; import com.yahoo.security.X509CertificateUtils; import com.yahoo.text.StringUtilities; import com.yahoo.vespa.athenz.api.AthenzPrincipal; @@ -28,7 +28,7 @@ public class PrincipalFromHeaderFilter implements SecurityRequestFilter { Optional<String> certificate = Optional.ofNullable(request.getHeader("CERTIFICATE")); certificate.ifPresent(cert -> { var x509cert = X509CertificateUtils.fromPem(StringUtilities.unescape(cert)); - request.setAttribute(RequestUtils.JDISC_REQUEST_X509CERT, new X509Certificate[]{x509cert}); + request.setAttribute(ServletRequest.JDISC_REQUEST_X509CERT, new X509Certificate[]{x509cert}); }); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index c4f506d81ba..f69b08ff300 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -25,6 +25,7 @@ import com.yahoo.container.jdisc.state.StateHandler; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.usability.BindingsOverviewHandler; import com.yahoo.document.config.DocumentmanagerConfig; +import com.yahoo.jdisc.http.filter.SecurityFilterInvoker; import com.yahoo.metrics.simple.runtime.MetricProperties; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.prelude.semantics.SemanticRulesConfig; @@ -175,6 +176,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> addSimpleComponent(AccessLog.class); addComponent(new DefaultThreadpoolProvider(this, deployState.featureFlags().metricsproxyNumThreads())); addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class); + addSimpleComponent(SecurityFilterInvoker.class); addSimpleComponent("com.yahoo.container.jdisc.metric.MetricConsumerProviderProvider"); addSimpleComponent("com.yahoo.container.jdisc.metric.MetricProvider"); addSimpleComponent("com.yahoo.container.jdisc.metric.MetricUpdater"); diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index 467f22b95ed..8c0f3e5fd80 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -1661,7 +1661,9 @@ }, "com.yahoo.jdisc.http.HttpRequest": { "superClass": "com.yahoo.jdisc.Request", - "interfaces": [], + "interfaces": [ + "com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest" + ], "attributes": [ "public" ], @@ -1721,7 +1723,9 @@ }, "com.yahoo.jdisc.http.HttpResponse": { "superClass": "com.yahoo.jdisc.Response", - "interfaces": [], + "interfaces": [ + "com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse" + ], "attributes": [ "public" ], @@ -2168,7 +2172,7 @@ "abstract" ], "methods": [ - "public void <init>(com.yahoo.jdisc.http.HttpRequest)", + "public void <init>(com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest)", "public abstract java.lang.String getMethod()", "public com.yahoo.jdisc.http.HttpRequest$Version getVersion()", "public java.net.URI getUri()", @@ -2251,7 +2255,7 @@ "abstract" ], "methods": [ - "public void <init>(com.yahoo.jdisc.http.HttpResponse)", + "public void <init>(com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse)", "public java.util.Enumeration getAttributeNames()", "public java.lang.Object getAttribute(java.lang.String)", "public void setAttribute(java.lang.String, java.lang.Object)", @@ -2416,6 +2420,21 @@ "methods": [], "fields": [] }, + "com.yahoo.jdisc.http.filter.SecurityFilterInvoker": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.jdisc.http.server.jetty.FilterInvoker" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>()", + "public javax.servlet.http.HttpServletRequest invokeRequestFilterChain(com.yahoo.jdisc.http.filter.RequestFilter, java.net.URI, javax.servlet.http.HttpServletRequest, com.yahoo.jdisc.handler.ResponseHandler)", + "public void invokeResponseFilterChain(com.yahoo.jdisc.http.filter.ResponseFilter, java.net.URI, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)" + ], + "fields": [] + }, "com.yahoo.jdisc.http.filter.SecurityRequestFilter": { "superClass": "java.lang.Object", "interfaces": [ diff --git a/container-core/src/main/java/com/yahoo/container/servlet/ServletProvider.java b/container-core/src/main/java/com/yahoo/container/servlet/ServletProvider.java new file mode 100644 index 00000000000..aabaa6dd378 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/servlet/ServletProvider.java @@ -0,0 +1,28 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.servlet; + +import javax.servlet.Servlet; + +import com.yahoo.container.di.componentgraph.Provider; +import org.eclipse.jetty.servlet.ServletHolder; + +/** + * @author stiankri + */ +public class ServletProvider implements Provider<ServletHolder> { + + private final ServletHolder servletHolder; + + public ServletProvider(Servlet servlet, ServletConfigConfig servletConfigConfig) { + servletHolder = new ServletHolder(servlet); + servletConfigConfig.map().forEach( (key, value) -> servletHolder.setInitParameter(key, value)); + } + + @Override + public ServletHolder get() { + return servletHolder; + } + + @Override + public void deconstruct() { } +} diff --git a/container-core/src/main/java/com/yahoo/container/servlet/package-info.java b/container-core/src/main/java/com/yahoo/container/servlet/package-info.java index 8ecb3cbe827..38c03998bf7 100644 --- a/container-core/src/main/java/com/yahoo/container/servlet/package-info.java +++ b/container-core/src/main/java/com/yahoo/container/servlet/package-info.java @@ -1,5 +1,4 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -// TODO Vespa 8 Remove export package @ExportPackage package com.yahoo.container.servlet; 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 598a924b327..387290065c9 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 @@ -7,6 +7,7 @@ import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseHandler; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest; import com.yahoo.jdisc.service.CurrentContainer; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.util.MultiMap; @@ -30,7 +31,7 @@ import java.util.concurrent.TimeUnit; * @author Anirudha Khanna * @author Einar M R Rosenvinge */ -public class HttpRequest extends Request { +public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { public enum Method { OPTIONS, @@ -140,6 +141,7 @@ public class HttpRequest extends Request { } /** Returns the remote address, or null if unresolved */ + @Override public String getRemoteHostAddress() { if (remoteAddress instanceof InetSocketAddress) { InetAddress remoteInetAddress = ((InetSocketAddress) remoteAddress).getAddress(); @@ -152,6 +154,7 @@ public class HttpRequest extends Request { } } + @Override public String getRemoteHostName() { if (remoteAddress instanceof InetSocketAddress) { InetAddress remoteInetAddress = ((InetSocketAddress) remoteAddress).getAddress(); @@ -163,6 +166,7 @@ public class HttpRequest extends Request { } } + @Override public int getRemotePort() { if (remoteAddress instanceof InetSocketAddress) return ((InetSocketAddress) remoteAddress).getPort(); @@ -198,6 +202,7 @@ public class HttpRequest extends Request { * @param unit the unit to return the time in * @return the timestamp of when the underlying HTTP channel was connected, or request creation time */ + @Override public long getConnectedAt(TimeUnit unit) { return unit.convert(connectedAt, TimeUnit.MILLISECONDS); } @@ -229,6 +234,7 @@ public class HttpRequest extends Request { return parameters; } + @Override public void copyHeaders(HeaderFields target) { target.addAll(headers()); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/HttpResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/HttpResponse.java index 20abb251c74..e9ff60ade20 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/HttpResponse.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/HttpResponse.java @@ -7,6 +7,7 @@ import com.yahoo.jdisc.Response; import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; import com.yahoo.jdisc.handler.ResponseHandler; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse; import java.util.List; @@ -15,7 +16,7 @@ import java.util.List; * * @author Einar M R Rosenvinge */ -public class HttpResponse extends Response { +public class HttpResponse extends Response implements ServletOrJdiscHttpResponse { private final HeaderFields trailers = new HeaderFields(); private boolean chunkedEncodingEnabled = true; @@ -53,10 +54,12 @@ public class HttpResponse extends Response { return message; } + @Override public void copyHeaders(HeaderFields target) { target.addAll(headers()); } + @Override public List<Cookie> decodeSetCookieHeader() { return CookieHelper.decodeSetCookieHeader(headers()); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java index 9b799e3a68f..a0933484f4f 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java @@ -7,6 +7,7 @@ import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.HttpRequest.Version; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest; import java.net.InetSocketAddress; import java.net.URI; @@ -36,7 +37,7 @@ public abstract class DiscFilterRequest { protected static final int DEFAULT_HTTP_PORT = 80; protected static final int DEFAULT_HTTPS_PORT = 443; - private final HttpRequest parent; + private final ServletOrJdiscHttpRequest parent; protected final Map<String, List<String>> untreatedParams; private final HeaderFields untreatedHeaders; private List<Cookie> untreatedCookies = null; @@ -44,7 +45,7 @@ public abstract class DiscFilterRequest { private String[] roles = null; private boolean overrideIsUserInRole = false; - public DiscFilterRequest(HttpRequest parent) { + public DiscFilterRequest(ServletOrJdiscHttpRequest parent) { this.parent = parent; // save untreated headers from parent diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterResponse.java index c187a2f032f..fad0f46402d 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterResponse.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterResponse.java @@ -1,10 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; -import com.yahoo.jdisc.HeaderFields; -import com.yahoo.jdisc.http.Cookie; -import com.yahoo.jdisc.http.HttpResponse; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -12,6 +8,14 @@ import java.util.Collections; import java.util.Enumeration; import java.util.List; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse; + +import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.http.Cookie; + + +import com.yahoo.jdisc.http.HttpResponse; + /** * This class was made abstract from 5.27. Test cases that need * a concrete instance should create a {@link JdiscFilterResponse}. @@ -20,11 +24,11 @@ import java.util.List; */ public abstract class DiscFilterResponse { - private final HttpResponse parent; + private final ServletOrJdiscHttpResponse parent; private final HeaderFields untreatedHeaders; private final List<Cookie> untreatedCookies; - public DiscFilterResponse(HttpResponse parent) { + public DiscFilterResponse(ServletOrJdiscHttpResponse parent) { this.parent = parent; this.untreatedHeaders = new HeaderFields(); @@ -112,7 +116,12 @@ public abstract class DiscFilterResponse { /** * Return the parent HttpResponse */ - public HttpResponse getParentResponse() { return parent; } + public HttpResponse getParentResponse() { + if (parent instanceof HttpResponse) + return (HttpResponse)parent; + throw new UnsupportedOperationException( + "getParentResponse is not supported for " + parent.getClass().getName()); + } public void addCookie(JDiscCookieWrapper cookie) { if(cookie != null) { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java index f78c03f7af7..eaa02680d48 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java @@ -3,7 +3,7 @@ package com.yahoo.jdisc.http.filter; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpRequest; -import com.yahoo.jdisc.http.server.jetty.RequestUtils; +import com.yahoo.jdisc.http.servlet.ServletRequest; import java.net.URI; import java.security.Principal; @@ -120,7 +120,7 @@ public class JdiscFilterRequest extends DiscFilterRequest { @Override public List<X509Certificate> getClientCertificateChain() { - return Optional.ofNullable(parent.context().get(RequestUtils.JDISC_REQUEST_X509CERT)) + return Optional.ofNullable(parent.context().get(ServletRequest.JDISC_REQUEST_X509CERT)) .map(X509Certificate[].class::cast) .map(Arrays::asList) .orElse(Collections.emptyList()); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterResponse.java index 3b035987c8a..ee2e1be3ebf 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterResponse.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterResponse.java @@ -1,12 +1,15 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter; +import com.yahoo.jdisc.HeaderFields; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.CookieHelper; import com.yahoo.jdisc.http.HttpResponse; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse; import java.util.List; +import java.util.Map; /** * JDisc implementation of a filter request. @@ -16,7 +19,8 @@ class JdiscFilterResponse extends DiscFilterResponse { private final Response parent; JdiscFilterResponse(Response parent) { - super((HttpResponse)parent); + // A separate adapter is required as DiscFilterResponse will invoke methods from ServletOrJdiscHttpResponse parameter in its constructor + super(parent instanceof HttpResponse ? (HttpResponse)parent : new Adapter(parent)); this.parent = parent; } @@ -64,4 +68,17 @@ class JdiscFilterResponse extends DiscFilterResponse { CookieHelper.encodeSetCookieHeader(parent.headers(), cookies); } + private static class Adapter implements ServletOrJdiscHttpResponse { + private final Response response; + + Adapter(Response response) { + this.response = response; + } + + @Override public void copyHeaders(HeaderFields target) { target.addAll(response.headers()); } + @Override public int getStatus() { return response.getStatus(); } + @Override public Map<String, Object> context() { return response.context(); } + @Override public List<Cookie> decodeSetCookieHeader() { return CookieHelper.decodeSetCookieHeader(response.headers()); } + } + } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityFilterInvoker.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityFilterInvoker.java new file mode 100644 index 00000000000..a0b9ec935cb --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/SecurityFilterInvoker.java @@ -0,0 +1,108 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.filter; + +import com.google.common.annotations.Beta; +import com.yahoo.jdisc.handler.ResponseHandler; +import com.yahoo.jdisc.http.HttpRequest.Method; +import com.yahoo.jdisc.http.servlet.ServletRequest; + +import com.yahoo.jdisc.http.servlet.ServletResponse; +import com.yahoo.jdisc.http.server.jetty.FilterInvoker; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.net.URI; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +/** + * Only intended for internal vespa use. + * + * Runs JDisc security filter without using JDisc request/response. + * Only intended to be used in a servlet context, as the error messages are tailored for that. + * + * Assumes that SecurityResponseFilters mutate DiscFilterResponse in the thread they are invoked from. + * + * @author Tony Vaagenes + */ +@Beta +public class SecurityFilterInvoker implements FilterInvoker { + + /** + * Returns the servlet request to be used in any servlets invoked after this. + */ + @Override + public HttpServletRequest invokeRequestFilterChain(RequestFilter requestFilterChain, + URI uri, HttpServletRequest httpRequest, + ResponseHandler responseHandler) { + + SecurityRequestFilterChain securityChain = cast(SecurityRequestFilterChain.class, requestFilterChain). + orElseThrow(SecurityFilterInvoker::newUnsupportedOperationException); + + ServletRequest wrappedRequest = new ServletRequest(httpRequest, uri); + securityChain.filter(new ServletFilterRequest(wrappedRequest), responseHandler); + return wrappedRequest; + } + + @Override + public void invokeResponseFilterChain( + ResponseFilter responseFilterChain, + URI uri, + HttpServletRequest request, + HttpServletResponse response) { + + SecurityResponseFilterChain securityChain = cast(SecurityResponseFilterChain.class, responseFilterChain). + orElseThrow(SecurityFilterInvoker::newUnsupportedOperationException); + + ServletFilterResponse wrappedResponse = new ServletFilterResponse(new ServletResponse(response)); + securityChain.filter(new ServletRequestView(uri, request), wrappedResponse); + } + + private static UnsupportedOperationException newUnsupportedOperationException() { + return new UnsupportedOperationException( + "Filter type not supported. If a request is handled by servlets or jax-rs, then any filters invoked for that request must be security filters."); + } + + private <T> Optional<T> cast(Class<T> securityFilterChainClass, Object filter) { + return (securityFilterChainClass.isInstance(filter))? + Optional.of(securityFilterChainClass.cast(filter)): + Optional.empty(); + } + + private static class ServletRequestView implements RequestView { + private final HttpServletRequest request; + private final URI uri; + + public ServletRequestView(URI uri, HttpServletRequest request) { + this.request = request; + this.uri = uri; + } + + @Override + public Object getAttribute(String name) { + return request.getAttribute(name); + } + + @Override + public List<String> getHeaders(String name) { + return Collections.unmodifiableList(Collections.list(request.getHeaders(name))); + } + + @Override + public Optional<String> getFirstHeader(String name) { + return getHeaders(name).stream().findFirst(); + } + + @Override + public Optional<Method> getMethod() { + return Optional.of(Method.valueOf(request.getMethod())); + } + + @Override + public URI getUri() { + return uri; + } + } + +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java new file mode 100644 index 00000000000..c27c0e56d30 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java @@ -0,0 +1,170 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.filter; + +import com.yahoo.jdisc.http.HttpHeaders; +import com.yahoo.jdisc.http.servlet.ServletRequest; + +import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.security.Principal; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +/** + * Servlet implementation for JDisc filter requests. + */ +class ServletFilterRequest extends DiscFilterRequest { + + private final ServletRequest parent; + + public ServletFilterRequest(ServletRequest parent) { + super(parent); + this.parent = parent; + } + + ServletRequest getServletRequest() { + return parent; + } + + @Deprecated + public void setUri(URI uri) { + parent.setUri(uri); + } + + @Override + public String getMethod() { + return parent.getRequest().getMethod(); + } + + @Override + public void setRemoteAddr(String remoteIpAddress) { + throw new UnsupportedOperationException( + "Setting remote address is not supported for " + this.getClass().getName()); + } + + @Override + public Enumeration<String> getAttributeNames() { + Set<String> names = new HashSet<>(Collections.list(super.getAttributeNames())); + names.addAll(Collections.list(parent.getRequest().getAttributeNames())); + return Collections.enumeration(names); + } + + @Override + public Object getAttribute(String name) { + Object jdiscAttribute = super.getAttribute(name); + return jdiscAttribute != null ? + jdiscAttribute : + parent.getRequest().getAttribute(name); + } + + @Override + public void setAttribute(String name, Object value) { + super.setAttribute(name, value); + parent.getRequest().setAttribute(name, value); + } + + @Override + public boolean containsAttribute(String name) { + return super.containsAttribute(name) + || parent.getRequest().getAttribute(name) != null; + } + + @Override + public void removeAttribute(String name) { + super.removeAttribute(name); + parent.getRequest().removeAttribute(name); + } + + @Override + public String getParameter(String name) { + return parent.getParameter(name); + } + + @Override + public Enumeration<String> getParameterNames() { + return parent.getParameterNames(); + } + + @Override + public void addHeader(String name, String value) { + parent.addHeader(name, value); + } + + @Override + public String getHeader(String name) { + return parent.getHeader(name); + } + + @Override + public Enumeration<String> getHeaderNames() { + return parent.getHeaderNames(); + } + + public List<String> getHeaderNamesAsList() { + return Collections.list(getHeaderNames()); + } + + @Override + public Enumeration<String> getHeaders(String name) { + return parent.getHeaders(name); + } + + @Override + public List<String> getHeadersAsList(String name) { + return Collections.list(getHeaders(name)); + } + + @Override + public void setHeaders(String name, String value) { + parent.setHeaders(name, value); + } + + @Override + public void setHeaders(String name, List<String> values) { + parent.setHeaders(name, values); + } + + @Override + public Principal getUserPrincipal() { + return parent.getUserPrincipal(); + } + + @Override + public void setUserPrincipal(Principal principal) { + parent.setUserPrincipal(principal); + } + + @Override + public List<X509Certificate> getClientCertificateChain() { + return Optional.ofNullable(parent.getRequest().getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT)) + .map(X509Certificate[].class::cast) + .map(Arrays::asList) + .orElse(Collections.emptyList()); + } + + @Override + public void removeHeaders(String name) { + parent.removeHeaders(name); + } + + @Override + public void clearCookies() { + parent.removeHeaders(HttpHeaders.Names.COOKIE); + } + + @Override + public void setCharacterEncoding(String encoding) { + super.setCharacterEncoding(encoding); + try { + parent.setCharacterEncoding(encoding); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException("Encoding not supported: " + encoding, e); + } + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java new file mode 100644 index 00000000000..b706e5a7ec6 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.filter; + +import com.google.common.collect.Iterables; +import com.yahoo.jdisc.http.Cookie; +import com.yahoo.jdisc.http.HttpHeaders; +import com.yahoo.jdisc.http.servlet.ServletResponse; + +import javax.servlet.http.HttpServletResponse; +import java.util.Collection; +import java.util.List; + +/** + * Servlet implementation for JDisc filter responses. + */ +class ServletFilterResponse extends DiscFilterResponse { + + private final ServletResponse parent; + + public ServletFilterResponse(ServletResponse parent) { + super(parent); + this.parent = parent; + } + + ServletResponse getServletResponse() { + return parent; + } + + public void setStatus(int status) { + parent.setStatus(status); + } + + @Override + public void setHeader(String name, String value) { + parent.setHeader(name, value); + } + + @Override + public void removeHeaders(String name) { + HttpServletResponse parentResponse = parent.getResponse(); + if (parentResponse instanceof org.eclipse.jetty.server.Response) { + org.eclipse.jetty.server.Response jettyResponse = (org.eclipse.jetty.server.Response)parentResponse; + jettyResponse.getHttpFields().remove(name); + } else { + throw new UnsupportedOperationException( + "Cannot remove headers for response of type " + parentResponse.getClass().getName()); + } + } + + // Why have a setHeaders that takes a single string? + @Override + public void setHeaders(String name, String value) { + parent.setHeader(name, value); + } + + @Override + public void setHeaders(String name, List<String> values) { + for (String value : values) + parent.addHeader(name, value); + } + + @Override + public void addHeader(String name, String value) { + parent.addHeader(name, value); + } + + @Override + public String getHeader(String name) { + Collection<String> headers = parent.getHeaders(name); + return headers.isEmpty() + ? null + : Iterables.getLast(headers); + } + + @Override + public void setCookies(List<Cookie> cookies) { + removeHeaders(HttpHeaders.Names.SET_COOKIE); + List<String> setCookieHeaders = Cookie.toSetCookieHeaders(cookies); + setCookieHeaders.forEach(cookie -> addHeader(HttpHeaders.Names.SET_COOKIE, cookie)); + } +} 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 b41c80a471c..2eea7f155ee 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 @@ -8,6 +8,7 @@ import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.http.servlet.ServletRequest; import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.server.HttpTransportOverHTTP2; import org.eclipse.jetty.server.HttpChannel; @@ -85,10 +86,10 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty addNonNullValue(builder, jdiscRequest.getUserPrincipal(), RequestLogEntry.Builder::userPrincipal); } - String requestFilterId = (String) request.getAttribute(RequestUtils.JDISC_REQUEST_CHAIN); + String requestFilterId = (String) request.getAttribute(ServletRequest.JDISC_REQUEST_CHAIN); addNonNullValue(builder, requestFilterId, (b, chain) -> b.addExtraAttribute("request-chain", chain)); - String responseFilterId = (String) request.getAttribute(RequestUtils.JDISC_RESPONSE_CHAIN); + String responseFilterId = (String) request.getAttribute(ServletRequest.JDISC_RESPONSE_CHAIN); addNonNullValue(builder, responseFilterId, (b, chain) -> b.addExtraAttribute("response-chain", chain)); UUID connectionId = (UUID) request.getAttribute(JettyConnectionLogger.CONNECTION_ID_REQUEST_ATTRIBUTE); @@ -108,7 +109,7 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty builder.addExtraAttribute(header, value); } }); - X509Certificate[] clientCert = (X509Certificate[]) request.getAttribute(RequestUtils.SERVLET_REQUEST_X509CERT); + X509Certificate[] clientCert = (X509Certificate[]) request.getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT); if (clientCert != null && clientCert.length > 0) { builder.sslPrincipal(clientCert[0].getSubjectX500Principal()); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvoker.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvoker.java new file mode 100644 index 00000000000..3c329bbf13b --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvoker.java @@ -0,0 +1,28 @@ +// Copyright Yahoo. 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.google.inject.ImplementedBy; +import com.yahoo.jdisc.handler.ResponseHandler; +import com.yahoo.jdisc.http.filter.RequestFilter; +import com.yahoo.jdisc.http.filter.ResponseFilter; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.net.URI; + +/** + * Separate interface since DiscFilterRequest/Response and Security filter chains are not accessible in this bundle + */ +@ImplementedBy(UnsupportedFilterInvoker.class) +public interface FilterInvoker { + HttpServletRequest invokeRequestFilterChain(RequestFilter requestFilterChain, + URI uri, + HttpServletRequest httpRequest, + ResponseHandler responseHandler); + + void invokeResponseFilterChain( + ResponseFilter responseFilterChain, + URI uri, + HttpServletRequest request, + HttpServletResponse response); +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingPrintWriter.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingPrintWriter.java new file mode 100644 index 00000000000..90b12e64a55 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingPrintWriter.java @@ -0,0 +1,266 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.Writer; +import java.util.Locale; + +/** + * Invokes the response filter the first time anything is output to the underlying PrintWriter. + * The filter must be invoked before the first output call since this might cause the response + * to be committed, i.e. locked and potentially put on the wire. + * Any changes to the response after it has been committed might be ignored or cause exceptions. + * @author Tony Vaagenes + */ +final class FilterInvokingPrintWriter extends PrintWriter { + private final PrintWriter delegate; + private final OneTimeRunnable filterInvoker; + + public FilterInvokingPrintWriter(PrintWriter delegate, OneTimeRunnable filterInvoker) { + /* The PrintWriter class both + * 1) exposes new methods, the PrintWriter "interface" + * 2) implements PrintWriter and Writer methods that does some extra things before calling down to the writer methods. + * If super was invoked with the delegate PrintWriter, the superclass would behave as a PrintWriter(PrintWriter), + * i.e. the extra things in 2. would be done twice. + * To avoid this, all the methods of PrintWriter are overridden with versions that forward directly to the underlying delegate + * instead of going through super. + * The super class is initialized with a non-functioning writer to catch mistakenly non-overridden methods. + */ + super(new Writer() { + @Override + public void write(char[] cbuf, int off, int len) throws IOException { + throwAssertionError(); + } + + private void throwAssertionError() { + throw new AssertionError(FilterInvokingPrintWriter.class.getName() + " failed to delegate to the underlying writer"); + } + + @Override + public void flush() throws IOException { + throwAssertionError(); + } + + @Override + public void close() throws IOException { + throwAssertionError(); + } + }); + + this.delegate = delegate; + this.filterInvoker = filterInvoker; + } + + @Override + public String toString() { + return getClass().getName() + " (" + super.toString() + ")"; + } + + private void runFilterIfFirstInvocation() { + filterInvoker.runIfFirstInvocation(); + } + + @Override + public void flush() { + runFilterIfFirstInvocation(); + delegate.flush(); + } + + @Override + public void close() { + runFilterIfFirstInvocation(); + delegate.close(); + } + + @Override + public boolean checkError() { + return delegate.checkError(); + } + + @Override + public void write(int c) { + runFilterIfFirstInvocation(); + delegate.write(c); + } + + @Override + public void write(char[] buf, int off, int len) { + runFilterIfFirstInvocation(); + delegate.write(buf, off, len); + } + + @Override + public void write(char[] buf) { + runFilterIfFirstInvocation(); + delegate.write(buf); + } + + @Override + public void write(String s, int off, int len) { + runFilterIfFirstInvocation(); + delegate.write(s, off, len); + } + + @Override + public void write(String s) { + runFilterIfFirstInvocation(); + delegate.write(s); + } + + @Override + public void print(boolean b) { + runFilterIfFirstInvocation(); + delegate.print(b); + } + + @Override + public void print(char c) { + runFilterIfFirstInvocation(); + delegate.print(c); + } + + @Override + public void print(int i) { + runFilterIfFirstInvocation(); + delegate.print(i); + } + + @Override + public void print(long l) { + runFilterIfFirstInvocation(); + delegate.print(l); + } + + @Override + public void print(float f) { + runFilterIfFirstInvocation(); + delegate.print(f); + } + + @Override + public void print(double d) { + runFilterIfFirstInvocation(); + delegate.print(d); + } + + @Override + public void print(char[] s) { + runFilterIfFirstInvocation(); + delegate.print(s); + } + + @Override + public void print(String s) { + runFilterIfFirstInvocation(); + delegate.print(s); + } + + @Override + public void print(Object obj) { + runFilterIfFirstInvocation(); + delegate.print(obj); + } + + @Override + public void println() { + runFilterIfFirstInvocation(); + delegate.println(); + } + + @Override + public void println(boolean x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(char x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(int x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(long x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(float x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(double x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(char[] x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(String x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public void println(Object x) { + runFilterIfFirstInvocation(); + delegate.println(x); + } + + @Override + public PrintWriter printf(String format, Object... args) { + runFilterIfFirstInvocation(); + return delegate.printf(format, args); + } + + @Override + public PrintWriter printf(Locale l, String format, Object... args) { + runFilterIfFirstInvocation(); + return delegate.printf(l, format, args); + } + + @Override + public PrintWriter format(String format, Object... args) { + runFilterIfFirstInvocation(); + return delegate.format(format, args); + } + + @Override + public PrintWriter format(Locale l, String format, Object... args) { + runFilterIfFirstInvocation(); + return delegate.format(l, format, args); + } + + @Override + public PrintWriter append(CharSequence csq) { + runFilterIfFirstInvocation(); + return delegate.append(csq); + } + + @Override + public PrintWriter append(CharSequence csq, int start, int end) { + runFilterIfFirstInvocation(); + return delegate.append(csq, start, end); + } + + @Override + public PrintWriter append(char c) { + runFilterIfFirstInvocation(); + return delegate.append(c); + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingServletOutputStream.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingServletOutputStream.java new file mode 100644 index 00000000000..d2be107ef86 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterInvokingServletOutputStream.java @@ -0,0 +1,165 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty; + +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import java.io.IOException; + +/** + * Invokes the response filter the first time anything is output to the underlying ServletOutputStream. + * The filter must be invoked before the first output call since this might cause the response + * to be committed, i.e. locked and potentially put on the wire. + * Any changes to the response after it has been committed might be ignored or cause exceptions. + * + * @author Tony Vaagenes + */ +class FilterInvokingServletOutputStream extends ServletOutputStream { + private final ServletOutputStream delegate; + private final OneTimeRunnable filterInvoker; + + public FilterInvokingServletOutputStream(ServletOutputStream delegate, OneTimeRunnable filterInvoker) { + this.delegate = delegate; + this.filterInvoker = filterInvoker; + } + + @Override + public boolean isReady() { + return delegate.isReady(); + } + + @Override + public void setWriteListener(WriteListener writeListener) { + delegate.setWriteListener(writeListener); + } + + + private void runFilterIfFirstInvocation() { + filterInvoker.runIfFirstInvocation(); + } + + @Override + public void write(int b) throws IOException { + runFilterIfFirstInvocation(); + delegate.write(b); + } + + + @Override + public void write(byte[] b) throws IOException { + runFilterIfFirstInvocation(); + delegate.write(b); + } + + @Override + public void print(String s) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(s); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + runFilterIfFirstInvocation(); + delegate.write(b, off, len); + } + + @Override + public void print(boolean b) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(b); + } + + @Override + public void flush() throws IOException { + runFilterIfFirstInvocation(); + delegate.flush(); + } + + @Override + public void print(char c) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(c); + } + + @Override + public void close() throws IOException { + runFilterIfFirstInvocation(); + delegate.close(); + } + + @Override + public void print(int i) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(i); + } + + @Override + public void print(long l) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(l); + } + + @Override + public void print(float f) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(f); + } + + @Override + public void print(double d) throws IOException { + runFilterIfFirstInvocation(); + delegate.print(d); + } + + @Override + public void println() throws IOException { + runFilterIfFirstInvocation(); + delegate.println(); + } + + @Override + public void println(String s) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(s); + } + + @Override + public void println(boolean b) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(b); + } + + @Override + public void println(char c) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(c); + } + + @Override + public void println(int i) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(i); + } + + @Override + public void println(long l) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(l); + } + + @Override + public void println(float f) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(f); + } + + @Override + public void println(double d) throws IOException { + runFilterIfFirstInvocation(); + delegate.println(d); + } + + @Override + public String toString() { + return getClass().getCanonicalName() + " (" + delegate.toString() + ")"; + } +} 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 32def124131..873f336f0c9 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 @@ -10,6 +10,7 @@ import com.yahoo.jdisc.handler.ResponseHandler; 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 java.net.URI; @@ -39,13 +40,13 @@ class FilterResolver { Optional<String> maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(request).listenPort()); if (maybeFilterId.isPresent()) { metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); - request.setAttribute(RequestUtils.JDISC_REQUEST_CHAIN, maybeFilterId.get()); + request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, maybeFilterId.get()); } else if (!strictFiltering) { 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(request, syntheticFilterId)); - request.setAttribute(RequestUtils.JDISC_REQUEST_CHAIN, syntheticFilterId); + request.setAttribute(ServletRequest.JDISC_REQUEST_CHAIN, syntheticFilterId); return Optional.of(RejectingRequestFilter.INSTANCE); } return maybeFilterId.map(bindings::getRequestFilter); @@ -55,7 +56,7 @@ class FilterResolver { Optional<String> maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(request).listenPort()); if (maybeFilterId.isPresent()) { metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); - request.setAttribute(RequestUtils.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); + request.setAttribute(ServletRequest.JDISC_RESPONSE_CHAIN, maybeFilterId.get()); } else { metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(request, null)); } 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 c54fa1cf1b9..52c2a83563e 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 @@ -2,6 +2,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; @@ -31,7 +32,7 @@ class HttpRequestFactory { HttpRequest.Version.fromString(servletRequest.getProtocol()), new InetSocketAddress(servletRequest.getRemoteAddr(), servletRequest.getRemotePort()), getConnection((Request) servletRequest).getCreatedTimeStamp()); - httpRequest.context().put(RequestUtils.JDISC_REQUEST_X509CERT, getCertChain(servletRequest)); + httpRequest.context().put(ServletRequest.JDISC_REQUEST_X509CERT, getCertChain(servletRequest)); servletRequest.setAttribute(HttpRequest.class.getName(), httpRequest); return httpRequest; } catch (Utf8Appendable.NotUtf8Exception e) { 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 new file mode 100644 index 00000000000..0fd4e8c42fb --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscFilterInvokerFilter.java @@ -0,0 +1,302 @@ +// Copyright Yahoo. 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.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; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; +import java.io.IOException; +import java.io.PrintWriter; +import java.net.URI; +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.yolean.Exceptions.throwUnchecked; + +/** + * Runs JDisc security filters for Servlets + * This component is split in two: + * 1) JDiscFilterInvokerFilter, which uses package private methods to support JDisc APIs + * 2) SecurityFilterInvoker, which is intended for use in a servlet context. + * + * @author Tony Vaagenes + */ +class JDiscFilterInvokerFilter implements Filter { + private final JDiscContext jDiscContext; + private final FilterInvoker filterInvoker; + + public JDiscFilterInvokerFilter(JDiscContext jDiscContext, + FilterInvoker filterInvoker) { + this.jDiscContext = jDiscContext; + this.filterInvoker = filterInvoker; + } + + + @Override + public void init(FilterConfig filterConfig) throws ServletException {} + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + HttpServletRequest httpRequest = (HttpServletRequest)request; + HttpServletResponse httpResponse = (HttpServletResponse)response; + + URI uri; + try { + uri = HttpRequestFactory.getUri(httpRequest); + } catch (RequestException e) { + httpResponse.sendError(e.getResponseStatus(), e.getMessage()); + return; + } + + AtomicReference<Boolean> responseReturned = new AtomicReference<>(null); + + HttpServletRequest newRequest = runRequestFilterWithMatchingBinding(responseReturned, uri, httpRequest, httpResponse); + assert newRequest != null; + responseReturned.compareAndSet(null, false); + + if (!responseReturned.get()) { + runChainAndResponseFilters(uri, newRequest, httpResponse, chain); + } + } + + private void runChainAndResponseFilters(URI uri, HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + Optional<OneTimeRunnable> responseFilterInvoker = + jDiscContext.filterResolver.resolveResponseFilter(toJettyRequest(request), uri) + .map(responseFilter -> + new OneTimeRunnable(() -> + filterInvoker.invokeResponseFilterChain(responseFilter, uri, request, response))); + + + HttpServletResponse responseForServlet = responseFilterInvoker + .<HttpServletResponse>map(invoker -> + new FilterInvokingResponseWrapper(response, invoker)) + .orElse(response); + + HttpServletRequest requestForServlet = responseFilterInvoker + .<HttpServletRequest>map(invoker -> + new FilterInvokingRequestWrapper(request, invoker, responseForServlet)) + .orElse(request); + + chain.doFilter(requestForServlet, responseForServlet); + + responseFilterInvoker.ifPresent(invoker -> { + boolean requestHandledSynchronously = !request.isAsyncStarted(); + + if (requestHandledSynchronously) { + invoker.runIfFirstInvocation(); + } + // For async requests, response filters will be invoked on AsyncContext.complete(). + }); + } + + private HttpServletRequest runRequestFilterWithMatchingBinding(AtomicReference<Boolean> responseReturned, URI uri, HttpServletRequest request, HttpServletResponse response) throws IOException { + try { + RequestFilter requestFilter = jDiscContext.filterResolver.resolveRequestFilter(toJettyRequest(request), uri).orElse(null); + if (requestFilter == null) + return request; + + ResponseHandler responseHandler = createResponseHandler(responseReturned, request, response); + return filterInvoker.invokeRequestFilterChain(requestFilter, uri, request, responseHandler); + } catch (Exception e) { + throw new RuntimeException("Failed running request filter chain for uri " + uri, e); + } + } + + private ResponseHandler createResponseHandler(AtomicReference<Boolean> responseReturned, HttpServletRequest httpRequest, HttpServletResponse httpResponse) { + return jdiscResponse -> { + boolean oldValueWasNull = responseReturned.compareAndSet(null, true); + if (!oldValueWasNull) + throw new RuntimeException("Can't return response from filter asynchronously"); + + HttpRequestDispatch requestDispatch = createRequestDispatch(httpRequest, httpResponse); + return requestDispatch.dispatchFilterRequest(jdiscResponse); + }; + } + + private HttpRequestDispatch createRequestDispatch(HttpServletRequest request, HttpServletResponse response) { + try { + final AccessLogEntry accessLogEntry = null; // Not used in this context. + return new HttpRequestDispatch(jDiscContext, + accessLogEntry, + getConnector(toJettyRequest(request)).createRequestMetricContext(request, Map.of()), + request, response); + } catch (IOException e) { + throw throwUnchecked(e); + } + } + + private static Request toJettyRequest(HttpServletRequest request) { + if (request instanceof com.yahoo.jdisc.http.servlet.ServletRequest) { + return (Request) ((com.yahoo.jdisc.http.servlet.ServletRequest)request).getRequest(); + } + return (Request) request; + } + + @Override + public void destroy() {} + + // ServletRequest wrapper that is necessary because we need to wrap AsyncContext. + private static class FilterInvokingRequestWrapper extends HttpServletRequestWrapper { + private final OneTimeRunnable filterInvoker; + private final HttpServletResponse servletResponse; + + public FilterInvokingRequestWrapper( + HttpServletRequest request, + OneTimeRunnable filterInvoker, + HttpServletResponse servletResponse) { + super(request); + this.filterInvoker = filterInvoker; + this.servletResponse = servletResponse; + } + + @Override + public AsyncContext startAsync() { + final AsyncContext asyncContext = super.startAsync(); + return new FilterInvokingAsyncContext(asyncContext, filterInvoker, this, servletResponse); + } + + @Override + public AsyncContext startAsync( + final ServletRequest wrappedRequest, + final ServletResponse wrappedResponse) { + // According to the documentation, the passed request/response parameters here must either + // _be_ or _wrap_ the original request/response objects passed to the servlet - which are + // our wrappers, so no need to wrap again - we can use the user-supplied objects. + final AsyncContext asyncContext = super.startAsync(wrappedRequest, wrappedResponse); + return new FilterInvokingAsyncContext(asyncContext, filterInvoker, this, wrappedResponse); + } + + @Override + public AsyncContext getAsyncContext() { + final AsyncContext asyncContext = super.getAsyncContext(); + return new FilterInvokingAsyncContext(asyncContext, filterInvoker, this, servletResponse); + } + } + + // AsyncContext wrapper that is necessary for two reasons: + // 1) Run response filters when AsyncContext.complete() is called. + // 2) Eliminate paths where application code can get its hands on un-wrapped response object, circumventing + // running of response filters. + private static class FilterInvokingAsyncContext implements AsyncContext { + private final AsyncContext delegate; + private final OneTimeRunnable filterInvoker; + private final ServletRequest servletRequest; + private final ServletResponse servletResponse; + + public FilterInvokingAsyncContext( + AsyncContext delegate, + OneTimeRunnable filterInvoker, + ServletRequest servletRequest, + ServletResponse servletResponse) { + this.delegate = delegate; + this.filterInvoker = filterInvoker; + this.servletRequest = servletRequest; + this.servletResponse = servletResponse; + } + + @Override + public ServletRequest getRequest() { + return servletRequest; + } + + @Override + public ServletResponse getResponse() { + return servletResponse; + } + + @Override + public boolean hasOriginalRequestAndResponse() { + return delegate.hasOriginalRequestAndResponse(); + } + + @Override + public void dispatch() { + delegate.dispatch(); + } + + @Override + public void dispatch(String s) { + delegate.dispatch(s); + } + + @Override + public void dispatch(ServletContext servletContext, String s) { + delegate.dispatch(servletContext, s); + } + + @Override + public void complete() { + // Completing may commit the response, so this is the last chance to run response filters. + filterInvoker.runIfFirstInvocation(); + delegate.complete(); + } + + @Override + public void start(Runnable runnable) { + delegate.start(runnable); + } + + @Override + public void addListener(AsyncListener asyncListener) { + delegate.addListener(asyncListener); + } + + @Override + public void addListener(AsyncListener asyncListener, ServletRequest servletRequest, ServletResponse servletResponse) { + delegate.addListener(asyncListener, servletRequest, servletResponse); + } + + @Override + public <T extends AsyncListener> T createListener(Class<T> aClass) throws ServletException { + return delegate.createListener(aClass); + } + + @Override + public void setTimeout(long l) { + delegate.setTimeout(l); + } + + @Override + public long getTimeout() { + return delegate.getTimeout(); + } + } + + private static class FilterInvokingResponseWrapper extends HttpServletResponseWrapper { + private final OneTimeRunnable filterInvoker; + + public FilterInvokingResponseWrapper(HttpServletResponse response, OneTimeRunnable filterInvoker) { + super(response); + this.filterInvoker = filterInvoker; + } + + @Override + public ServletOutputStream getOutputStream() throws IOException { + ServletOutputStream delegate = super.getOutputStream(); + return new FilterInvokingServletOutputStream(delegate, filterInvoker); + } + + @Override + public PrintWriter getWriter() throws IOException { + PrintWriter delegate = super.getWriter(); + return new FilterInvokingPrintWriter(delegate, filterInvoker); + } + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java index 79cdb8f67cf..0e511fd3eaf 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java @@ -73,7 +73,7 @@ class JDiscServerConnector extends ServerConnector { public Metric.Context createRequestMetricContext(HttpServletRequest request, Map<String, String> extraDimensions) { String method = request.getMethod(); String scheme = request.getScheme(); - boolean clientAuthenticated = request.getAttribute(RequestUtils.SERVLET_REQUEST_X509CERT) != null; + boolean clientAuthenticated = request.getAttribute(com.yahoo.jdisc.http.servlet.ServletRequest.SERVLET_REQUEST_X509CERT) != null; Map<String, Object> dimensions = createConnectorDimensions(listenPort, connectorName); dimensions.put(MetricDefinitions.METHOD_DIMENSION, method); dimensions.put(MetricDefinitions.SCHEME_DIMENSION, scheme); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index fca34f3bbd7..3f2a91c60b5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -2,12 +2,14 @@ package com.yahoo.jdisc.http.server.jetty; import com.google.inject.Inject; +import com.yahoo.component.ComponentId; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.logging.ConnectionLog; import com.yahoo.container.logging.RequestLog; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.jdisc.service.AbstractServerProvider; import com.yahoo.jdisc.service.CurrentContainer; import org.eclipse.jetty.http.HttpField; @@ -22,6 +24,7 @@ import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.server.handler.gzip.GzipHttpOutputInterceptor; +import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.log.JavaUtilLog; @@ -29,12 +32,14 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.thread.QueuedThreadPool; import javax.management.remote.JMXServiceURL; +import javax.servlet.DispatcherType; import java.io.IOException; import java.lang.management.ManagementFactory; import java.net.BindException; import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.EnumSet; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -58,9 +63,12 @@ public class JettyHttpServer extends AbstractServerProvider { public JettyHttpServer(CurrentContainer container, Metric metric, ServerConfig serverConfig, + ServletPathsConfig servletPathsConfig, FilterBindings filterBindings, Janitor janitor, ComponentRegistry<ConnectorFactory> connectorFactories, + ComponentRegistry<ServletHolder> servletHolders, + FilterInvoker filterInvoker, RequestLog requestLog, ConnectionLog connectionLog) { super(container); @@ -90,13 +98,18 @@ public class JettyHttpServer extends AbstractServerProvider { serverConfig); ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(jDiscContext)); + FilterHolder jDiscFilterInvokerFilter = new FilterHolder(new JDiscFilterInvokerFilter(jDiscContext, filterInvoker)); + List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors()) .map(JDiscServerConnector.class::cast) .collect(toList()); server.setHandler(getHandlerCollection(serverConfig, + servletPathsConfig, connectors, - jdiscServlet)); + jdiscServlet, + servletHolders, + jDiscFilterInvokerFilter)); this.metricsReporter = new ServerMetricReporter(metric, server); } @@ -137,9 +150,19 @@ public class JettyHttpServer extends AbstractServerProvider { } private HandlerCollection getHandlerCollection(ServerConfig serverConfig, + ServletPathsConfig servletPathsConfig, List<JDiscServerConnector> connectors, - ServletHolder jdiscServlet) { + ServletHolder jdiscServlet, + ComponentRegistry<ServletHolder> servletHolders, + FilterHolder jDiscFilterInvokerFilter) { ServletContextHandler servletContextHandler = createServletContextHandler(); + + servletHolders.allComponentsById().forEach((id, servlet) -> { + String path = getServletPath(servletPathsConfig, id); + servletContextHandler.addServlet(servlet, path); + servletContextHandler.addFilter(jDiscFilterInvokerFilter, path, EnumSet.allOf(DispatcherType.class)); + }); + servletContextHandler.addServlet(jdiscServlet, "/*"); List<ConnectorConfig> connectorConfigs = connectors.stream().map(JDiscServerConnector::connectorConfig).collect(toList()); @@ -168,6 +191,10 @@ public class JettyHttpServer extends AbstractServerProvider { return handlerCollection; } + private static String getServletPath(ServletPathsConfig servletPathsConfig, ComponentId id) { + return "/" + servletPathsConfig.servlets(id.stringValue()).path(); + } + private ServletContextHandler createServletContextHandler() { ServletContextHandler servletContextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS); servletContextHandler.setContextPath("/"); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/OneTimeRunnable.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/OneTimeRunnable.java new file mode 100644 index 00000000000..24cc41d009f --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/OneTimeRunnable.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * @author Tony Vaagenes + */ +public class OneTimeRunnable { + private final Runnable runnable; + private final AtomicBoolean hasRun = new AtomicBoolean(false); + + public OneTimeRunnable(Runnable runnable) { + this.runnable = runnable; + } + + public void runIfFirstInvocation() { + boolean previous = hasRun.getAndSet(true); + if (!previous) { + runnable.run(); + } + } +} 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 index ae18c78a7d3..1bddd491496 100644 --- 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 @@ -12,11 +12,6 @@ import javax.servlet.http.HttpServletRequest; * @author bjorncs */ public class RequestUtils { - public static final String JDISC_REQUEST_X509CERT = "jdisc.request.X509Certificate"; - public static final String JDISC_REQUEST_CHAIN = "jdisc.request.chain"; - public static final String JDISC_RESPONSE_CHAIN = "jdisc.response.chain"; - public static final String SERVLET_REQUEST_X509CERT = "javax.servlet.request.X509Certificate"; - private RequestUtils() {} public static Connection getConnection(Request request) { 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 3059f972ce9..b9293226528 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 @@ -3,6 +3,7 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.ConnectorConfig; +import com.yahoo.jdisc.http.servlet.ServletRequest; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.HandlerWrapper; @@ -77,6 +78,6 @@ class TlsClientAuthenticationEnforcer extends HandlerWrapper { } private boolean isClientAuthenticated(HttpServletRequest servletRequest) { - return servletRequest.getAttribute(RequestUtils.SERVLET_REQUEST_X509CERT) != null; + return servletRequest.getAttribute(ServletRequest.SERVLET_REQUEST_X509CERT) != null; } } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/UnsupportedFilterInvoker.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/UnsupportedFilterInvoker.java new file mode 100644 index 00000000000..8d878b64e6f --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/UnsupportedFilterInvoker.java @@ -0,0 +1,32 @@ +// Copyright Yahoo. 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.handler.ResponseHandler; +import com.yahoo.jdisc.http.filter.RequestFilter; +import com.yahoo.jdisc.http.filter.ResponseFilter; + +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletRequest; +import java.net.URI; + +/** + * @author Tony Vaagenes + */ +public class UnsupportedFilterInvoker implements FilterInvoker { + @Override + public HttpServletRequest invokeRequestFilterChain(RequestFilter requestFilterChain, + URI uri, + HttpServletRequest httpRequest, + ResponseHandler responseHandler) { + throw new UnsupportedOperationException(); + } + + @Override + public void invokeResponseFilterChain( + ResponseFilter responseFilterChain, + URI uri, + HttpServletRequest request, + HttpServletResponse response) { + throw new UnsupportedOperationException(); + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/ServletModule.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/ServletModule.java new file mode 100644 index 00000000000..bb69832d767 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/ServletModule.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty.testutils; + +import com.google.inject.Binder; +import com.google.inject.Module; +import com.google.inject.Provides; +import com.yahoo.component.provider.ComponentRegistry; + +import org.eclipse.jetty.servlet.ServletHolder; + +/** + * @author Tony Vaagenes + */ +public class ServletModule implements Module { + + @SuppressWarnings("unused") + @Provides + public ComponentRegistry<ServletHolder> servletHolderComponentRegistry() { + return new ComponentRegistry<>(); + } + + @Override public void configure(Binder binder) { } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/TestDriver.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/TestDriver.java index ec0258e8763..99c49527ea5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/TestDriver.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/testutils/TestDriver.java @@ -10,6 +10,7 @@ import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.http.ServletPathsConfig; import com.yahoo.jdisc.http.server.jetty.FilterBindings; import com.yahoo.jdisc.http.server.jetty.JettyHttpServer; import com.yahoo.jdisc.http.server.jetty.VoidConnectionLog; @@ -83,6 +84,7 @@ public class TestDriver implements AutoCloseable { new AbstractModule() { @Override protected void configure() { + bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder())); bind(ServerConfig.class).toInstance(serverConfig); bind(ConnectorConfig.class).toInstance(connectorConfig); bind(FilterBindings.class).toInstance(new FilterBindings.Builder().build()); @@ -90,7 +92,8 @@ public class TestDriver implements AutoCloseable { bind(RequestLog.class).toInstance(new VoidRequestLog()); } }, - new ConnectorFactoryRegistryModule(connectorConfig)); + new ConnectorFactoryRegistryModule(connectorConfig), + new ServletModule()); } public static class Builder { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpRequest.java new file mode 100644 index 00000000000..3990c9a8910 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpRequest.java @@ -0,0 +1,40 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.servlet; + +import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.http.Cookie; +import com.yahoo.jdisc.http.HttpRequest; + +import java.net.SocketAddress; +import java.net.URI; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +/** + * Common interface for JDisc and servlet http requests. + */ +public interface ServletOrJdiscHttpRequest { + + void copyHeaders(HeaderFields target); + + Map<String, List<String>> parameters(); + + URI getUri(); + + HttpRequest.Version getVersion(); + + String getRemoteHostAddress(); + String getRemoteHostName(); + int getRemotePort(); + + void setRemoteAddress(SocketAddress remoteAddress); + + Map<String, Object> context(); + + List<Cookie> decodeCookieHeader(); + + void encodeCookieHeader(List<Cookie> cookies); + + long getConnectedAt(TimeUnit unit); +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpResponse.java new file mode 100644 index 00000000000..a40e257b67d --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletOrJdiscHttpResponse.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.servlet; + +import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.http.Cookie; + +import java.util.List; +import java.util.Map; + +/** + * Common interface for JDisc and servlet http responses. + */ +public interface ServletOrJdiscHttpResponse { + + public void copyHeaders(HeaderFields target); + + public int getStatus(); + + public Map<String, Object> context(); + + public List<Cookie> decodeSetCookieHeader(); + +} 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 new file mode 100644 index 00000000000..8f17c9dc523 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java @@ -0,0 +1,273 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.servlet; + +import com.google.common.collect.ImmutableMap; +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; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.URI; +import java.security.Principal; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnection; + +/** + * Mutable wrapper to use a {@link javax.servlet.http.HttpServletRequest} + * with JDisc security filters. + * <p> + * You might find it tempting to remove e.g. the getParameter... methods, + * but keep in mind that this IS-A servlet request and must provide the + * full api of such a request for use outside the "JDisc filter world". + */ +public class ServletRequest extends HttpServletRequestWrapper implements ServletOrJdiscHttpRequest { + + public static final String JDISC_REQUEST_PRINCIPAL = "jdisc.request.principal"; + public static final String JDISC_REQUEST_X509CERT = "jdisc.request.X509Certificate"; + public static final String JDISC_REQUEST_CHAIN = "jdisc.request.chain"; + public static final String JDISC_RESPONSE_CHAIN = "jdisc.response.chain"; + public static final String SERVLET_REQUEST_X509CERT = "javax.servlet.request.X509Certificate"; + public static final String SERVLET_REQUEST_SSL_SESSION_ID = "javax.servlet.request.ssl_session_id"; + public static final String SERVLET_REQUEST_CIPHER_SUITE = "javax.servlet.request.cipher_suite"; + + private final HttpServletRequest request; + private final HeaderFields headerFields; + private final Set<String> removedHeaders = new HashSet<>(); + private final Map<String, Object> context = new HashMap<>(); + private final Map<String, List<String>> parameters = new HashMap<>(); + private final long connectedAt; + + private URI uri; + private String remoteHostAddress; + private String remoteHostName; + private int remotePort; + + public ServletRequest(HttpServletRequest request, URI uri) { + super(request); + this.request = request; + + this.uri = uri; + + super.getParameterMap().forEach( + (key, values) -> parameters.put(key, Arrays.asList(values))); + + remoteHostAddress = request.getRemoteAddr(); + remoteHostName = request.getRemoteHost(); + remotePort = request.getRemotePort(); + connectedAt = getConnection((Request) request).getCreatedTimeStamp(); + + headerFields = new HeaderFields(); + Enumeration<String> parentHeaders = request.getHeaderNames(); + while (parentHeaders.hasMoreElements()) { + String name = parentHeaders.nextElement(); + Enumeration<String> values = request.getHeaders(name); + while (values.hasMoreElements()) { + headerFields.add(name, values.nextElement()); + } + } + } + + public HttpServletRequest getRequest() { + return request; + } + + @Override + public Map<String, List<String>> parameters() { + return parameters; + } + + /* We cannot just return the parameter map from the request, as the map + * may have been modified by the JDisc filters. */ + @Override + public Map<String, String[]> getParameterMap() { + Map<String, String[]> parameterMap = new HashMap<>(); + parameters().forEach( + (key, values) -> + parameterMap.put(key, values.toArray(new String[values.size()])) + ); + return ImmutableMap.copyOf(parameterMap); + } + + @Override + public String getParameter(String name) { + return parameters().containsKey(name) ? + parameters().get(name).get(0) : + null; + } + + @Override + public Enumeration<String> getParameterNames() { + return Collections.enumeration(parameters.keySet()); + } + + @Override + public String[] getParameterValues(String name) { + List<String> values = parameters().get(name); + return values != null ? + values.toArray(new String[values.size()]) : + null; + } + + @Override + public void copyHeaders(HeaderFields target) { + target.addAll(headerFields); + } + + @Override + public Enumeration<String> getHeaders(String name) { + if (removedHeaders.contains(name)) + return null; + + /* We don't need to merge headerFields and the servlet request's headers + * because setHeaders() replaces the old value. There is no 'addHeader(s)'. */ + List<String> headerFields = this.headerFields.get(name); + return headerFields == null || headerFields.isEmpty() ? + super.getHeaders(name) : + Collections.enumeration(headerFields); + } + + @Override + public String getHeader(String name) { + if (removedHeaders.contains(name)) + return null; + + String headerField = headerFields.getFirst(name); + return headerField != null ? + headerField : + super.getHeader(name); + } + + @Override + public Enumeration<String> getHeaderNames() { + Set<String> names = new HashSet<>(Collections.list(super.getHeaderNames())); + names.addAll(headerFields.keySet()); + names.removeAll(removedHeaders); + return Collections.enumeration(names); + } + + public void addHeader(String name, String value) { + headerFields.add(name, value); + removedHeaders.remove(name); + } + + public void setHeaders(String name, String value) { + headerFields.put(name, value); + removedHeaders.remove(name); + } + + public void setHeaders(String name, List<String> values) { + headerFields.put(name, values); + removedHeaders.remove(name); + } + + public void removeHeaders(String name) { + headerFields.remove(name); + removedHeaders.add(name); + } + + @Override + public URI getUri() { + return uri; + } + + public void setUri(URI uri) { + this.uri = uri; + } + + @Override + public HttpRequest.Version getVersion() { + String protocol = request.getProtocol(); + try { + return HttpRequest.Version.fromString(protocol); + } catch (NullPointerException | IllegalArgumentException e) { + throw new RuntimeException("Servlet request protocol '" + protocol + + "' could not be mapped to a JDisc http version.", e); + } + } + + @Override + public String getRemoteHostAddress() { + return remoteHostAddress; + } + + @Override + public String getRemoteHostName() { + return remoteHostName; + } + + @Override + public int getRemotePort() { + return remotePort; + } + + @Override + public void setRemoteAddress(SocketAddress remoteAddress) { + if (remoteAddress instanceof InetSocketAddress) { + remoteHostAddress = ((InetSocketAddress) remoteAddress).getAddress().getHostAddress(); + remoteHostName = ((InetSocketAddress) remoteAddress).getAddress().getHostName(); + remotePort = ((InetSocketAddress) remoteAddress).getPort(); + } else + throw new RuntimeException("Unknown SocketAddress class: " + remoteHostAddress.getClass().getName()); + + } + + @Override + public Map<String, Object> context() { + return context; + } + + @Override + public javax.servlet.http.Cookie[] getCookies() { + return decodeCookieHeader().stream(). + map(jdiscCookie -> new javax.servlet.http.Cookie(jdiscCookie.getName(), jdiscCookie.getValue())). + toArray(javax.servlet.http.Cookie[]::new); + } + + @Override + public List<Cookie> decodeCookieHeader() { + Enumeration<String> cookies = getHeaders(HttpHeaders.Names.COOKIE); + if (cookies == null) + return Collections.emptyList(); + + List<Cookie> ret = new LinkedList<>(); + while(cookies.hasMoreElements()) + ret.addAll(Cookie.fromCookieHeader(cookies.nextElement())); + + return ret; + } + + @Override + public void encodeCookieHeader(List<Cookie> cookies) { + setHeaders(HttpHeaders.Names.COOKIE, Cookie.toCookieHeader(cookies)); + } + + @Override + public long getConnectedAt(TimeUnit unit) { + return unit.convert(connectedAt, TimeUnit.MILLISECONDS); + } + + @Override + public Principal getUserPrincipal() { + // NOTE: The principal from the underlying servlet request is ignored. JDisc filters are the source-of-truth. + return (Principal) request.getAttribute(JDISC_REQUEST_PRINCIPAL); + } + + public void setUserPrincipal(Principal principal) { + request.setAttribute(JDISC_REQUEST_PRINCIPAL, principal); + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java new file mode 100644 index 00000000000..44a23c22b4a --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java @@ -0,0 +1,66 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.servlet; + +import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.http.Cookie; +import com.yahoo.jdisc.http.HttpHeaders; + +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +/** + * JDisc wrapper to use a {@link javax.servlet.http.HttpServletResponse} + * with JDisc security filters. + */ +public class ServletResponse extends HttpServletResponseWrapper implements ServletOrJdiscHttpResponse { + + private final HttpServletResponse response; + private final Map<String, Object> context = new HashMap<>(); + + public ServletResponse(HttpServletResponse response) { + super(response); + this.response = response; + } + + public HttpServletResponse getResponse() { + return response; + } + + @Override + public int getStatus() { + return response.getStatus(); + } + + @Override + public Map<String, Object> context() { + return context; + } + + @Override + public void copyHeaders(HeaderFields target) { + response.getHeaderNames().forEach( header -> + target.add(header, new ArrayList<>(response.getHeaders(header))) + ); + } + + @Override + public List<Cookie> decodeSetCookieHeader() { + Collection<String> cookies = getHeaders(HttpHeaders.Names.SET_COOKIE); + if (cookies == null) { + return Collections.emptyList(); + } + List<Cookie> ret = new LinkedList<>(); + for (String cookie : cookies) { + ret.add(Cookie.fromSetCookieHeader(cookie)); + } + return ret; + } + +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/servlet/package-info.java b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/package-info.java new file mode 100644 index 00000000000..098506aa86f --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/servlet/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.jdisc.http.servlet; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/container-core/src/main/resources/configdefinitions/container.servlet.servlet-config.def b/container-core/src/main/resources/configdefinitions/container.servlet.servlet-config.def index 3cc65475913..d169ceb27d7 100644 --- a/container-core/src/main/resources/configdefinitions/container.servlet.servlet-config.def +++ b/container-core/src/main/resources/configdefinitions/container.servlet.servlet-config.def @@ -1,5 +1,4 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# TODO Vespa 8 Remove config definition namespace=container.servlet map{} string diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.servlet-paths.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.servlet-paths.def index af788764364..db00df042bf 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.servlet-paths.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.servlet-paths.def @@ -1,5 +1,4 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# TODO Vespa 8 Remove config definition namespace=jdisc.http # path by servlet componentId 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 new file mode 100644 index 00000000000..dfd240d3723 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java @@ -0,0 +1,173 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +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.junit.Before; +import org.junit.Test; + +import java.net.URI; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static com.yahoo.jdisc.http.HttpRequest.Version; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Test the parts of the DiscFilterRequest API that are implemented + * by ServletFilterRequest, both directly and indirectly via + * {@link com.yahoo.jdisc.http.servlet.ServletRequest}. + * + * @author gjoranv + */ +public class ServletFilterRequestTest { + + private final String host = "host1"; + private final int port = 8080; + private final String path = "/path1"; + private final String paramName = "param1"; + private final String paramValue = "p1"; + private final String listParamName = "listParam"; + private final String[] listParamValue = new String[]{"1", "2"}; + private final String headerName = "header1"; + private final String headerValue = "h1"; + private final String attributeName = "attribute1"; + private final String attributeValue = "a1"; + + private URI uri; + private DiscFilterRequest filterRequest; + private ServletRequest parentRequest; + + @Before + public void init() throws Exception { + uri = new URI("http", null, host, port, path, paramName + "=" + paramValue, null); + + filterRequest = new ServletFilterRequest(newServletRequest()); + 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(); + return new ServletRequest(parent, uri); + } + + @Test + public void parent_properties_are_propagated_to_disc_filter_request() throws Exception { + assertEquals(filterRequest.getVersion(), Version.HTTP_1_1); + assertEquals(filterRequest.getMethod(), "GET"); + assertEquals(filterRequest.getUri(), uri); + assertEquals(filterRequest.getRemoteHost(), host); + assertEquals(filterRequest.getRemotePort(), port); + assertEquals(filterRequest.getRequestURI(), path); // getRequestUri return only the path by design + + assertEquals(filterRequest.getParameter(paramName), paramValue); + assertEquals(filterRequest.getParameterMap().get(paramName), + Collections.singletonList(paramValue)); + assertEquals(filterRequest.getParameterValuesAsList(listParamName), Arrays.asList(listParamValue)); + + assertEquals(filterRequest.getHeader(headerName), headerValue); + assertEquals(filterRequest.getAttribute(attributeName), attributeValue); + } + + @Test + public void untreatedHeaders_is_populated_from_the_parent_request() { + assertEquals(filterRequest.getUntreatedHeaders().getFirst(headerName), headerValue); + } + + @Test + @SuppressWarnings("deprecation") + public void uri_can_be_set() throws Exception { + URI newUri = new URI("http", null, host, port + 1, path, paramName + "=" + paramValue, null); + filterRequest.setUri(newUri); + + assertEquals(filterRequest.getUri(), newUri); + assertEquals(parentRequest.getUri(), newUri); + } + + @Test + public void attributes_can_be_set() throws Exception { + String name = "newAttribute"; + String value = name + "Value"; + filterRequest.setAttribute(name, value); + + assertEquals(filterRequest.getAttribute(name), value); + assertEquals(parentRequest.getAttribute(name), value); + } + + @Test + public void attributes_can_be_removed() { + filterRequest.removeAttribute(attributeName); + + assertEquals(filterRequest.getAttribute(attributeName), null); + assertEquals(parentRequest.getAttribute(attributeName), null); + } + + @Test + public void headers_can_be_set() throws Exception { + String name = "myHeader"; + String value = name + "Value"; + filterRequest.setHeaders(name, value); + + assertEquals(filterRequest.getHeader(name), value); + assertEquals(parentRequest.getHeader(name), value); + } + + @Test + public void headers_can_be_removed() throws Exception { + filterRequest.removeHeaders(headerName); + + assertEquals(filterRequest.getHeader(headerName), null); + assertEquals(parentRequest.getHeader(headerName), null); + } + + @Test + public void headers_can_be_added() { + String value = "h2"; + filterRequest.addHeader(headerName, value); + + List<String> expected = Arrays.asList(headerValue, value); + assertEquals(filterRequest.getHeadersAsList(headerName), expected); + assertEquals(Collections.list(parentRequest.getHeaders(headerName)), expected); + } + + @Test + public void cookies_can_be_added_and_removed() { + Cookie cookie = new Cookie("name", "value"); + filterRequest.addCookie(JDiscCookieWrapper.wrap(cookie)); + + assertEquals(filterRequest.getCookies(), Collections.singletonList(cookie)); + assertEquals(parentRequest.getCookies().length, 1); + + javax.servlet.http.Cookie servletCookie = parentRequest.getCookies()[0]; + assertEquals(servletCookie.getName(), cookie.getName()); + assertEquals(servletCookie.getValue(), cookie.getValue()); + + filterRequest.clearCookies(); + assertTrue(filterRequest.getCookies().isEmpty()); + assertEquals(parentRequest.getCookies().length, 0); + } + + @Test + public void character_encoding_can_be_set() throws Exception { + // ContentType must be non-null before setting character encoding + filterRequest.setHeaders(HttpHeaders.Names.CONTENT_TYPE, ""); + + String encoding = "myEncoding"; + filterRequest.setCharacterEncoding(encoding); + + assertTrue(filterRequest.getCharacterEncoding().contains(encoding)); + assertTrue(parentRequest.getCharacterEncoding().contains(encoding)); + } + +} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterResponseTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterResponseTest.java new file mode 100644 index 00000000000..1b49ad7ddd1 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterResponseTest.java @@ -0,0 +1,87 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.filter; + +import com.yahoo.jdisc.http.Cookie; +import com.yahoo.jdisc.http.HttpHeaders; +import com.yahoo.jdisc.http.servlet.ServletResponse; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; + +import static org.junit.Assert.assertEquals; + +/** + * @author gjoranv + * @since 5.27 + */ +public class ServletFilterResponseTest { + + private final String headerName = "header1"; + private final String headerValue = "h1"; + + private DiscFilterResponse filterResponse; + private ServletResponse parentResponse; + + @Before + public void init() throws Exception { + filterResponse = new ServletFilterResponse(newServletResponse()); + parentResponse = ((ServletFilterResponse)filterResponse).getServletResponse(); + + } + + private ServletResponse newServletResponse() throws Exception { + MockServletResponse parent = new MockServletResponse(); + parent.addHeader(headerName, headerValue); + return new ServletResponse(parent); + } + + + @Test + public void headers_can_be_set() throws Exception { + String name = "myHeader"; + String value = name + "Value"; + filterResponse.setHeaders(name, value); + + assertEquals(filterResponse.getHeader(name), value); + assertEquals(parentResponse.getHeader(name), value); + } + + @Test + public void headers_can_be_added() throws Exception { + String newValue = "h2"; + filterResponse.addHeader(headerName, newValue); + + // The DiscFilterResponse has no getHeaders() + assertEquals(filterResponse.getHeader(headerName), newValue); + + assertEquals(parentResponse.getHeaders(headerName), Arrays.asList(headerValue, newValue)); + } + + @Test + public void headers_can_be_removed() throws Exception { + filterResponse.removeHeaders(headerName); + + assertEquals(filterResponse.getHeader(headerName), null); + assertEquals(parentResponse.getHeader(headerName), null); + } + + @Test + public void set_cookie_overwrites_old_values() { + Cookie to_be_removed = new Cookie("to-be-removed", ""); + Cookie to_keep = new Cookie("to-keep", ""); + filterResponse.setCookie(to_be_removed.getName(), to_be_removed.getValue()); + filterResponse.setCookie(to_keep.getName(), to_keep.getValue()); + + assertEquals(filterResponse.getCookies(), Arrays.asList(to_keep)); + assertEquals(parentResponse.getHeaders(HttpHeaders.Names.SET_COOKIE), Arrays.asList(to_keep.toString())); + } + + + private static class MockServletResponse extends org.eclipse.jetty.server.Response { + private MockServletResponse() { + super(null, null); + } + } + +} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java new file mode 100644 index 00000000000..c6d416b2b99 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/JDiscFilterForServletTest.java @@ -0,0 +1,165 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty.servlet; + +import com.google.inject.AbstractModule; +import com.google.inject.Module; +import com.google.inject.util.Modules; +import com.yahoo.jdisc.AbstractResource; +import com.yahoo.jdisc.Request; +import com.yahoo.jdisc.Response; +import com.yahoo.jdisc.handler.ContentChannel; +import com.yahoo.jdisc.handler.ResponseHandler; +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.server.jetty.FilterBindings; +import com.yahoo.jdisc.http.server.jetty.FilterInvoker; +import com.yahoo.jdisc.http.server.jetty.SimpleHttpClient.ResponseValidator; +import com.yahoo.jdisc.http.server.jetty.JettyTestDriver; +import org.junit.Test; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.net.URI; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; + +/** + * @author Tony Vaagenes + * @author bjorncs + */ +public class JDiscFilterForServletTest extends ServletTestBase { + @Test + public void request_filter_can_return_response() throws IOException, InterruptedException { + JettyTestDriver testDriver = requestFilterTestDriver(); + ResponseValidator response = httpGet(testDriver, TestServlet.PATH).execute(); + + response.expectContent(containsString(TestRequestFilter.responseContent)); + } + + @Test + public void request_can_be_forwarded_through_request_filter_to_servlet() throws IOException { + JettyTestDriver testDriver = requestFilterTestDriver(); + ResponseValidator response = httpGet(testDriver, TestServlet.PATH). + addHeader(TestRequestFilter.BYPASS_FILTER_HEADER, Boolean.TRUE.toString()). + execute(); + + response.expectContent(containsString(TestServlet.RESPONSE_CONTENT)); + } + + @Test + public void response_filter_can_modify_response() throws IOException { + JettyTestDriver testDriver = responseFilterTestDriver(); + ResponseValidator response = httpGet(testDriver, TestServlet.PATH).execute(); + + response.expectHeader(TestResponseFilter.INVOKED_HEADER, is(Boolean.TRUE.toString())); + } + + @Test + public void response_filter_is_run_on_empty_sync_response() throws IOException { + JettyTestDriver testDriver = responseFilterTestDriver(); + ResponseValidator response = httpGet(testDriver, NoContentTestServlet.PATH).execute(); + + response.expectHeader(TestResponseFilter.INVOKED_HEADER, is(Boolean.TRUE.toString())); + } + + @Test + public void response_filter_is_run_on_empty_async_response() throws IOException { + JettyTestDriver testDriver = responseFilterTestDriver(); + ResponseValidator response = httpGet(testDriver, NoContentTestServlet.PATH). + addHeader(NoContentTestServlet.HEADER_ASYNC, Boolean.TRUE.toString()). + execute(); + + response.expectHeader(TestResponseFilter.INVOKED_HEADER, is(Boolean.TRUE.toString())); + } + + private JettyTestDriver requestFilterTestDriver() throws IOException { + FilterBindings filterBindings = new FilterBindings.Builder() + .addRequestFilter("my-request-filter", new TestRequestFilter()) + .addRequestFilterBinding("my-request-filter", "http://*/*") + .build(); + return JettyTestDriver.newInstance(dummyRequestHandler, bindings(filterBindings)); + } + + private JettyTestDriver responseFilterTestDriver() throws IOException { + FilterBindings filterBindings = new FilterBindings.Builder() + .addResponseFilter("my-response-filter", new TestResponseFilter()) + .addResponseFilterBinding("my-response-filter", "http://*/*") + .build(); + return JettyTestDriver.newInstance(dummyRequestHandler, bindings(filterBindings)); + } + + + + private Module bindings(FilterBindings filterBindings) { + return Modules.combine( + new AbstractModule() { + @Override + protected void configure() { + bind(FilterBindings.class).toInstance(filterBindings); + bind(FilterInvoker.class).toInstance(new FilterInvoker() { + @Override + public HttpServletRequest invokeRequestFilterChain( + RequestFilter requestFilter, + URI uri, + HttpServletRequest httpRequest, + ResponseHandler responseHandler) { + TestRequestFilter filter = (TestRequestFilter) requestFilter; + filter.runAsSecurityFilter(httpRequest, responseHandler); + return httpRequest; + } + + @Override + public void invokeResponseFilterChain( + ResponseFilter responseFilter, + URI uri, + HttpServletRequest request, + HttpServletResponse response) { + + TestResponseFilter filter = (TestResponseFilter) responseFilter; + filter.runAsSecurityFilter(request, response); + } + }); + } + }, + guiceModule()); + } + + static class TestRequestFilter extends AbstractResource implements RequestFilter { + static final String simpleName = TestRequestFilter.class.getSimpleName(); + static final String responseContent = "Rejected by " + simpleName; + static final String BYPASS_FILTER_HEADER = "BYPASS_HEADER" + simpleName; + + @Override + public void filter(HttpRequest request, ResponseHandler handler) { + throw new UnsupportedOperationException(); + } + + public void runAsSecurityFilter(HttpServletRequest request, ResponseHandler responseHandler) { + if (Boolean.parseBoolean(request.getHeader(BYPASS_FILTER_HEADER))) + return; + + ContentChannel contentChannel = responseHandler.handleResponse(new Response(500)); + contentChannel.write(ByteBuffer.wrap(responseContent.getBytes(StandardCharsets.UTF_8)), null); + contentChannel.close(null); + } + } + + + static class TestResponseFilter extends AbstractResource implements ResponseFilter { + static final String INVOKED_HEADER = TestResponseFilter.class.getSimpleName() + "_INVOKED_HEADER"; + + @Override + public void filter(Response response, Request request) { + throw new UnsupportedClassVersionError(); + } + + public void runAsSecurityFilter(HttpServletRequest request, HttpServletResponse response) { + response.addHeader(INVOKED_HEADER, Boolean.TRUE.toString()); + } + } +} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java new file mode 100644 index 00000000000..17802b7f466 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletAccessLoggingTest.java @@ -0,0 +1,63 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty.servlet; + +import com.google.inject.AbstractModule; +import com.google.inject.Module; +import com.google.inject.util.Modules; +import com.yahoo.container.logging.AccessLog; +import com.yahoo.container.logging.RequestLog; +import com.yahoo.container.logging.RequestLogEntry; +import com.yahoo.jdisc.http.server.jetty.JettyTestDriver; +import org.junit.Test; +import org.mockito.verification.VerificationMode; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; + +/** + * @author bakksjo + * @author bjorncs + */ +public class ServletAccessLoggingTest extends ServletTestBase { + private static final long MAX_LOG_WAIT_TIME_MILLIS = TimeUnit.SECONDS.toMillis(60); + + @Test + public void accessLogIsInvokedForNonJDiscServlet() throws Exception { + final AccessLog accessLog = mock(AccessLog.class); + final JettyTestDriver testDriver = newTestDriver(accessLog); + httpGet(testDriver, TestServlet.PATH).execute(); + verifyCallsLog(accessLog, timeout(MAX_LOG_WAIT_TIME_MILLIS).times(1)); + } + + @Test + public void accessLogIsInvokedForJDiscServlet() throws Exception { + final AccessLog accessLog = mock(AccessLog.class); + final JettyTestDriver testDriver = newTestDriver(accessLog); + testDriver.client().newGet("/status.html").execute(); + verifyCallsLog(accessLog, timeout(MAX_LOG_WAIT_TIME_MILLIS).times(1)); + } + + private void verifyCallsLog(RequestLog requestLog, final VerificationMode verificationMode) { + verify(requestLog, verificationMode).log(any(RequestLogEntry.class)); + } + + private JettyTestDriver newTestDriver(RequestLog requestLog) throws IOException { + return JettyTestDriver.newInstance(dummyRequestHandler, bindings(requestLog)); + } + + private Module bindings(RequestLog requestLog) { + return Modules.combine( + new AbstractModule() { + @Override + protected void configure() { + bind(RequestLog.class).toInstance(requestLog); + } + }, + guiceModule()); + } +} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletTestBase.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletTestBase.java new file mode 100644 index 00000000000..f13769dec38 --- /dev/null +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/servlet/ServletTestBase.java @@ -0,0 +1,132 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.http.server.jetty.servlet; + +import com.google.inject.AbstractModule; +import com.google.inject.Module; +import com.google.inject.TypeLiteral; +import com.yahoo.component.ComponentId; +import com.yahoo.component.provider.ComponentRegistry; +import com.yahoo.jdisc.Request; +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 com.yahoo.jdisc.http.ServletPathsConfig; +import com.yahoo.jdisc.http.ServletPathsConfig.Servlets.Builder; +import com.yahoo.jdisc.http.server.jetty.SimpleHttpClient.RequestExecutor; +import com.yahoo.jdisc.http.server.jetty.JettyTestDriver; +import org.eclipse.jetty.servlet.ServletHolder; + +import javax.servlet.ServletException; +import javax.servlet.annotation.WebServlet; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.List; + +/** + * @author Tony Vaagenes + * @author bakksjo + */ +public class ServletTestBase { + + private static class ServletInstance { + final ComponentId componentId; final String path; final HttpServlet instance; + + ServletInstance(ComponentId componentId, String path, HttpServlet instance) { + this.componentId = componentId; + this.path = path; + this.instance = instance; + } + } + + private final List<ServletInstance> servlets = List.of( + new ServletInstance(TestServlet.ID, TestServlet.PATH, new TestServlet()), + new ServletInstance(NoContentTestServlet.ID, NoContentTestServlet.PATH, new NoContentTestServlet())); + + protected RequestExecutor httpGet(JettyTestDriver testDriver, String path) { + return testDriver.client().newGet("/" + path); + } + + protected ServletPathsConfig createServletPathConfig() { + ServletPathsConfig.Builder configBuilder = new ServletPathsConfig.Builder(); + + servlets.forEach(servlet -> + configBuilder.servlets( + servlet.componentId.stringValue(), + new Builder().path(servlet.path))); + + return new ServletPathsConfig(configBuilder); + } + + protected ComponentRegistry<ServletHolder> servlets() { + ComponentRegistry<ServletHolder> result = new ComponentRegistry<>(); + + servlets.forEach(servlet -> + result.register(servlet.componentId, new ServletHolder(servlet.instance))); + + result.freeze(); + return result; + } + + protected Module guiceModule() { + return new AbstractModule() { + @Override + protected void configure() { + bind(new TypeLiteral<ComponentRegistry<ServletHolder>>(){}).toInstance(servlets()); + bind(ServletPathsConfig.class).toInstance(createServletPathConfig()); + } + }; + } + + protected static class TestServlet extends HttpServlet { + static final String PATH = "servlet/test-servlet"; + static final ComponentId ID = ComponentId.fromString("test-servlet"); + static final String RESPONSE_CONTENT = "Response from " + TestServlet.class.getSimpleName(); + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + response.setContentType("text/plain"); + PrintWriter writer = response.getWriter(); + writer.write(RESPONSE_CONTENT); + writer.close(); + } + } + + @WebServlet(asyncSupported = true) + protected static class NoContentTestServlet extends HttpServlet { + static final String HEADER_ASYNC = "HEADER_ASYNC"; + + static final String PATH = "servlet/no-content-test-servlet"; + static final ComponentId ID = ComponentId.fromString("no-content-test-servlet"); + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if (request.getHeader(HEADER_ASYNC) != null) { + asyncGet(request); + } + } + + private void asyncGet(HttpServletRequest request) { + request.startAsync().start(() -> { + try { + Thread.sleep(100); + } catch (InterruptedException e) { + log("Interrupted", e); + } finally { + request.getAsyncContext().complete(); + } + }); + } + } + + + protected static final RequestHandler dummyRequestHandler = new AbstractRequestHandler() { + @Override + public ContentChannel handleRequest(Request request, ResponseHandler handler) { + throw new UnsupportedOperationException(); + } + }; +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java index 5f580b6f6b3..b25cb913c83 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java @@ -2,9 +2,13 @@ package com.yahoo.vespa.hosted.controller.restapi; import com.yahoo.application.container.handler.Request; +import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.filter.DiscFilterRequest; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest; +import java.net.SocketAddress; import java.net.URI; import java.security.Principal; import java.security.cert.X509Certificate; @@ -12,11 +16,7 @@ import java.util.Collections; import java.util.Enumeration; import java.util.List; import java.util.Map; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; +import java.util.concurrent.TimeUnit; /** * Wraps an {@link Request} into a {@link DiscFilterRequest}. Only a few methods are supported. @@ -35,20 +35,72 @@ public class ApplicationRequestToDiscFilterRequestWrapper extends DiscFilterRequ } public ApplicationRequestToDiscFilterRequestWrapper(Request request, List<X509Certificate> clientCertificateChain) { - super(createDummyHttpRequest(request)); + super(new ServletOrJdiscHttpRequest() { + @Override + public void copyHeaders(HeaderFields target) { + request.getHeaders().forEach(target::add); + } + + @Override + public Map<String, List<String>> parameters() { + return Collections.emptyMap(); + } + + @Override + public URI getUri() { + return URI.create(request.getUri()).normalize(); // Consistent with what JDisc does. + } + + @Override + public HttpRequest.Version getVersion() { + throw new UnsupportedOperationException(); + } + + @Override + public String getRemoteHostAddress() { + throw new UnsupportedOperationException(); + } + + @Override + public String getRemoteHostName() { + throw new UnsupportedOperationException(); + } + + @Override + public int getRemotePort() { + throw new UnsupportedOperationException(); + } + + @Override + public void setRemoteAddress(SocketAddress remoteAddress) { + throw new UnsupportedOperationException(); + } + + @Override + public Map<String, Object> context() { + throw new UnsupportedOperationException(); + } + + @Override + public List<Cookie> decodeCookieHeader() { + throw new UnsupportedOperationException(); + } + + @Override + public void encodeCookieHeader(List<Cookie> cookies) { + throw new UnsupportedOperationException(); + } + + @Override + public long getConnectedAt(TimeUnit unit) { + throw new UnsupportedOperationException(); + } + }); this.request = request; this.userPrincipal = request.getUserPrincipal().orElse(null); this.clientCertificateChain = clientCertificateChain; } - private static HttpRequest createDummyHttpRequest(Request req) { - HttpRequest dummy = mock(HttpRequest.class, invocation -> { throw new UnsupportedOperationException(); }); - doReturn(URI.create(req.getUri()).normalize()).when(dummy).getUri(); - doNothing().when(dummy).copyHeaders(any()); - doReturn(Map.of()).when(dummy).parameters(); - return dummy; - } - public Request getUpdatedRequest() { Request updatedRequest = new Request(this.request.getUri(), this.request.getBody(), this.request.getMethod(), this.userPrincipal); this.request.getHeaders().forEach(updatedRequest.getHeaders()::put); diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsResponseFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsResponseFilterTest.java index 9803418e9ae..ecadd0b1b87 100644 --- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsResponseFilterTest.java +++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsResponseFilterTest.java @@ -2,11 +2,11 @@ package com.yahoo.jdisc.http.filter.security.cors; import com.yahoo.jdisc.http.Cookie; -import com.yahoo.jdisc.http.HttpResponse; import com.yahoo.jdisc.http.filter.DiscFilterResponse; import com.yahoo.jdisc.http.filter.RequestView; import com.yahoo.jdisc.http.filter.SecurityResponseFilter; import com.yahoo.jdisc.http.filter.security.cors.CorsFilterConfig.Builder; +import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpResponse; import org.junit.Test; import java.util.Arrays; @@ -78,7 +78,7 @@ public class CorsResponseFilterTest { Map<String, String> headers = new HashMap<>(); TestResponse() { - super(mock(HttpResponse.class)); + super(mock(ServletOrJdiscHttpResponse.class)); } @Override |