Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-…
Browse files Browse the repository at this point in the history
…legacyMultipartParser
  • Loading branch information
lachlan-roberts committed May 25, 2022
2 parents 1c24238 + f1c2b0d commit a61f145
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,10 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r
InputStreamResponseListener listener = new InputStreamResponseListener();
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.timeout(5, TimeUnit.SECONDS)
.timeout(20, TimeUnit.SECONDS)
.send(listener);

Response response = listener.get(5, TimeUnit.SECONDS);
Response response = listener.get(20, TimeUnit.SECONDS);
assertEquals(HttpStatus.OK_200, response.getStatus());

ByteArrayOutputStream output = new ByteArrayOutputStream();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,11 @@ public Runnable onRequest(HeadersFrame frame)
{
if (LOG.isDebugEnabled())
LOG.debug("onRequest() failure", x);
onBadMessage(x);
return null;
return () -> onBadMessage(x);
}
catch (Throwable x)
{
onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
return null;
return () -> onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
}
}

Expand Down
2 changes: 1 addition & 1 deletion jetty-p2/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
artifacts are created -->
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-bom</artifactId>
<artifactId>jetty-home</artifactId>
<version>${project.version}</version>
<type>pom</type>
</dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,14 +664,20 @@ public void setInflateBufferSize(int size)
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
if (baseRequest.isHandled())
{
super.handle(target, baseRequest, request, response);
return;
}

final ServletContext context = baseRequest.getServletContext();
final String path = baseRequest.getPathInContext();
LOG.debug("{} handle {} in {}", this, baseRequest, context);

if (!_dispatchers.contains(baseRequest.getDispatcherType()))
{
LOG.debug("{} excluded by dispatcherType {}", this, baseRequest.getDispatcherType());
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
return;
}

Expand All @@ -688,6 +694,15 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
baseRequest.getHttpInput().addInterceptor(gzipHttpInputInterceptor);
}

// From here on out, the response output gzip determination is made

// Don't attempt to modify the response output if it's already committed.
if (response.isCommitted())
{
super.handle(target, baseRequest, request, response);
return;
}

// Are we already being gzipped?
HttpOutput out = baseRequest.getResponse().getHttpOutput();
HttpOutput.Interceptor interceptor = out.getInterceptor();
Expand Down Expand Up @@ -764,15 +779,15 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
if (alreadyGzipped)
{
LOG.debug("{} already intercepting {}", this, request);
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
return;
}

// If not a supported method - no Vary because no matter what client, this URI is always excluded
if (!_methods.test(baseRequest.getMethod()))
{
LOG.debug("{} excluded by method {}", this, request);
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
return;
}

Expand All @@ -781,7 +796,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
if (!isPathGzipable(path))
{
LOG.debug("{} excluded by path {}", this, request);
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
return;
}

Expand All @@ -794,7 +809,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
{
LOG.debug("{} excluded by path suffix mime type {}", this, request);
// handle normally without setting vary header
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
return;
}
}
Expand All @@ -804,9 +819,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
{
// install interceptor and handle
out.setInterceptor(new GzipHttpOutputInterceptor(this, getVaryField(), baseRequest.getHttpChannel(), origInterceptor, isSyncFlush()));

if (_handler != null)
_handler.handle(target, baseRequest, request, response);
super.handle(target, baseRequest, request, response);
}
finally
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.util.concurrent.LinkedBlockingQueue;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.server.handler.ResourceHandler;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.PathResource;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

/**
* Tests of behavior of GzipHandler when Request.isHandled() or Response.isCommitted() is true
*/
public class GzipHandlerIsHandledTest
{
public WorkDir workDir;

private Server server;
private HttpClient client;
public LinkedBlockingQueue<String> events = new LinkedBlockingQueue<>();

public void startServer(Handler rootHandler) throws Exception
{
server = new Server();
ServerConnector connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);

server.setHandler(rootHandler);
server.start();

client = new HttpClient();
client.start();
}

@AfterEach
public void tearDown()
{
LifeCycle.stop(client);
LifeCycle.stop(server);
}

