You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/11/11 15:01:19 UTC

[tomcat] 03/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63909

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 53d6115d683471d1094cc72a545db891b42241d5
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 6351c5e..13dfa5e 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 0e701f3..efe9306 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -73,6 +73,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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org