This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 28c60f19a3660329e06a6455753584c6c8450eb8 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Nov 11 14:41:12 2019 +0000 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63909 When the ExpiresFilter is used without a default and the response is served by the Default Servlet, ensure that the filter processes the response if the Default Servlet sets a 304 (Not Found) status code. --- .../org/apache/catalina/filters/ExpiresFilter.java | 48 +++++++++++-- .../apache/catalina/filters/TestExpiresFilter.java | 84 ++++++++++++++++++++++ test/webapp/bug6nnnn/bug69303.txt | 18 +++++ webapps/docs/changelog.xml | 6 ++ 4 files changed, 150 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/filters/ExpiresFilter.java b/java/org/apache/catalina/filters/ExpiresFilter.java index f746cc2..85d5fce 100644 --- a/java/org/apache/catalina/filters/ExpiresFilter.java +++ b/java/org/apache/catalina/filters/ExpiresFilter.java @@ -40,6 +40,7 @@ import javax.servlet.WriteListener; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; +import javax.servlet.http.MappingMatch; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -379,7 +380,7 @@ import org.apache.juli.logging.LogFactory; * {@link #isEligibleToExpirationHeaderGeneration(HttpServletRequest, XHttpServletResponse)} * </li> * <li> - * {@link #getExpirationDate(XHttpServletResponse)}</li> + * {@link #getExpirationDate(HttpServletRequest, XHttpServletResponse)}</li> * </ul> * <h2>Troubleshooting</h2> * <p> @@ -1247,22 +1248,57 @@ public class ExpiresFilter extends FilterBase { return excludedResponseStatusCodes; } + /** - * <p> * Returns the expiration date of the given {@link XHttpServletResponse} or * {@code null} if no expiration date has been configured for the * declared content type. - * </p> * <p> * {@code protected} for extension. - * </p> * - * @param response The Servlet response + * @param response The wrapped HTTP response + * * @return the expiration date * @see HttpServletResponse#getContentType() + * + * @deprecated Will be removed in Tomcat 10. + * Use {@link #getExpirationDate(HttpServletRequest, XHttpServletResponse)} */ + @Deprecated protected Date getExpirationDate(XHttpServletResponse response) { + return getExpirationDate((HttpServletRequest) null, response); + } + + + /** + * Returns the expiration date of the given {@link XHttpServletResponse} or + * {@code null} if no expiration date has been configured for the + * declared content type. + * <p> + * {@code protected} for extension. + * + * @param request The HTTP request + * @param response The wrapped HTTP response + * + * @return the expiration date + * @see HttpServletResponse#getContentType() + */ + protected Date getExpirationDate(HttpServletRequest request, XHttpServletResponse response) { String contentType = response.getContentType(); + if (contentType == null && request != null && + request.getHttpServletMapping().getMappingMatch() == MappingMatch.DEFAULT && + response.getStatus() == HttpServletResponse.SC_NOT_MODIFIED) { + // Default servlet normally sets the content type but does not for + // 304 responses. Look it up. + String servletPath = request.getServletPath(); + if (servletPath != null) { + int lastSlash = servletPath.lastIndexOf('/'); + if (lastSlash > -1) { + String fileName = servletPath.substring(lastSlash + 1); + contentType = request.getServletContext().getMimeType(fileName); + } + } + } if (contentType != null) { contentType = contentType.toLowerCase(Locale.ENGLISH); } @@ -1485,7 +1521,7 @@ public class ExpiresFilter extends FilterBase { return; } - Date expirationDate = getExpirationDate(response); + Date expirationDate = getExpirationDate(request, response); if (expirationDate == null) { if (log.isDebugEnabled()) { log.debug(sm.getString("expiresFilter.noExpirationConfigured", diff --git a/test/org/apache/catalina/filters/TestExpiresFilter.java b/test/org/apache/catalina/filters/TestExpiresFilter.java index 4049eb7..1a7a5a3 100644 --- a/test/org/apache/catalina/filters/TestExpiresFilter.java +++ b/test/org/apache/catalina/filters/TestExpiresFilter.java @@ -18,6 +18,7 @@ package org.apache.catalina.filters; import java.io.IOException; +import java.util.ArrayList; import java.util.Calendar; import java.util.HashMap; import java.util.List; @@ -42,8 +43,10 @@ import org.apache.catalina.filters.ExpiresFilter.StartingPoint; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; import org.apache.tomcat.util.buf.ByteChunk; +import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap; import org.apache.tomcat.util.descriptor.web.FilterDef; import org.apache.tomcat.util.descriptor.web.FilterMap; +import org.apache.tomcat.util.http.FastHttpDateFormat; public class TestExpiresFilter extends TomcatBaseTest { public static final String TEMP_DIR = System.getProperty("java.io.tmpdir"); @@ -484,4 +487,85 @@ public class TestExpiresFilter extends TomcatBaseTest { Assert.assertEquals(expected, actual); } + + + /* + * Tests Expires filter with: + * - per content type expires + * - no default + * - Default servlet returning 304s (without content-type) + */ + @Test + public void testBug63909() throws Exception { + + Tomcat tomcat = getTomcatInstanceTestWebapp(false, false); + Context ctxt = (Context) tomcat.getHost().findChild("/test"); + + FilterDef filterDef = new FilterDef(); + filterDef.addInitParameter("ExpiresByType text/xml;charset=utf-8", "access plus 3 minutes"); + filterDef.addInitParameter("ExpiresByType text/xml", "access plus 5 minutes"); + filterDef.addInitParameter("ExpiresByType text", "access plus 7 minutes"); + filterDef.addInitParameter("ExpiresExcludedResponseStatusCodes", ""); + + filterDef.setFilterClass(ExpiresFilter.class.getName()); + filterDef.setFilterName(ExpiresFilter.class.getName()); + + ctxt.addFilterDef(filterDef); + + FilterMap filterMap = new FilterMap(); + filterMap.setFilterName(ExpiresFilter.class.getName()); + filterMap.addURLPatternDecoded("*"); + ctxt.addFilterMap(filterMap); + + tomcat.start(); + + ByteChunk bc = new ByteChunk(); + Map<String,List<String>> requestHeaders = new CaseInsensitiveKeyMap<>(); + List<String> ifModifiedSinceValues = new ArrayList<>(); + ifModifiedSinceValues.add(FastHttpDateFormat.getCurrentDate()); + requestHeaders.put("If-Modified-Since", ifModifiedSinceValues); + Map<String,List<String>> responseHeaders = new CaseInsensitiveKeyMap<>(); + + int rc = getUrl("http://localhost:" + getPort() + "/test/bug6nnnn/bug69303.txt", bc, requestHeaders, responseHeaders); + + Assert.assertEquals(HttpServletResponse.SC_NOT_MODIFIED, rc); + + StringBuilder msg = new StringBuilder(); + for (Entry<String, List<String>> field : responseHeaders.entrySet()) { + for (String value : field.getValue()) { + msg.append((field.getKey() == null ? "" : field.getKey() + + ": ") + + value + "\n"); + } + } + System.out.println(msg); + + Integer actualMaxAgeInSeconds; + + String cacheControlHeader = getSingleHeader("Cache-Control", responseHeaders); + + if (cacheControlHeader == null) { + actualMaxAgeInSeconds = null; + } else { + actualMaxAgeInSeconds = null; + StringTokenizer cacheControlTokenizer = new StringTokenizer( + cacheControlHeader, ","); + while (cacheControlTokenizer.hasMoreTokens() && + actualMaxAgeInSeconds == null) { + String cacheDirective = cacheControlTokenizer.nextToken(); + StringTokenizer cacheDirectiveTokenizer = new StringTokenizer( + cacheDirective, "="); + if (cacheDirectiveTokenizer.countTokens() == 2) { + String key = cacheDirectiveTokenizer.nextToken().trim(); + String value = cacheDirectiveTokenizer.nextToken().trim(); + if (key.equalsIgnoreCase("max-age")) { + actualMaxAgeInSeconds = Integer.valueOf(value); + } + } + } + } + + Assert.assertNotNull(actualMaxAgeInSeconds); + Assert.assertTrue(Math.abs(actualMaxAgeInSeconds.intValue() - 420) < 3); + } } diff --git a/test/webapp/bug6nnnn/bug69303.txt b/test/webapp/bug6nnnn/bug69303.txt new file mode 100644 index 0000000..972f9d4 --- /dev/null +++ b/test/webapp/bug6nnnn/bug69303.txt @@ -0,0 +1,18 @@ +================================================================================ + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +================================================================================ + +The content is not important. \ No newline at end of file diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 970270b..c4bb27d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -74,6 +74,12 @@ <update> <bug>63905</bug> Clean up Tomcat CSS. (michaelo) </update> + <fix> + <bug>63909</bug>: When the <code>ExpiresFilter</code> is used without a + default and the response is served by the Default Servlet, ensure that + the filter processes the response if the Default Servlet sets a 304 (Not + Found) status code. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org