Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow inflation exclusion based on path #4416

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -178,6 +178,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
private final IncludeExclude<String> _agentPatterns = new IncludeExclude<>(RegexSet.class);
private final IncludeExclude<String> _methods = new IncludeExclude<>();
private final IncludeExclude<String> _paths = new IncludeExclude<>(PathSpecSet.class);
private final IncludeExclude<String> _inflationPaths = new IncludeExclude<>(PathSpecSet.class);
private final IncludeExclude<String> _mimeTypes = new IncludeExclude<>();
private HttpField _vary;

Expand Down Expand Up @@ -320,6 +321,41 @@ public void addExcludedPaths(String... pathspecs)
}
}

/**
* Adds excluded Path Specs for request inflation filtering.
*
* <p>
* There are 2 syntaxes supported, Servlet <code>url-pattern</code> based, and
* Regex based. This means that the initial characters on the path spec
* line are very strict, and determine the behavior of the path matching.
* <ul>
* <li>If the spec starts with <code>'^'</code> the spec is assumed to be
* a regex based path spec and will match with normal Java regex rules.</li>
* <li>If the spec starts with <code>'/'</code> then spec is assumed to be
* a Servlet url-pattern rules path spec for either an exact match
* or prefix based match.</li>
* <li>If the spec starts with <code>'*.'</code> then spec is assumed to be
* a Servlet url-pattern rules path spec for a suffix based match.</li>
* <li>All other syntaxes are unsupported</li>
* </ul>
* <p>
* Note: inclusion takes precedence over exclude.
*
* @param pathspecs Path specs (as per servlet spec) to exclude. If a
* ServletContext is available, the paths are relative to the context path,
* otherwise they are absolute.<br>
* For backward compatibility the pathspecs may be comma separated strings, but this
* will not be supported in future versions.
* @see #addIncludedInflationPaths(String...)
*/
public void addExcludedInflatePaths(String... pathspecs)
{
for (String p: pathspecs)
{
_inflationPaths.exclude(StringUtil.csvSplit(p));
}
}

/**
* Adds included User-Agents for filtering.
*
Expand Down Expand Up @@ -417,6 +453,38 @@ public void addIncludedPaths(String... pathspecs)
}
}

/**
* Add included Path Specs for inflation filtering.
*
* <p>
* There are 2 syntaxes supported, Servlet <code>url-pattern</code> based, and
* Regex based. This means that the initial characters on the path spec
* line are very strict, and determine the behavior of the path matching.
* <ul>
* <li>If the spec starts with <code>'^'</code> the spec is assumed to be
* a regex based path spec and will match with normal Java regex rules.</li>
* <li>If the spec starts with <code>'/'</code> then spec is assumed to be
* a Servlet url-pattern rules path spec for either an exact match
* or prefix based match.</li>
* <li>If the spec starts with <code>'*.'</code> then spec is assumed to be
* a Servlet url-pattern rules path spec for a suffix based match.</li>
* <li>All other syntaxes are unsupported</li>
* </ul>
* <p>
* Note: inclusion takes precedence over exclusion.
*
* @param pathspecs Path specs (as per servlet spec) to include. If a
* ServletContext is available, the paths are relative to the context path,
* otherwise they are absolute
*/
public void addIncludedInflationPaths(String... pathspecs)
{
for (String p : pathspecs)
{
_inflationPaths.include(StringUtil.csvSplit(p));
}
}

@Override
protected void doStart() throws Exception
{
Expand Down Expand Up @@ -514,6 +582,18 @@ public String[] getExcludedPaths()
return excluded.toArray(new String[excluded.size()]);
}

/**
* Get the current filter list of excluded Path Specs
*
* @return the filter list of excluded Path Specs
* @see #getIncludedInflationPaths()
*/
public String[] getExcludedInflationPaths()
{
Set<String> excluded = _inflationPaths.getExcluded();
return excluded.toArray(new String[excluded.size()]);
}

/**
* Get the current filter list of included User-Agent patterns
*
Expand Down Expand Up @@ -562,6 +642,18 @@ public String[] getIncludedPaths()
return includes.toArray(new String[includes.size()]);
}

/**
* Get the current filter list of included inflation Path Specs
*
* @return the filter list of included inflation Path Specs
* @see #getExcludedInflationPaths()
*/
public String[] getIncludedInflationPaths()
{
Set<String> includes = _inflationPaths.getIncluded();
return includes.toArray(new String[includes.size()]);
}

