From: Jim Jagielski Date: Wed, 10 Oct 2018 15:40:35 +0000 (+0000) Subject: Merge r1843242 from trunk: X-Git-Tag: 2.4.36~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a6f522c01092c0786a2b827c3ece9192b366d05;p=thirdparty%2Fapache%2Fhttpd.git 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 b145fee99e4..99f80843423 100644 --- 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 e58751b0b8b..59bf41d777f 100644 --- 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 024f6b760ea..56717e7b8d4 100644 --- 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 d218bab83cc..d60b2de6f43 100644 --- 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) ) {