Skip to content

Commit

Permalink
Issue #7567 - don't compare params when checking MIME type for GzipHa…
Browse files Browse the repository at this point in the history
…ndler

Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
lachlan-roberts authored and joakime committed Feb 24, 2022
1 parent 353b132 commit aad7845
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
String mimeType = context == null ? MimeTypes.getDefaultMimeByExtension(path) : context.getMimeType(path);
if (mimeType != null)
{
mimeType = MimeTypes.getContentTypeWithoutCharset(mimeType);
mimeType = HttpField.valueParameters(mimeType, null);
if (!isMimeTypeGzipable(mimeType))
{
LOG.debug("{} excluded by path suffix mime type {}", this, request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpOutput;
Expand Down Expand Up @@ -160,8 +159,8 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
String ct = response.getContentType();
if (ct != null)
{
ct = MimeTypes.getContentTypeWithoutCharset(ct);
if (!_factory.isMimeTypeGzipable(StringUtil.asciiToLowerCase(ct)))
String baseType = HttpField.valueParameters(ct, null);
if (!_factory.isMimeTypeGzipable(StringUtil.asciiToLowerCase(baseType)))
{
LOG.debug("{} exclude by mimeType {}", this, ct);
noCompression();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@
package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.zip.GZIPInputStream;

import jakarta.servlet.MultipartConfigElement;
import jakarta.servlet.ServletException;
Expand All @@ -26,8 +31,13 @@
import jakarta.servlet.http.Part;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.util.BytesRequestContent;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.client.util.MultiPartRequestContent;
import org.eclipse.jetty.client.util.StringRequestContent;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.MimeTypes;
Expand All @@ -36,6 +46,7 @@
import org.eclipse.jetty.server.MultiPartFormInputStream;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.util.IO;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -44,7 +55,9 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

public class MultiPartServletTest
{
Expand Down Expand Up @@ -77,29 +90,53 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S
}
}

public static class MultiPartEchoServlet extends HttpServlet
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
if (!req.getContentType().contains(MimeTypes.Type.MULTIPART_FORM_DATA.asString()))
{
resp.sendError(400);
return;
}

resp.setContentType(req.getContentType());
IO.copy(req.getInputStream(), resp.getOutputStream());
}
}

@BeforeEach
public void start() throws Exception
{
tmpDir = Files.createTempDirectory(MultiPartServletTest.class.getSimpleName());
assertNotNull(tmpDir);

server = new Server();
connector = new ServerConnector(server);
server.addConnector(connector);

MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
MAX_FILE_SIZE, -1, 1);

ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
ServletHolder servletHolder = contextHandler.addServlet(MultiPartServlet.class, "/");

MultipartConfigElement config = new MultipartConfigElement(tmpDir.toAbsolutePath().toString(),
MAX_FILE_SIZE, -1, 1);
servletHolder.getRegistration().setMultipartConfig(config);
servletHolder = contextHandler.addServlet(MultiPartEchoServlet.class, "/echo");
servletHolder.getRegistration().setMultipartConfig(config);

server.setHandler(contextHandler);
GzipHandler gzipHandler = new GzipHandler();
gzipHandler.addIncludedMimeTypes("multipart/form-data");
gzipHandler.setMinGzipSize(32);
gzipHandler.setHandler(contextHandler);
server.setHandler(gzipHandler);

server.start();

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

@AfterEach
Expand Down Expand Up @@ -135,6 +172,44 @@ public void testTempFilesDeletedOnError() throws Exception
containsString("Multipart Mime part largePart exceeds max filesize"));
}

assertThat(tmpDir.toFile().list().length, is(0));
String[] fileList = tmpDir.toFile().list();
assertNotNull(fileList);
assertThat(fileList.length, is(0));
}

@Test
public void testMultiPartGzip() throws Exception
{
String contentString = "the quick brown fox jumps over the lazy dog, " +
"the quick brown fox jumps over the lazy dog";
StringRequestContent content = new StringRequestContent(contentString);

MultiPartRequestContent multiPart = new MultiPartRequestContent();
multiPart.addFieldPart("largePart", content, null);
multiPart.close();

try (StacklessLogging ignored = new StacklessLogging(HttpChannel.class, MultiPartFormInputStream.class))
{
InputStreamResponseListener responseStream = new InputStreamResponseListener();
client.newRequest("localhost", connector.getLocalPort())
.path("/echo")
.scheme(HttpScheme.HTTP.asString())
.method(HttpMethod.POST)
.headers(h -> h.add(HttpHeader.ACCEPT_ENCODING, "gzip"))
.body(multiPart)
.send(responseStream);

Response response = responseStream.get(5, TimeUnit.SECONDS);
HttpFields headers = response.getHeaders();
assertThat(headers.get(HttpHeader.CONTENT_TYPE), startsWith("multipart/form-data"));
assertThat(headers.get(HttpHeader.CONTENT_ENCODING), is("gzip"));

InputStream inputStream = new GZIPInputStream(responseStream.getInputStream());
String contentType = headers.get(HttpHeader.CONTENT_TYPE);
MultiPartFormInputStream mpis = new MultiPartFormInputStream(inputStream, contentType, null, null);
List<Part> parts = new ArrayList<>(mpis.getParts());
assertThat(parts.size(), is(1));
assertThat(IO.toString(parts.get(0).getInputStream()), is(contentString));
}
}
}

0 comments on commit aad7845

Please sign in to comment.