Skip to content

Commit

Permalink
Issue #1622 - HeaderFilter does not work if response has been committ…
Browse files Browse the repository at this point in the history 
…ed (#1628)


Signed-off-by: S K (xz64) <xz64.xyz@gmail.com>
  • Loading branch information
@xz64 @joakime
xz64 authored and joakime committed Aug 8, 2017
1 parent e32136f commit a7e4f92
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 70 deletions.
17 changes: 9 additions & 8 deletions jetty-documentation/src/main/asciidoc/administration/extras/header-filter.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
[[header-filter-usage]]
==== Usage

The header filter sets or adds headers to each response based on an optionally included/excluded list of path specs, mime types, and/or HTTP methods.
The header filter sets or adds headers to each response based on an optionally included/excluded list of path specs, mime types, and/or HTTP methods.
This filter processes its configured headers before calling `doFilter` in the filter chain. Some of the headers configured in this filter may get overwritten by other filters and/or the servlet processing the request.

===== Required JARs

Expand Down Expand Up @@ -76,25 +77,25 @@ ____
The following `init` parameters control the behavior of the filter:

includedPaths::
Optional. CSV of included path specs.
Optional. Comma separated values of included path specs.

excludedPaths::
Optional. CSV of excluded path specs.
Optional. Comma separated values of excluded path specs.

includedMimeTypes::
Optional. CSV of included mime types.
Optional. Comma separated values of included mime types. The mime type will be guessed from the extension at the end of the request URL if the content type has not been set on the response.

excludedMimeTypes::
Optional. CSV of excluded mime types.
Optional. Comma separated values of excluded mime types. The mime type will be guessed from the extension at the end of the request URL if the content type has not been set on the response.

includedHttpMethods::
Optional. CSV of included http methods.
Optional. Comma separated values of included http methods.

excludedHttpMethods::
Optional. CSV of excluded http methods.
Optional. Comma separated values of excluded http methods.

headerConfig::
CSV of actions to perform on headers. The syntax for each action is `action headerName: headerValue`.
Comma separated values of actions to perform on headers. The syntax for each action is `action headerName: headerValue`.

Supported header actions:

Expand Down
48 changes: 23 additions & 25 deletions jetty-servlets/src/main/java/org/eclipse/jetty/servlets/HeaderFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,42 +80,40 @@ public void init(FilterConfig filterConfig) throws ServletException
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException
{
chain.doFilter(request,response);

HttpServletRequest http_request = (HttpServletRequest)request;
HttpServletResponse http_response = (HttpServletResponse)response;

if (!super.shouldFilter(http_request,http_response))
if (super.shouldFilter(http_request,http_response))
{
return;
}

for (ConfiguredHeader header : _configuredHeaders)
{
if (header.isDate())
for (ConfiguredHeader header : _configuredHeaders)
{
long header_value = System.currentTimeMillis() + header.getMsOffset();
if (header.isAdd())
if (header.isDate())
{
http_response.addDateHeader(header.getName(),header_value);
long header_value = System.currentTimeMillis() + header.getMsOffset();
if (header.isAdd())
{
http_response.addDateHeader(header.getName(),header_value);
}
else
{
http_response.setDateHeader(header.getName(),header_value);
}
}
else
else // constant header value
{
http_response.setDateHeader(header.getName(),header_value);
}
}
else // constant header value
{
if (header.isAdd())
{
http_response.addHeader(header.getName(),header.getValue());
}
else
{
http_response.setHeader(header.getName(),header.getValue());
if (header.isAdd())
{
http_response.addHeader(header.getName(),header.getValue());
}
else
{
http_response.setHeader(header.getName(),header.getValue());
}
}
}
}

chain.doFilter(request,response);
}

@Override
Expand Down
33 changes: 28 additions & 5 deletions jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,33 @@ public void init(FilterConfig filterConfig) throws ServletException
}
}

protected String guessMimeType(HttpServletRequest http_request, HttpServletResponse http_response)
{
String content_type = http_response.getContentType();
LOG.debug("Content Type is: {}",content_type);

String mime_type = "";
if (content_type != null)
{
mime_type = MimeTypes.getContentTypeWithoutCharset(content_type);
LOG.debug("Mime Type is: {}",mime_type);
}
else
{
String request_url = http_request.getPathInfo();
mime_type = MimeTypes.getDefaultMimeByExtension(request_url);

if (mime_type == null)
{
mime_type = "";
}

LOG.debug("Guessed mime type is {}",mime_type);
}

return mime_type;
}

