Merge r1843242 from trunk:

mod_brotli, mod_deflate: Restore the separate handling of 304 Not Modified
responses allowing these modules to properly set or fix-up the response
headers such as Vary or ETag.

This change follows up on r1837056 that disabled that special handling and
thus resulted in a potential violation of RFC7232, 4.1:

   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.)

References:
  https://lists.apache.org/thread.html/f5733ca6743757e8aa8b58a0cd9e27680971551c2a20f5606c66507e@%3Cdev.httpd.apache.org%3E
  https://tools.ietf.org/html/rfc7232#section-4.1
Submitted by: kotkov
Reviewed by: kotkov, ylavic, jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1843469 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index b145fee99e41215d2f606c480c2444a1909b48d9..99f80843423ae2942afb1b54927c5aa41a1d7ed3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.36
 
+  *) mod_brotli, mod_deflate: Restore the separate handling of 304 Not Modified
+     responses. Regression introduced in 2.4.35.
+
   *) mod_proxy_scgi, mod_proxy_uwsgi: improve error handling when sending the
      body of the response. [Jim Jagielski]
 

diff --git a/STATUS b/STATUS
index e58751b0b8b24c60b97ffade79a7aafec9b9cf86..59bf41d777f2d97583481fc9a2696a5af4ec25be 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -124,17 +124,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_brotli, mod_deflate: Restore the separate handling of 304 Not Modified
-     responses allowing these modules to properly set or fix-up the response
-     headers such as Vary or ETag. Fixes a potential violation of RFC7232, 4.1.
-     Regression introduced in 2.4.35.
-     trunk patch: https://svn.apache.org/r1843242
-     2.4.x patch: svn merge -c 1843242 ^/httpd/httpd/trunk .
-     Note: The related discussion is in
-       https://lists.apache.org/thread.html/f5733ca6743757e8aa8b58a0cd9e27680971551c2a20f5606c66507e@%3Cdev.httpd.apache.org%3E
-     Note: Presumably, this needs a separate CHANGES entry that is not included
-     in the 2.4.x patch.
-     +1: kotkov, ylavic, jim
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/filters/mod_brotli.c b/modules/filters/mod_brotli.c
index 024f6b760ea172b5f59f399c62c4b6657dc8182f..56717e7b8d4ba8ba5f5c6d8b7f311de9e16d837f 100644 (file)
--- a/modules/filters/mod_brotli.c
+++ b/modules/filters/mod_brotli.c
@@ -346,12 +346,14 @@ static apr_status_t compress_filter(ap_filter_t *f, apr_bucket_brigade *bb)
         const char *accepts;
 
         /* Only work on main request, not subrequests, that are not
-         * responses with no content (204/304), and are not tagged with the
+         * a 204 response with no content, and are not tagged with the
          * no-brotli env variable, and are not a partial response to
          * a Range request.
+         *
+         * Note that responding to 304 is handled separately to set
+         * the required headers (such as ETag) per RFC7232, 4.1.
          */
-        if (r->main
-            || AP_STATUS_IS_HEADER_ONLY(r->status)
+        if (r->main || r->status == HTTP_NO_CONTENT
             || apr_table_get(r->subprocess_env, "no-brotli")
             || apr_table_get(r->headers_out, "Content-Range")) {
             ap_remove_output_filter(f);

diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c
index d218bab83cce213892cf84a99410abd74a92b481..d60b2de6f436247bcd15f8a0b54e10290aa1e82f 100644 (file)
--- a/modules/filters/mod_deflate.c
+++ b/modules/filters/mod_deflate.c
@@ -602,19 +602,21 @@ static apr_status_t deflate_out_filter(ap_filter_t *f,
 
         /*
          * Only work on main request, not subrequests,
-         * that are not responses with no content (204/304),
+         * that are not a 204 response with no content
          * and are not tagged with the no-gzip env variable
          * and not a partial response to a Range request.
+         *
+         * Note that responding to 304 is handled separately to
+         * set the required headers (such as ETag) per RFC7232, 4.1.
          */
-        if ((r->main != NULL) ||
-            AP_STATUS_IS_HEADER_ONLY(r->status) ||
+        if ((r->main != NULL) || (r->status == HTTP_NO_CONTENT) ||
             apr_table_get(r->subprocess_env, "no-gzip") ||
             apr_table_get(r->headers_out, "Content-Range")
            ) {
             if (APLOG_R_IS_LEVEL(r, APLOG_TRACE1)) {
                 const char *reason =
                     (r->main != NULL)                           ? "subrequest" :
-                    AP_STATUS_IS_HEADER_ONLY(r->status)         ? "no content" :
+                    (r->status == HTTP_NO_CONTENT)              ? "no content" :
                     apr_table_get(r->subprocess_env, "no-gzip") ? "no-gzip" :
                     "content-range";
                 ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
@@ -1525,12 +1527,14 @@ static apr_status_t inflate_out_filter(ap_filter_t *f,
 
         /*
          * Only work on main request, not subrequests,
-         * that are not responses with no content (204/304),
+         * that are not a 204 response with no content
          * and not a partial response to a Range request,
          * and only when Content-Encoding ends in gzip.
+         *
+         * Note that responding to 304 is handled separately to
+         * set the required headers (such as ETag) per RFC7232, 4.1.
          */
-        if (!ap_is_initial_req(r) ||
-            AP_STATUS_IS_HEADER_ONLY(r->status) ||
+        if (!ap_is_initial_req(r) || (r->status == HTTP_NO_CONTENT) ||
             (apr_table_get(r->headers_out, "Content-Range") != NULL) ||
             (check_gzip(r, r->headers_out, r->err_headers_out) == 0)
            ) {