@Test
public void testRequest() throws Exception
{
HandlerCollection handlers = new HandlerCollection();

ResourceHandler resourceHandler = new ResourceHandler();
resourceHandler.setBaseResource(new PathResource(workDir.getPath()));
resourceHandler.setDirectoriesListed(true);
resourceHandler.setHandler(new EventHandler(events, "ResourceHandler"));

GzipHandler gzipHandler = new GzipHandler();
gzipHandler.setMinGzipSize(32);
gzipHandler.setHandler(new EventHandler(events, "GzipHandler-wrapped-handler"));

handlers.setHandlers(new Handler[]{resourceHandler, gzipHandler, new DefaultHandler()});

startServer(handlers);

ContentResponse response = client.GET(server.getURI().resolve("/"));
assertThat("response.status", response.getStatus(), is(200));
// we should have received a directory listing from the ResourceHandler
assertThat("response.content", response.getContentAsString(), containsString("Directory: /"));
// resource handler should have handled the request
// the gzip handler and default handlers should have been executed, seeing as this is a HandlerCollection
// but the gzip handler should not have acted on the request, as the response is committed
assertThat("One event should have been recorded", events.size(), is(1));
// the event handler should see the request.isHandled = true
// and response.isCommitted = true as the gzip handler didn't really do anything due to these
// states and let the wrapped handler (the EventHandler in this case) make the call on what it should do.
assertThat("Event indicating that GzipHandler-wrapped-handler ran", events.remove(), is("GzipHandler-wrapped-handler [request.isHandled=true, response.isCommitted=true]"));
}

private static class EventHandler extends AbstractHandler
{
private final LinkedBlockingQueue<String> events;
private final String action;

public EventHandler(LinkedBlockingQueue<String> events, String action)
{
this.events = events;
this.action = action;
}

@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
events.offer(String.format("%s [request.isHandled=%b, response.isCommitted=%b]", action, baseRequest.isHandled(), response.isCommitted()));
}
}
}
12 changes: 6 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
<junit.version>5.8.2</junit.version>
<kerb-simplekdc.version>2.0.2</kerb-simplekdc.version>
<log4j2.version>2.17.2</log4j2.version>
<logback.version>1.3.0-alpha15</logback.version>
<logback.version>1.3.0-alpha16</logback.version>
<mariadb.version>3.0.4</mariadb.version>
<mariadb.docker.version>10.3.6</mariadb.docker.version>
<maven-artifact-transfer.version>0.13.1</maven-artifact-transfer.version>
Expand All @@ -116,12 +116,12 @@
<org.osgi.util.function.version>1.2.0</org.osgi.util.function.version>
<org.osgi.util.promise.version>1.2.0</org.osgi.util.promise.version>
<plexus-component-annotations.version>2.1.1</plexus-component-annotations.version>
<plexus-utils.version>3.4.1</plexus-utils.version>
<plexus-utils.version>3.4.2</plexus-utils.version>
<slf4j.version>2.0.0-alpha6</slf4j.version>
<springboot.version>2.1.1.RELEASE</springboot.version>
<taglibs-standard-impl.version>1.2.5</taglibs-standard-impl.version>
<taglibs-standard-spec.version>1.2.5</taglibs-standard-spec.version>
<testcontainers.version>1.17.1</testcontainers.version>
<testcontainers.version>1.17.2</testcontainers.version>
<weld.version>3.1.9.Final</weld.version>
<wildfly.common.version>1.6.0.Final</wildfly.common.version>
<wildfly.elytron.version>1.19.0.Final</wildfly.elytron.version>
Expand All @@ -141,7 +141,7 @@
<license.maven.plugin.version>4.1</license.maven.plugin.version>
<maven.antrun.plugin.version>3.1.0</maven.antrun.plugin.version>
<maven.assembly.plugin.version>3.3.0</maven.assembly.plugin.version>
<maven.bundle.plugin.version>5.1.4</maven.bundle.plugin.version>
<maven.bundle.plugin.version>5.1.6</maven.bundle.plugin.version>
<maven.clean.plugin.version>3.2.0</maven.clean.plugin.version>
<maven.checkstyle.plugin.version>3.1.2</maven.checkstyle.plugin.version>
<maven.compiler.plugin.version>3.10.1</maven.compiler.plugin.version>
Expand All @@ -166,8 +166,8 @@
<maven.surefire.plugin.version>3.0.0-M5</maven.surefire.plugin.version>
<maven.source.plugin.version>3.2.1</maven.source.plugin.version>
<maven.war.plugin.version>3.3.2</maven.war.plugin.version>
<spotbugs.maven.plugin.version>4.6.0.0</spotbugs.maven.plugin.version>
<versions.maven.plugin.version>2.10.0</versions.maven.plugin.version>
<spotbugs.maven.plugin.version>4.7.0.0</spotbugs.maven.plugin.version>
<versions.maven.plugin.version>2.11.0</versions.maven.plugin.version>

<!-- testing -->
<invoker.mergeUserSettings>false</invoker.mergeUserSettings>
Expand Down

0 comments on commit a61f145

Please sign in to comment.