/**
* Get the current filter list of included HTTP methods
*
Expand Down Expand Up @@ -627,7 +719,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
}

// Handle request inflation
if (_inflateBufferSize > 0)
if (_inflateBufferSize > 0 && isPathInflatable(path))
{
boolean inflate = false;
for (ListIterator<HttpField> i = baseRequest.getHttpFields().listIterator(); i.hasNext(); )
Expand Down Expand Up @@ -814,6 +906,23 @@ protected boolean isPathGzipable(String requestURI)
return _paths.test(requestURI);
}

/**
* Test if the provided Request URI is allowed based on the Path Specs inflation filters.
*
* @param requestURI the request uri
* @return whether inflation is allowed for the given the path
*/
protected boolean isPathInflatable(String requestURI)
{
if (_inflationPaths.isEmpty())
return true;

if (requestURI == null)
return true;

return _inflationPaths.test(requestURI);
}

@Override
public void recycle(Deflater deflater)
{
Expand Down
Expand Up @@ -64,6 +64,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;

@SuppressWarnings("serial")
public class GzipHandlerTest
Expand Down Expand Up @@ -105,6 +106,9 @@ public void init() throws Exception
gzipHandler.setMinGzipSize(16);
gzipHandler.setInflateBufferSize(4096);

gzipHandler.addIncludedMethods("POST");
gzipHandler.addExcludedInflatePaths("/ctx/noinflation");

ServletContextHandler context = new ServletContextHandler(gzipHandler, "/ctx");
ServletHandler servlets = context.getServletHandler();

Expand All @@ -119,11 +123,28 @@ public void init() throws Exception
servlets.addServletWithMapping(DumpServlet.class, "/dump/*");
servlets.addServletWithMapping(AsyncServlet.class, "/async/*");
servlets.addServletWithMapping(BufferServlet.class, "/buffer/*");
servlets.addServletWithMapping(NoInflationServlet.class, "/noinflation");
servlets.addFilterWithMapping(CheckFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));

_server.start();
}

public static class NoInflationServlet extends HttpServlet
{
@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
try(InputStream ignored = new GZIPInputStream(request.getInputStream()))
{
response.setStatus(200);
IO.copy(ignored, response.getOutputStream());
response.flushBuffer();
} catch (IOException ignore) {
fail("The inputstream was already inflated. addExcludedInflatePaths does not seem to be effective.");
}
}
}

public static class MicroServlet extends HttpServlet
{
@Override
Expand Down Expand Up @@ -676,6 +697,13 @@ public void testAddGetPaths()
String[] includedPaths = gzip.getIncludedPaths();
assertThat("Included Paths.size", includedPaths.length, is(2));
assertThat("Included Paths", Arrays.asList(includedPaths), contains("/foo", "^/bar.*$"));

gzip.addIncludedInflationPaths("/fiz");
gzip.addIncludedInflationPaths("^/buz.*$");

String[] includedInflationPaths = gzip.getIncludedInflationPaths();
assertThat("Included Paths.size", includedInflationPaths.length, is(2));
assertThat("Included Paths", Arrays.asList(includedInflationPaths), contains("/fiz", "^/buz.*$"));
}

@Test
Expand Down Expand Up @@ -801,6 +829,32 @@ public void testGzipBomb() throws Exception
assertThat(response.getContentBytes().length, is(512 * 1024));
}

@Test
public void testExcludeInflationButShouldBeZipped() throws Exception
{
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GZIPOutputStream output = new GZIPOutputStream(baos);
output.write(__content.getBytes(StandardCharsets.UTF_8));
output.close();
byte[] bytes = baos.toByteArray();

// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;

request.setMethod("POST");
request.setURI("/ctx/noinflation");
request.setVersion("HTTP/1.0");
request.setHeader("Host", "tester");
request.setHeader("Content-Type", "text/plain");
request.setHeader("Content-Encoding", "gzip");
request.setContent(bytes);

response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
assertThat(response.getStatus(), is(200));
assertThat(response.getContent(), is(__content));
}

public static class CheckFilter implements Filter
{
@Override
Expand Down