This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 7538d05b36 Fix BZ 69415. ExpiresFilter is too eager to add headers
7538d05b36 is described below
commit 7538d05b36a8286846b046df4c9628b8443e9356
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Nov 1 17:15:32 2024 +0000
Fix BZ 69415. ExpiresFilter is too eager to add headers
https://bz.apache.org/bugzilla/show_bug.cgi?id=69415
The headers should only be added to methods that support caching and
where no-store has not been set
---
.../org/apache/catalina/filters/ExpiresFilter.java | 22 +++++++++++++++++++---
.../catalina/filters/LocalStrings.properties | 4 +++-
.../apache/catalina/filters/TestExpiresFilter.java | 18 ++++++++++++++++++
webapps/docs/changelog.xml | 6 ++++++
4 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/catalina/filters/ExpiresFilter.java
b/java/org/apache/catalina/filters/ExpiresFilter.java
index b2215cdb2e..1cb05f0d82 100644
--- a/java/org/apache/catalina/filters/ExpiresFilter.java
+++ b/java/org/apache/catalina/filters/ExpiresFilter.java
@@ -1397,9 +1397,17 @@ public class ExpiresFilter extends FilterBase {
*/
protected boolean
isEligibleToExpirationHeaderGeneration(HttpServletRequest request,
XHttpServletResponse response) {
- boolean expirationHeaderHasBeenSet =
- response.containsHeader(HEADER_EXPIRES) ||
contains(response.getCacheControlHeader(), "max-age");
- if (expirationHeaderHasBeenSet) {
+
+ // Don't add cache headers unless the request is a GET or a HEAD
request
+ String method = request.getMethod();
+ if (!"GET".equals(method) && !"HEAD".equals(method)) {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("expiresFilter.invalidMethod",
request.getRequestURI(), method));
+ }
+ return false;
+ }
+
+ if (response.containsHeader(HEADER_EXPIRES) ||
contains(response.getCacheControlHeader(), "max-age")) {
if (log.isDebugEnabled()) {
log.debug(sm.getString("expiresFilter.expirationHeaderAlreadyDefined",
request.getRequestURI(),
Integer.valueOf(response.getStatus()),
response.getContentType()));
@@ -1407,6 +1415,14 @@ public class ExpiresFilter extends FilterBase {
return false;
}
+ if (contains(response.getCacheControlHeader(), "no-store")) {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("expiresFilter.cacheControlNoStore",
request.getRequestURI(),
+ Integer.valueOf(response.getStatus()),
response.getContentType()));
+ }
+ return false;
+ }
+
for (int skippedStatusCode : this.excludedResponseStatusCodes) {
if (response.getStatus() == skippedStatusCode) {
if (log.isDebugEnabled()) {
diff --git a/java/org/apache/catalina/filters/LocalStrings.properties
b/java/org/apache/catalina/filters/LocalStrings.properties
index 89628aa04f..6a4f97cd5a 100644
--- a/java/org/apache/catalina/filters/LocalStrings.properties
+++ b/java/org/apache/catalina/filters/LocalStrings.properties
@@ -34,8 +34,10 @@ csrfPrevention.rejectNoCache=Rejecting request for [{0}]
with session [{1}] due
csrfPrevention.rejectNoNonce=Rejecting request for [{0}] with session [{1}]
because no CSRF nonce was found
csrfPrevention.unsupportedPattern=Unsupported pattern [{0}]
+expiresFilter.cacheControlNoStore=Request [{0}] with response status [{1}]
content-type [{2}], Cache-Control: no-store header already defined
expiresFilter.exceptionProcessingParameter=Exception processing configuration
parameter [{0}]:[{1}]
-expiresFilter.expirationHeaderAlreadyDefined=Request [{0}] with response
status [{1}] content-type [{2}], expiration header already defined
+expiresFilter.expirationHeaderAlreadyDefined=Request [{0}] with response
status [{1}] content-type [{2}], Expires header already defined
+expiresFilter.invalidMethod=Request [{0}] with request method [{1}],
non-cacheable method
expiresFilter.filterInitialized=Filter initialized with configuration [{0}]
expiresFilter.invalidDurationNumber=Invalid duration (number) [{0}] in
directive [{1}]
expiresFilter.invalidDurationUnit=Invalid duration unit
(years|months|weeks|days|hours|minutes|seconds) [{0}] in directive [{1}]
diff --git a/test/org/apache/catalina/filters/TestExpiresFilter.java
b/test/org/apache/catalina/filters/TestExpiresFilter.java
index 38acb2eaa2..83a82d3a6e 100644
--- a/test/org/apache/catalina/filters/TestExpiresFilter.java
+++ b/test/org/apache/catalina/filters/TestExpiresFilter.java
@@ -327,6 +327,24 @@ public class TestExpiresFilter extends TomcatBaseTest {
validate(servlet, Integer.valueOf(1 * 60));
}
+ @Test
+ public void testUseDefaultConfiguration3() throws Exception {
+ HttpServlet servlet = new HttpServlet() {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void service(HttpServletRequest request,
HttpServletResponse response)
+ throws ServletException, IOException {
+ response.setContentType("image/jpeg");
+ response.addHeader("Cache-Control", "no-store");
+
+ response.getWriter().print("Hello world");
+ }
+ };
+
+ validate(servlet, null);
+ }
+
@Test
public void testUseMajorTypeExpiresConfiguration() throws Exception {
HttpServlet servlet = new HttpServlet() {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8503206349..cc8494d6d5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -223,6 +223,12 @@
file exists and some as set as if it does not. This resulted in
inconsistent metadata. (markt)
</fix>
+ <fix>
+ <bug>69415</bug>: Ensure that the <code>ExpiresFilter</code> only sets
+ cache headers on <code>GET</code> and <code>HEAD</code> requests. Also
+ skip requests where the application has set <code>Cache-Control:
+ no-store</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
