diff options
author | jonmv <venstad@gmail.com> | 2023-10-05 16:56:47 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-10-05 16:56:47 +0200 |
commit | 1031323b5f08f02e25c79a8653987ed7d70e12a6 (patch) | |
tree | 3db700aa4a8be893136dd8f87ed6f0d794561fdf | |
parent | 4442e0ebf35867a85128088471be38afc61e0ef0 (diff) |
Isolate inner jetty server component from wrapper with filters
18 files changed, 176 insertions, 105 deletions
diff --git a/application/src/test/java/com/yahoo/application/ApplicationTest.java b/application/src/test/java/com/yahoo/application/ApplicationTest.java index 6b394cdebd9..e22083505af 100644 --- a/application/src/test/java/com/yahoo/application/ApplicationTest.java +++ b/application/src/test/java/com/yahoo/application/ApplicationTest.java @@ -24,6 +24,7 @@ import com.yahoo.search.handler.SearchHandler; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.DefaultHttpClient; import org.junit.jupiter.api.Test; import java.io.BufferedReader; @@ -50,9 +51,7 @@ public class ApplicationTest { @Test void minimal_application_can_be_constructed() { - try (Application application = Application.fromServicesXml("<container version=\"1.0\"/>", Networking.disable)) { - Application unused = application; - } + try (Application application = Application.fromServicesXml("<container version=\"1.0\"/>", Networking.disable)) { } } /** Tests that an application with search chains referencing a content cluster can be constructed. */ @@ -80,10 +79,6 @@ public class ApplicationTest { assertEquals("select * from sources * where weakAnd(substring contains \"foobar\") limit 2 timeout 20000000", result.getQuery().yqlRepresentation(true)); } } - private void printTrace(Result result) { - for (String message : result.getQuery().getContext(true).getTrace().traceNode().descendants(String.class)) - System.out.println(message); - } @Test void empty_container() throws Exception { @@ -348,17 +343,17 @@ public class ApplicationTest { void http_interface_is_on_when_networking_is_enabled() throws Exception { int httpPort = getFreePort(); try (Application application = Application.fromServicesXml(servicesXmlWithServer(httpPort), Networking.enable)) { - HttpClient client = new org.apache.http.impl.client.DefaultHttpClient(); - HttpResponse response = client.execute(new HttpGet("http://localhost:" + httpPort)); - assertEquals(200, response.getStatusLine().getStatusCode()); - BufferedReader r = new BufferedReader(new InputStreamReader(response.getEntity().getContent())); - String line; - StringBuilder sb = new StringBuilder(); - while ((line = r.readLine()) != null) { - sb.append(line).append("\n"); + try (DefaultHttpClient client = new org.apache.http.impl.client.DefaultHttpClient()) { + HttpResponse response = client.execute(new HttpGet("http://localhost:" + httpPort)); + assertEquals(200, response.getStatusLine().getStatusCode()); + BufferedReader r = new BufferedReader(new InputStreamReader(response.getEntity().getContent())); + String line; + StringBuilder sb = new StringBuilder(); + while ((line = r.readLine()) != null) { + sb.append(line).append("\n"); + } + assertTrue(sb.toString().contains("Handler")); } - assertTrue(sb.toString().contains("Handler")); - Application unused = application; } } @@ -366,7 +361,6 @@ public class ApplicationTest { void athenz_in_deployment_xml() { try (Application application = Application.fromApplicationPackage(new File("src/test/app-packages/athenz-in-deployment-xml/"), Networking.disable)) { // Deployment succeeded - Application unused = application; } } @@ -386,9 +380,7 @@ public class ApplicationTest { @Test void application_with_access_control_can_be_constructed() { - try (Application application = Application.fromServicesXml(servicesXmlWithAccessControl(), Networking.disable)) { - Application unused = application; - } + try (Application application = Application.fromServicesXml(servicesXmlWithAccessControl(), Networking.disable)) { } } private static String servicesXmlWithAccessControl() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java index 0388230fa6a..1ab01839ef1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java @@ -32,9 +32,11 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro super(new ComponentModel(componentId, com.yahoo.jdisc.http.server.jetty.JettyHttpServer.class.getName(), null)); this.isHostedVespa = deployState.isHosted(); this.cluster = cluster; - final FilterBindingsProviderComponent filterBindingsProviderComponent = new FilterBindingsProviderComponent(componentId); + FilterBindingsProviderComponent filterBindingsProviderComponent = new FilterBindingsProviderComponent(componentId); addChild(filterBindingsProviderComponent); - inject(filterBindingsProviderComponent); + addChild(new SimpleComponent(childComponentModel(componentId, com.yahoo.jdisc.http.server.jetty.JettyHttpServerContext.class.getName())) { + { inject(filterBindingsProviderComponent); } + }); for (String agent : deployState.featureFlags().ignoredHttpUserAgents()) { addIgnoredUserAgent(agent); } @@ -79,7 +81,7 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro } } - static ComponentModel providerComponentModel(String parentId, String className) { + static ComponentModel childComponentModel(String parentId, String className) { final ComponentSpecification classNameSpec = new ComponentSpecification( className); return new ComponentModel(new BundleInstantiationSpecification( @@ -90,9 +92,8 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro public static final class FilterBindingsProviderComponent extends SimpleComponent { public FilterBindingsProviderComponent(String parentId) { - super(providerComponentModel(parentId, "com.yahoo.container.jdisc.FilterBindingsProvider")); + super(childComponentModel(parentId, "com.yahoo.container.jdisc.FilterBindingsProvider")); } - } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java index 89cce7feacb..5da5930515c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java @@ -77,23 +77,27 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas } @Test - void verifyThatJettyHttpServerHasFilterBindingsProvider() { + void verifyThatJettyHttpServerContextHasFilterBindingsProvider() { final Element clusterElem = DomBuilderTest.parse( "<container id='default' version='1.0'>", nodesXml, "</container>"); createModel(root, clusterElem); - final ComponentsConfig.Components jettyHttpServerComponent = extractComponentByClassName( + ComponentsConfig.Components jettyHttpServerComponent = extractComponentByClassName( containerComponentsConfig(), com.yahoo.jdisc.http.server.jetty.JettyHttpServer.class.getName()); assertNotNull(jettyHttpServerComponent); - final ComponentsConfig.Components filterBindingsProviderComponent = extractComponentByClassName( + ComponentsConfig.Components jettyHttpServerContextComponent = extractComponentByClassName( + containerComponentsConfig(), com.yahoo.jdisc.http.server.jetty.JettyHttpServerContext.class.getName()); + assertNotNull(jettyHttpServerContextComponent); + + ComponentsConfig.Components filterBindingsProviderComponent = extractComponentByClassName( containerComponentsConfig(), FilterBindingsProvider.class.getName()); assertNotNull(filterBindingsProviderComponent); - final ComponentsConfig.Components.Inject filterBindingsProviderInjection = extractInjectionById( - jettyHttpServerComponent, filterBindingsProviderComponent.id()); + ComponentsConfig.Components.Inject filterBindingsProviderInjection = extractInjectionById( + jettyHttpServerContextComponent, filterBindingsProviderComponent.id()); assertNotNull(filterBindingsProviderInjection); } @@ -112,12 +116,16 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas clusterComponentsConfig(), com.yahoo.jdisc.http.server.jetty.JettyHttpServer.class.getName()); assertNotNull(jettyHttpServerComponent); + final ComponentsConfig.Components jettyHttpServerContextComponent = extractComponentByClassName( + clusterComponentsConfig(), com.yahoo.jdisc.http.server.jetty.JettyHttpServerContext.class.getName()); + assertNotNull(jettyHttpServerContextComponent); + final ComponentsConfig.Components filterBindingsProviderComponent = extractComponentByClassName( clusterComponentsConfig(), FilterBindingsProvider.class.getName()); assertNotNull(filterBindingsProviderComponent); final ComponentsConfig.Components.Inject filterBindingsProviderInjection = extractInjectionById( - jettyHttpServerComponent, filterBindingsProviderComponent.id()); + jettyHttpServerContextComponent, filterBindingsProviderComponent.id()); assertNotNull(filterBindingsProviderInjection); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ExceptionWrapper.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ExceptionWrapper.java index 3ba159e5ef6..5205f8f1253 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ExceptionWrapper.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ExceptionWrapper.java @@ -7,7 +7,7 @@ package com.yahoo.jdisc.http.server.jetty; * ensures some extra information is automatically added to the contents of * getMessage(). * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class ExceptionWrapper extends RuntimeException { private final String message; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterBindings.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterBindings.java index e4e8188dc41..b39041b63e5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterBindings.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterBindings.java @@ -27,20 +27,22 @@ public class FilterBindings { private final Map<Integer, String> defaultResponseFilters; private final BindingSet<String> requestFilterBindings; private final BindingSet<String> responseFilterBindings; - + private final boolean strictFiltering; private FilterBindings( Map<String, RequestFilter> requestFilters, Map<String, ResponseFilter> responseFilters, Map<Integer, String> defaultRequestFilters, Map<Integer, String> defaultResponseFilters, BindingSet<String> requestFilterBindings, - BindingSet<String> responseFilterBindings) { + BindingSet<String> responseFilterBindings, + boolean strictFiltering) { this.requestFilters = requestFilters; this.responseFilters = responseFilters; this.defaultRequestFilters = defaultRequestFilters; this.defaultResponseFilters = defaultResponseFilters; this.requestFilterBindings = requestFilterBindings; this.responseFilterBindings = responseFilterBindings; + this.strictFiltering = strictFiltering; } public Optional<String> resolveRequestFilter(URI uri, int localPort) { @@ -67,6 +69,8 @@ public class FilterBindings { public Collection<ResponseFilter> responseFilters() { return responseFilters.values(); } + public boolean strictFiltering() { return strictFiltering; } + public static class Builder { private final Map<String, RequestFilter> requestFilters = new TreeMap<>(); private final Map<String, ResponseFilter> responseFilters = new TreeMap<>(); @@ -74,6 +78,7 @@ public class FilterBindings { private final Map<Integer, String> defaultResponseFilters = new TreeMap<>(); private final BindingRepository<String> requestFilterBindings = new BindingRepository<>(); private final BindingRepository<String> responseFilterBindings = new BindingRepository<>(); + private boolean strictFiltering = false; public Builder() {} @@ -89,6 +94,8 @@ public class FilterBindings { public Builder setResponseFilterDefaultForPort(String id, int port) { defaultResponseFilters.put(port, id); return this; } + public Builder setStrictFiltering(boolean strictFiltering) { this.strictFiltering = strictFiltering; return this; } + public FilterBindings build() { return new FilterBindings( Collections.unmodifiableMap(requestFilters), @@ -96,7 +103,8 @@ public class FilterBindings { Collections.unmodifiableMap(defaultRequestFilters), Collections.unmodifiableMap(defaultResponseFilters), requestFilterBindings.activate(), - responseFilterBindings.activate()); + responseFilterBindings.activate(), + strictFiltering); } } } 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..83eb63fff8d 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 @@ -27,12 +27,10 @@ class FilterResolver { private final FilterBindings bindings; private final Metric metric; - private final boolean strictFiltering; - FilterResolver(FilterBindings bindings, Metric metric, boolean strictFiltering) { + FilterResolver(FilterBindings bindings, Metric metric) { this.bindings = bindings; this.metric = metric; - this.strictFiltering = strictFiltering; } Optional<RequestFilter> resolveRequestFilter(Request request, URI jdiscUri) { @@ -40,7 +38,7 @@ class FilterResolver { if (maybeFilterId.isPresent()) { metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(request, maybeFilterId.get())); request.setAttribute(RequestUtils.JDISC_REQUEST_CHAIN, maybeFilterId.get()); - } else if (!strictFiltering) { + } else if (!bindings.strictFiltering()) { metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(request, null)); } else { String syntheticFilterId = RejectingRequestFilter.SYNTHETIC_FILTER_CHAIN_ID; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index 6fdcc96bdc9..6720a7d95ed 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -54,19 +54,19 @@ class HttpRequestDispatch { private final RequestMetricReporter metricReporter; HttpRequestDispatch(JDiscContext jDiscContext, - AccessLogEntry accessLogEntry, - Context metricContext, - HttpServletRequest servletRequest, - HttpServletResponse servletResponse) throws IOException { + AccessLogEntry accessLogEntry, + Context metricContext, + HttpServletRequest servletRequest, + HttpServletResponse servletResponse) throws IOException { this.jDiscContext = jDiscContext; requestHandler = newRequestHandler(jDiscContext, accessLogEntry, servletRequest); this.jettyRequest = (Request) servletRequest; - this.metricReporter = new RequestMetricReporter(jDiscContext.metric, metricContext, jettyRequest.getTimeStamp()); + this.metricReporter = new RequestMetricReporter(jDiscContext.metric(), metricContext, jettyRequest.getTimeStamp()); this.servletResponseController = new ServletResponseController(servletRequest, servletResponse, - jDiscContext.janitor, + jDiscContext.janitor(), metricReporter, jDiscContext.developerMode()); shutdownConnectionGracefullyIfThresholdReached(jettyRequest); @@ -195,21 +195,21 @@ class HttpRequestDispatch { @SuppressWarnings("try") private ServletRequestReader handleRequest() throws IOException { - HttpRequest jdiscRequest = HttpRequestFactory.newJDiscRequest(jDiscContext.container, jettyRequest); + HttpRequest jdiscRequest = HttpRequestFactory.newJDiscRequest(jDiscContext.container(), jettyRequest); ContentChannel requestContentChannel; try (ResourceReference ref = References.fromResource(jdiscRequest)) { HttpRequestFactory.copyHeaders(jettyRequest, jdiscRequest); requestContentChannel = requestHandler.handleRequest(jdiscRequest, servletResponseController.responseHandler()); } - return new ServletRequestReader(jettyRequest, requestContentChannel, jDiscContext.janitor, metricReporter); + return new ServletRequestReader(jettyRequest, requestContentChannel, jDiscContext.janitor(), metricReporter); } private static RequestHandler newRequestHandler(JDiscContext context, AccessLogEntry accessLogEntry, HttpServletRequest servletRequest) { RequestHandler requestHandler = wrapHandlerIfFormPost( - new FilteringRequestHandler(context.filterResolver, (Request)servletRequest), - servletRequest, context.serverConfig.removeRawPostBodyForWwwUrlEncodedPost()); + new FilteringRequestHandler(context.filterResolver(), (Request)servletRequest), + servletRequest, context.removeRawPostBodyForWwwUrlEncodedPost()); return new AccessLoggingRequestHandler(requestHandler, accessLogEntry); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java index c80299b4737..9615b35fbd5 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java @@ -5,27 +5,17 @@ import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.service.CurrentContainer; -public class JDiscContext { - final FilterResolver filterResolver; - final CurrentContainer container; - final Janitor janitor; - final Metric metric; - final ServerConfig serverConfig; +record JDiscContext(FilterResolver filterResolver, + CurrentContainer container, + Janitor janitor, + Metric metric, + boolean developerMode, + boolean removeRawPostBodyForWwwUrlEncodedPost) { - public JDiscContext(FilterBindings filterBindings, - CurrentContainer container, - Janitor janitor, - Metric metric, - ServerConfig serverConfig) { - - this.filterResolver = new FilterResolver(filterBindings, metric, serverConfig.strictFiltering()); - this.container = container; - this.janitor = janitor; - this.metric = metric; - this.serverConfig = serverConfig; + public static JDiscContext of(FilterBindings filterBindings, CurrentContainer container, + Janitor janitor, Metric metric, ServerConfig config) { + return new JDiscContext(new FilterResolver(filterBindings, metric), container, janitor, + metric, config.developerMode(), config.removeRawPostBodyForWwwUrlEncodedPost()); } - public boolean developerMode() { - return serverConfig.developerMode(); - } } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java index bd052f14867..ac772b91539 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.Enumeration; import java.util.Map; import java.util.Set; +import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -33,14 +34,15 @@ class JDiscHttpServlet extends HttpServlet { public static final String ATTRIBUTE_NAME_ACCESS_LOG_ENTRY = JDiscHttpServlet.class.getName() + "_access-log-entry"; private final static Logger log = Logger.getLogger(JDiscHttpServlet.class.getName()); - private final JDiscContext context; private static final Set<String> servletSupportedMethods = Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE) .map(Method::name) .collect(Collectors.toSet()); - public JDiscHttpServlet(JDiscContext context) { + private final Supplier<JDiscContext> context; + + public JDiscHttpServlet(Supplier<JDiscContext> context) { this.context = context; } @@ -89,8 +91,8 @@ class JDiscHttpServlet extends HttpServlet { request.setAttribute(JDiscServerConnector.REQUEST_ATTRIBUTE, getConnector((Request) request)); Metric.Context metricContext = getMetricContext(request); - context.metric.add(MetricDefinitions.NUM_REQUESTS, 1, metricContext); - context.metric.add(MetricDefinitions.JDISC_HTTP_REQUESTS, 1, metricContext); + context.get().metric().add(MetricDefinitions.NUM_REQUESTS, 1, metricContext); + context.get().metric().add(MetricDefinitions.JDISC_HTTP_REQUESTS, 1, metricContext); String method = request.getMethod().toUpperCase(); if (servletSupportedMethods.contains(method)) { @@ -114,7 +116,7 @@ class JDiscHttpServlet extends HttpServlet { try { switch (request.getDispatcherType()) { case REQUEST: - new HttpRequestDispatch(context, accessLogEntry, metricContext, request, response).dispatchRequest(); + new HttpRequestDispatch(context.get(), accessLogEntry, metricContext, request, response).dispatchRequest(); break; default: if (log.isLoggable(Level.INFO)) { 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 7d84ee6f8a3..43e5fdaa397 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 @@ -5,11 +5,13 @@ import com.google.inject.Inject; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.logging.ConnectionLog; import com.yahoo.container.logging.RequestLog; +import com.yahoo.jdisc.AbstractResource; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.service.AbstractServerProvider; import com.yahoo.jdisc.service.CurrentContainer; +import com.yahoo.jdisc.service.ServerProvider; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.jmx.ConnectorServer; import org.eclipse.jetty.jmx.MBeanContainer; @@ -36,7 +38,10 @@ import java.net.BindException; import java.net.MalformedURLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Deque; import java.util.List; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -45,27 +50,26 @@ import java.util.stream.Collectors; * @author Simon Thoresen Hult * @author bjorncs */ -public class JettyHttpServer extends AbstractServerProvider { +public class JettyHttpServer extends AbstractResource implements ServerProvider { private final static Logger log = Logger.getLogger(JettyHttpServer.class.getName()); + private final ServerConfig config; private final Server server; private final List<Integer> listenedPorts = new ArrayList<>(); private final ServerMetricReporter metricsReporter; + private final Deque<JDiscContext> contexts = new ConcurrentLinkedDeque<>(); @Inject // ServerProvider implementors must use com.google.inject.Inject - public JettyHttpServer(CurrentContainer container, - Metric metric, + public JettyHttpServer(Metric metric, ServerConfig serverConfig, - FilterBindings filterBindings, - Janitor janitor, ComponentRegistry<ConnectorFactory> connectorFactories, RequestLog requestLog, ConnectionLog connectionLog) { - super(container); if (connectorFactories.allComponents().isEmpty()) throw new IllegalArgumentException("No connectors configured."); + this.config = serverConfig; server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); server.setRequestLog(new AccessLogRequestLog(requestLog)); @@ -81,9 +85,7 @@ public class JettyHttpServer extends AbstractServerProvider { } server.addBeanToAllConnectors(new ResponseMetricAggregator(serverConfig.metric())); - JDiscContext jDiscContext = new JDiscContext(filterBindings, container, janitor, metric, serverConfig); - - ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(jDiscContext)); + ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(this::newestContext)); List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors()) .map(JDiscServerConnector.class::cast) .toList(); @@ -91,6 +93,22 @@ public class JettyHttpServer extends AbstractServerProvider { this.metricsReporter = new ServerMetricReporter(metric, server); } + JDiscContext registerContext(FilterBindings filterBindings, CurrentContainer container, Janitor janitor, Metric metric) { + JDiscContext context = JDiscContext.of(filterBindings, container, janitor, metric, config); + contexts.addFirst(context); + return context; + } + + void deregisterContext(JDiscContext context) { + contexts.remove(context); + } + + JDiscContext newestContext() { + JDiscContext context = contexts.peekFirst(); + if (context == null) throw new IllegalStateException("JettyHttpServer has no registered JDiscContext"); + return context; + } + private static void setupJmx(Server server, ServerConfig serverConfig) { if (serverConfig.jmx().enabled()) { System.setProperty("java.rmi.server.hostname", "localhost"); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServerContext.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServerContext.java new file mode 100644 index 00000000000..4ff224e9087 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServerContext.java @@ -0,0 +1,37 @@ +package com.yahoo.jdisc.http.server.jetty; + +import com.google.inject.Inject; +import com.yahoo.component.AbstractComponent; +import com.yahoo.component.provider.ComponentRegistry; +import com.yahoo.container.Container; +import com.yahoo.container.logging.ConnectionLog; +import com.yahoo.container.logging.RequestLog; +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.service.CurrentContainer; + +/** + * @author jonmv + * + * Context that a {@link JettyHttpServer} uses to pass requests to JDisc. + * This registers itself with the server upon construction, and unregisters + * on deconstruction, and the server always uses the most recent context. + */ +public class JettyHttpServerContext extends AbstractComponent { + + private final JDiscContext context; + private final JettyHttpServer server; + + @Inject // Must use guice annotation due to setup in TestDriver + public JettyHttpServerContext(CurrentContainer container, Metric metric, FilterBindings filterBindings, + Janitor janitor, JettyHttpServer server) { + this.server = server; + this.context = server.registerContext(filterBindings, container, janitor, metric); + } + + @Override + public void deconstruct() { + server.deregisterContext(context); + } + +} 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..c6e8b3a997e 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 @@ -3,6 +3,7 @@ package com.yahoo.jdisc.http.server.jetty.testutils; import com.google.inject.AbstractModule; import com.google.inject.Module; +import com.google.inject.Singleton; import com.google.inject.util.Modules; import com.yahoo.container.logging.ConnectionLog; import com.yahoo.container.logging.RequestLog; @@ -12,6 +13,7 @@ import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.server.jetty.FilterBindings; import com.yahoo.jdisc.http.server.jetty.JettyHttpServer; +import com.yahoo.jdisc.http.server.jetty.JettyHttpServerContext; import com.yahoo.jdisc.http.server.jetty.VoidConnectionLog; import com.yahoo.jdisc.http.server.jetty.VoidRequestLog; import com.yahoo.security.SslContextBuilder; @@ -32,6 +34,7 @@ public class TestDriver implements AutoCloseable { private final com.yahoo.jdisc.test.TestDriver jdiscCoreTestDriver; private final JettyHttpServer server; + private final JettyHttpServerContext context; private final SSLContext sslContext; private TestDriver(Builder builder) { @@ -46,6 +49,7 @@ public class TestDriver implements AutoCloseable { com.yahoo.jdisc.test.TestDriver.newSimpleApplicationInstance(combinedModule); ContainerBuilder containerBuilder = jdiscCoreTestDriver.newContainerBuilder(); JettyHttpServer server = containerBuilder.getInstance(JettyHttpServer.class); + this.context = containerBuilder.getInstance(JettyHttpServerContext.class); containerBuilder.serverProviders().install(server); builder.handlers.forEach((binding, handler) -> containerBuilder.serverBindings().bind(binding, handler)); jdiscCoreTestDriver.activateContainer(containerBuilder); @@ -63,6 +67,7 @@ public class TestDriver implements AutoCloseable { @Override public void close() { shutdown(); } public boolean shutdown() { + context.deconstruct(); server.close(); server.release(); return jdiscCoreTestDriver.close(); @@ -83,9 +88,10 @@ public class TestDriver implements AutoCloseable { new AbstractModule() { @Override protected void configure() { + bind(JettyHttpServer.class).in(Singleton.class); bind(ServerConfig.class).toInstance(serverConfig); bind(ConnectorConfig.class).toInstance(connectorConfig); - bind(FilterBindings.class).toInstance(new FilterBindings.Builder().build()); + bind(FilterBindings.class).toInstance(new FilterBindings.Builder().setStrictFiltering(serverConfig.strictFiltering()).build()); bind(ConnectionLog.class).toInstance(new VoidConnectionLog()); bind(RequestLog.class).toInstance(new VoidRequestLog()); } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java index cc839768ad5..26e88bccf41 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java @@ -498,7 +498,7 @@ public class FilterTestCase { .build(); MetricConsumerMock metricConsumerMock = new MetricConsumerMock(); MyRequestHandler requestHandler = new MyRequestHandler(); - JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, metricConsumerMock, false); + JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, metricConsumerMock); testDriver.client().get("/status.html"); assertThat(requestHandler.awaitInvocation(), is(true)); @@ -519,9 +519,10 @@ public class FilterTestCase { FilterBindings filterBindings = new FilterBindings.Builder() .addRequestFilter("my-request-filter", filter) .addRequestFilterBinding("my-request-filter", "http://*/filtered/*") + .setStrictFiltering(true) .build(); MyRequestHandler requestHandler = new MyRequestHandler(); - JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, new MetricConsumerMock(), true); + JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, new MetricConsumerMock()); testDriver.client().get("/unfiltered/") .expectStatusCode(is(Response.Status.FORBIDDEN)) @@ -551,7 +552,7 @@ public class FilterTestCase { .addRequestFilterBinding("my-request-filter", "http://*/filtered/*") .build(); - JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, new MetricConsumerMock(), true); + JettyTestDriver testDriver = newDriver(requestHandler, filterBindings, new MetricConsumerMock()); testDriver.client().get("/filtered/") .expectStatusCode(is(Response.Status.OK)); @@ -561,28 +562,27 @@ public class FilterTestCase { } private static JettyTestDriver newDriver(MyRequestHandler requestHandler, FilterBindings filterBindings) { - return newDriver(requestHandler, filterBindings, new MetricConsumerMock(), false); + return newDriver(requestHandler, filterBindings, new MetricConsumerMock()); } private static JettyTestDriver newDriver( MyRequestHandler requestHandler, FilterBindings filterBindings, - MetricConsumerMock metricConsumer, - boolean strictFiltering) { + MetricConsumerMock metricConsumer) { return JettyTestDriver.newInstance( requestHandler, - newFilterModule(filterBindings, metricConsumer, strictFiltering)); + newFilterModule(filterBindings, metricConsumer)); } private static com.google.inject.Module newFilterModule( - FilterBindings filterBindings, MetricConsumerMock metricConsumer, boolean strictFiltering) { + FilterBindings filterBindings, MetricConsumerMock metricConsumer) { return Modules.combine( new AbstractModule() { @Override protected void configure() { bind(FilterBindings.class).toInstance(filterBindings); - bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder().strictFiltering(strictFiltering))); + bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder())); bind(ConnectorConfig.class).toInstance(new ConnectorConfig(new ConnectorConfig.Builder())); bind(ConnectionLog.class).toInstance(new VoidConnectionLog()); bind(RequestLog.class).toInstance(new VoidRequestLog()); @@ -602,7 +602,7 @@ public class FilterTestCase { @Override public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { try { - headerCopy.set(new HashMap<String, List<String>>(request.headers())); + headerCopy.set(new HashMap<>(request.headers())); ResponseDispatch.newInstance(Response.Status.OK).dispatch(handler); return null; } finally { diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java index cbe21d5581b..fffb4de2d8f 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java @@ -3,9 +3,11 @@ package com.yahoo.jdisc.http.server.jetty; import com.google.inject.AbstractModule; import com.google.inject.Module; +import com.google.inject.Singleton; import com.google.inject.util.Modules; import com.yahoo.container.logging.ConnectionLog; import com.yahoo.container.logging.RequestLog; +import com.yahoo.jdisc.application.GuiceRepository; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.server.jetty.testutils.ConnectorFactoryRegistryModule; import com.yahoo.jdisc.test.ServerProviderConformanceTest; @@ -767,14 +769,11 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { new AbstractModule() { @Override protected void configure() { - bind(FilterBindings.class) - .toInstance(new FilterBindings.Builder().build()); - bind(ServerConfig.class) - .toInstance(new ServerConfig(new ServerConfig.Builder())); - bind(ConnectionLog.class) - .toInstance(new VoidConnectionLog()); - bind(RequestLog.class) - .toInstance(new VoidRequestLog()); + bind(JettyHttpServer.class).in(Singleton.class); + bind(FilterBindings.class).toInstance(new FilterBindings.Builder().build()); + bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder())); + bind(ConnectionLog.class).toInstance(new VoidConnectionLog()); + bind(RequestLog.class).toInstance(new VoidRequestLog()); } }, new ConnectorFactoryRegistryModule()); @@ -786,6 +785,11 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { } @Override + public AutoCloseable configureServerProvider(GuiceRepository guice) { + return guice.getInstance(JettyHttpServerContext.class)::deconstruct; + } + + @Override public Integer newClient(final JettyHttpServer server) throws Throwable { return server.getListenPort(); } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java index 6e5ad367590..3398913221e 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/FilterBindingsProvider.java @@ -38,6 +38,7 @@ public class FilterBindingsProvider implements Provider<FilterBindings> { FilterBindings.Builder builder = new FilterBindings.Builder(); configureLegacyFilters(builder, componentId, legacyRequestFilters); configureFilters(builder, config, filterChainRepository); + builder.setStrictFiltering(config.strictFiltering()); this.filterBindings = builder.build(); } catch (Exception e) { throw new RuntimeException( diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java index bf6c75584be..0a7836ff6d3 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/ProcessingFactory.java @@ -82,7 +82,7 @@ class ProcessingFactory { String componentId = typeConfig.factorycomponent(); // Class name of the factory AbstractConcreteDocumentFactory cdf = docFactoryRegistry.getComponent(new ComponentId(componentId)); if (cdf == null) { - log.fine("Unable to get document factory component '" + componentId + "' from document factory registry."); + log.fine(() -> "Unable to get document factory component '" + componentId + "' from document factory registry."); return document; } return cdf.getDocumentCopy(document.getDataType().getName(), document, document.getId()); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java index 1cf4e3dd858..e30bb927204 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/test/ServerProviderConformanceTest.java @@ -11,6 +11,7 @@ import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.application.BindingSetSelector; import com.yahoo.jdisc.application.ContainerBuilder; +import com.yahoo.jdisc.application.GuiceRepository; import com.yahoo.jdisc.handler.AbstractRequestHandler; import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; @@ -75,6 +76,9 @@ public abstract class ServerProviderConformanceTest { Iterable<ByteBuffer> newResponseContent(); void validateResponse(V response) throws Throwable; + + default AutoCloseable configureServerProvider(GuiceRepository guice) { return () -> { }; } + } /** @@ -2784,6 +2788,7 @@ public abstract class ServerProviderConformanceTest { builder.serverBindings().bind(builder.getInstance(Key.get(String.class, Names.named("serverBinding"))), requestHandler); final T serverProvider = builder.guiceModules().getInstance(adapter.getServerProviderClass()); + AutoCloseable scaffolding = adapter.configureServerProvider(builder.guiceModules()); builder.serverProviders().install(serverProvider); if (builder.getInstance(Key.get(Boolean.class, Names.named("activateContainer")))) { driver.activateContainer(builder); @@ -2804,6 +2809,7 @@ public abstract class ServerProviderConformanceTest { requestHandler.awaitAsyncTasks(); } + scaffolding.close(); serverProvider.close(); driver.close(); } diff --git a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java index 7de2627209c..2b70cdc58aa 100644 --- a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java +++ b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java @@ -53,7 +53,7 @@ public class DocumentGenMojo extends AbstractMojo { private static final int STD_INDENT = 4; - @Parameter( defaultValue = "${project}", readonly = true ) + @Parameter(defaultValue = "${project}", readonly = true) private MavenProject project; /** |