protected boolean shouldFilter(HttpServletRequest http_request, HttpServletResponse http_response)
{
String http_method = http_request.getMethod();
Expand All @@ -120,12 +147,8 @@ protected boolean shouldFilter(HttpServletRequest http_request, HttpServletRespo
return false;
}

String content_type = http_response.getContentType();
LOG.debug("Content Type is: {}",content_type);
content_type = (content_type == null)?"":content_type;
String mime_type = MimeTypes.getContentTypeWithoutCharset(content_type);
String mime_type = guessMimeType(http_request,http_response);

LOG.debug("Mime Type is: {}",content_type);
if (!_mimeTypes.test(mime_type))
{
LOG.debug("should not apply filter because mime type does not match");
Expand Down
56 changes: 24 additions & 32 deletions jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,112 +224,111 @@ public void testIncludeExcludeFilterIncludeMimeTypeMatch() throws Exception
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json");
request.setURI("/context/test/json.json");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertTrue(response.contains("X-Custom-Value","1"));
}

@Test
public void testIncludeExcludeFilterIncludeMimeTypeNoMatch() throws Exception
public void testIncludeExcludeFilterIncludeMimeTypeMatchWithQueryString() throws Exception
{
FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class);
holder.setInitParameter("includedMimeTypes","application/xml");
holder.setInitParameter("includedMimeTypes","application/json");
_tester.getContext().getServletHandler().addFilterWithMapping(holder,"/*",EnumSet.of(DispatcherType.REQUEST));

HttpTester.Request request = HttpTester.newRequest();
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json");
request.setURI("/context/test/json.json?some=value");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertFalse(response.contains("X-Custom-Value","1"));
Assert.assertTrue(response.contains("X-Custom-Value","1"));
}

@Test
public void testIncludeExcludeFilterExcludeMimeTypeMatch() throws Exception
public void testIncludeExcludeFilterIncludeMimeTypeNoMatch() throws Exception
{
FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class);
holder.setInitParameter("excludedMimeTypes","application/json");
holder.setInitParameter("includedMimeTypes","application/xml");
_tester.getContext().getServletHandler().addFilterWithMapping(holder,"/*",EnumSet.of(DispatcherType.REQUEST));

HttpTester.Request request = HttpTester.newRequest();
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json");
request.setURI("/context/test/json.json");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertFalse(response.contains("X-Custom-Value","1"));
}

@Test
public void testIncludeExcludeFilterExcludeMimeTypeNoMatch() throws Exception
public void testIncludeExcludeFilterIncludeMimeTypeNoMatchNoExtension() throws Exception
{
FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class);
holder.setInitParameter("excludedMimeTypes","application/xml");
holder.setInitParameter("includedMimeTypes","application/json");
_tester.getContext().getServletHandler().addFilterWithMapping(holder,"/*",EnumSet.of(DispatcherType.REQUEST));

HttpTester.Request request = HttpTester.newRequest();
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json");
request.setURI("/context/test/abcdef");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertTrue(response.contains("X-Custom-Value","1"));
Assert.assertFalse(response.contains("X-Custom-Value","1"));
}

@Test
public void testIncludeExcludeFilterIncludeMimeTypeSemicolonMatch() throws Exception
public void testIncludeExcludeFilterExcludeMimeTypeMatch() throws Exception
{
FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class);
holder.setInitParameter("includedMimeTypes","application/json");
holder.setInitParameter("excludedMimeTypes","application/json");
_tester.getContext().getServletHandler().addFilterWithMapping(holder,"/*",EnumSet.of(DispatcherType.REQUEST));

HttpTester.Request request = HttpTester.newRequest();
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json-utf8");
request.setURI("/context/test/json.json");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertTrue(response.contains("X-Custom-Value","1"));
Assert.assertFalse(response.contains("X-Custom-Value","1"));
}

@Test
public void testIncludeExcludeFilterIncludeMimeTypeSemicolonNoMatch() throws Exception
public void testIncludeExcludeFilterExcludeMimeTypeNoMatch() throws Exception
{
FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class);
holder.setInitParameter("includedMimeTypes","application/xml");
holder.setInitParameter("excludedMimeTypes","application/xml");
_tester.getContext().getServletHandler().addFilterWithMapping(holder,"/*",EnumSet.of(DispatcherType.REQUEST));

HttpTester.Request request = HttpTester.newRequest();
request.setMethod("GET");
request.setVersion("HTTP/1.1");
request.setHeader("Host","localhost");
request.setURI("/context/test/json-utf8");
request.setURI("/context/test/json.json");

HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate()));
Assert.assertFalse(response.contains("X-Custom-Value","1"));
Assert.assertTrue(response.contains("X-Custom-Value","1"));
}

public static class MockIncludeExcludeFilter extends IncludeExcludeBasedFilter
{
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException
{
chain.doFilter(request,response);
HttpServletRequest http_request = (HttpServletRequest)request;
HttpServletResponse http_response = (HttpServletResponse)response;

if (!super.shouldFilter(http_request,http_response))
if (super.shouldFilter(http_request,http_response))
{
return;
http_response.setHeader("X-Custom-Value","1");
}

http_response.setHeader("X-Custom-Value","1");
chain.doFilter(request,response);
}
}

Expand All @@ -338,15 +337,8 @@ public static class NullServlet extends HttpServlet
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
if (req.getPathInfo().equals("/json"))
{
resp.setContentType("application/json");
}
else if (req.getPathInfo().equals("/json-utf8"))
{
resp.setContentType("application/json; charset=utf-8");
}
resp.setStatus(HttpStatus.NO_CONTENT_204);
resp.flushBuffer();
}

}
Expand Down

0 comments on commit a7e4f92

Please sign in to comment.