| Summary: | ExpiresFilter not account for 304 when content-type is null | ||
|---|---|---|---|
| Product: | Tomcat 8 | Reporter: | some.java.guy |
| Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 8.5.x-trunk | ||
| Target Milestone: | ---- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
“CustomExpiresFilter” should be “ExpiresFilter” in original config example in description. From RFC 7232, section 4.1: <quote> The server generating a 304 response MUST generate any of the following header fields that would have been sent in a 200 (OK) response to the same request: Cache-Control, Content-Location, Date, ETag, Expires, and Vary. </quote> I'm currently looking at ways to fix this. I'm not sure a complete solution is practical. A solution that addresses static resources and Servlets that override HttpServlet.getLastmodified() looks more likely. The Servlet API does not expose the information required to fix this for Servlets that override getLastModified(). That could be overcome by reflection but then the code would have to assume that the Content-Type for a given Servlet was constant. That may be true in most cases but it isn't guaranteed to true and I don't like building solutions based on assumptions I know to be false. The Default Servlet case can be fixed generically, and entirely in the ExpiresFilter but only by using Servlet 4.0 API. Therefore I have fixed this in 9.0.x using the Servlet 4.0 API and in 8.5.x using the Servlet 4 preview package. I don't see an easy way to fix this in 7.0.x. Fixed in: - master for 9.0.28 onwards - 8.5.x for 8.5.48 onwards |
Firstly, I apologize as I am quite new to contributing so I hope I am pursuing the correct path. I have struggled mightily recently trying to track down why our caching configuration was not really working with tomcat 8.5.x and we finally found the nugget in the ExpiresFilter. Here is our ExpiresFilter configuration snippet. <filter> <filter-name>ExpiresFilter</filter-name> <filter-class>org.apache.catalina.filters.ExpiresFilter</filter-class> <init-param> <param-name>ExpiresByType image/gif</param-name> <param-value>access plus 6 hours</param-value> </init-param> <init-param> <param-name>ExpiresByType text/css</param-name> <param-value>access plus 6 hours</param-value> </init-param> <init-param> <param-name>ExpiresByType application/javascript</param-name> <param-value>access plus 6 hours</param-value> </init-param> <init-param> <param-name>ExpiresExcludedResponseStatusCodes</param-name> <param-value>302, 500, 503</param-value> </init-param> </filter> <filter-mapping> <filter-name>CustomExpiresFilter</filter-name> <url-pattern>/*</url-pattern> <dispatcher>REQUEST</dispatcher> </filter-mapping> The ExpiresFilter according to the documentation can be configured to expire content by type or with a default or with both. However, there is a hole in the current logic of the getExpirationDate(XHttpServletResponse response) method if the filter is not configured with a default. When the response status is 304 (which in our case not excluded - see config above) the content-type is null in the first lines of the getExpirationDate method. The ExpiresFilter is unable to set the appropriate cache-control and expires headers because the content-type is not present. The net effect is that once the content has expired with the ExpiresFilter configuration settings above, the cache-control and expires headers cannot be set by the ExpiresFilter and a slew of incoming browser requests for the same static content are met with repeated 304s. Essentially, using only ExpiresByType in ExpiresFilter results in successful caching, but only until the content expires, at which point, the wave of 304s begin. I don't think this was the intended effect. One could argue that, even when a 304 is returned, one can still set the cache-control and expires headers appropriately and extend the cache lifetime thus reducing load as intended. One way to do this is to add logic to the ExpiresFilter to use the request URI and request file metadata to determine what the content type would have been in this scenario - another might be an in memory collection of URIs to content type for lookup. I am sure there are others but not knowing the source code surrounding this in depth, I cannot say what would be best. In our research several people have solved this issue by changing or entirely removing other headers like the ETag header and the Last-Modified and that does not seem correct. I think it is perfectly valid to have ETag with a 304 and cache-control and expires headers and maybe even more elegant and appropriate?