From 3cca6642d13144667cb8f60d2a025e1c83542a41 Mon Sep 17 00:00:00 2001 From: S K Date: Wed, 21 Jun 2017 23:18:30 -0700 Subject: [PATCH] Issue #1622 - HeaderFilter does not work if response has been committed (#1628) Signed-off-by: S K (xz64) --- .../administration/extras/header-filter.adoc | 17 ++-- .../eclipse/jetty/servlets/HeaderFilter.java | 58 ++++++------ .../servlets/IncludeExcludeBasedFilter.java | 33 ++++++- .../IncludeExcludeBasedFilterTest.java | 92 +++++++++---------- 4 files changed, 107 insertions(+), 93 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/administration/extras/header-filter.adoc b/jetty-documentation/src/main/asciidoc/administration/extras/header-filter.adoc index e7e97d9079e..58a08ac74ae 100644 --- a/jetty-documentation/src/main/asciidoc/administration/extras/header-filter.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/extras/header-filter.adoc @@ -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 @@ -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: diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/HeaderFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/HeaderFilter.java index c23b6b5b88c..0c2dc0dba24 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/HeaderFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/HeaderFilter.java @@ -80,42 +80,40 @@ public class HeaderFilter 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; + for (ConfiguredHeader header : _configuredHeaders) + { + if (header.isDate()) + { + 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 // constant header value + { + if (header.isAdd()) + { + http_response.addHeader(header.getName(),header.getValue()); + } + else + { + http_response.setHeader(header.getName(),header.getValue()); + } + } + } } - for (ConfiguredHeader header : _configuredHeaders) - { - if (header.isDate()) - { - 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 // constant header value - { - if (header.isAdd()) - { - http_response.addHeader(header.getName(),header.getValue()); - } - else - { - http_response.setHeader(header.getName(),header.getValue()); - } - } - } + chain.doFilter(request,response); } @Override diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilter.java index b1ba2cf4342..35f40be4823 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilter.java @@ -110,6 +110,33 @@ public abstract class IncludeExcludeBasedFilter implements Filter } } + 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(); @@ -120,12 +147,8 @@ public abstract class IncludeExcludeBasedFilter implements Filter 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"); diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilterTest.java index f9971247ad2..29285f70f82 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludeExcludeBasedFilterTest.java @@ -224,7 +224,24 @@ public class IncludeExcludeBasedFilterTest 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 testIncludeExcludeFilterIncludeMimeTypeMatchWithQueryString() throws Exception + { + FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class); + 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.json?some=value"); HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate())); Assert.assertTrue(response.contains("X-Custom-Value","1")); @@ -241,7 +258,24 @@ public class IncludeExcludeBasedFilterTest 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 testIncludeExcludeFilterIncludeMimeTypeNoMatchNoExtension() throws Exception + { + FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class); + 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/abcdef"); HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate())); Assert.assertFalse(response.contains("X-Custom-Value","1")); @@ -258,7 +292,7 @@ public class IncludeExcludeBasedFilterTest 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")); @@ -275,61 +309,26 @@ public class IncludeExcludeBasedFilterTest 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 testIncludeExcludeFilterIncludeMimeTypeSemicolonMatch() throws Exception - { - FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class); - 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-utf8"); - - HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate())); - Assert.assertTrue(response.contains("X-Custom-Value","1")); - } - - @Test - public void testIncludeExcludeFilterIncludeMimeTypeSemicolonNoMatch() throws Exception - { - FilterHolder holder = new FilterHolder(MockIncludeExcludeFilter.class); - 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-utf8"); - - HttpTester.Response response = HttpTester.parseResponse(_tester.getResponses(request.generate())); - Assert.assertFalse(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); } } @@ -338,15 +337,8 @@ public class IncludeExcludeBasedFilterTest @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(); } }