From: Yann Ylavic Date: Mon, 30 Jul 2018 13:08:23 +0000 (+0000) Subject: http: Enforce consistently no response body with both 204 and 304 statuses. X-Git-Tag: 2.5.0-alpha2-ci-test-only~2428 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f673148e9b8674aa6d5bd1efdd12679fc0099c79;p=thirdparty%2Fapache%2Fhttpd.git http: Enforce consistently no response body with both 204 and 304 statuses. Provide AP_STATUS_IS_HEADER_ONLY() helper/macro to check for 204 or 304 and use it where some special treatment is needed when no body is expected. Some of those places handled 204 only. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1837056 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 952e220839a..5643a51b75b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.1 + *) http: Enforce consistently no response body with both 204 and 304 + statuses. [Yann Ylavic] + *) mod_proxy_http: forward 100-continue, and minimize race conditions when reusing backend connections. PR 60330. [Yann Ylavic, Jean-Frederic Clere] diff --git a/include/httpd.h b/include/httpd.h index 45ad9765b18..ff620aefeca 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -570,6 +570,10 @@ AP_DECLARE(const char *) ap_get_server_built(void); ((x) == HTTP_INTERNAL_SERVER_ERROR) || \ ((x) == HTTP_SERVICE_UNAVAILABLE) || \ ((x) == HTTP_NOT_IMPLEMENTED)) + +/** does the status imply header only response (i.e. never w/ a body)? */ +#define AP_STATUS_IS_HEADER_ONLY(x) ((x) == HTTP_NO_CONTENT || \ + (x) == HTTP_NOT_MODIFIED) /** @} */ /** diff --git a/modules/filters/mod_brotli.c b/modules/filters/mod_brotli.c index dd91f6e703c..024f6b760ea 100644 --- a/modules/filters/mod_brotli.c +++ b/modules/filters/mod_brotli.c @@ -346,11 +346,12 @@ 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 - * a 204 response with no content, and are not tagged with the + * responses with no content (204/304), and are not tagged with the * no-brotli env variable, and are not a partial response to * a Range request. */ - if (r->main || r->status == HTTP_NO_CONTENT + if (r->main + || AP_STATUS_IS_HEADER_ONLY(r->status) || 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 3ee78c7d0b0..ab586d27c97 100644 --- a/modules/filters/mod_deflate.c +++ b/modules/filters/mod_deflate.c @@ -642,18 +642,19 @@ static apr_status_t deflate_out_filter(ap_filter_t *f, /* * Only work on main request, not subrequests, - * that are not a 204 response with no content + * that are not responses with no content (204/304), * and are not tagged with the no-gzip env variable * and not a partial response to a Range request. */ - if ((r->main != NULL) || (r->status == HTTP_NO_CONTENT) || + if ((r->main != NULL) || + AP_STATUS_IS_HEADER_ONLY(r->status) || 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" : - (r->status == HTTP_NO_CONTENT) ? "no content" : + AP_STATUS_IS_HEADER_ONLY(r->status) ? "no content" : apr_table_get(r->subprocess_env, "no-gzip") ? "no-gzip" : "content-range"; ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, @@ -1532,11 +1533,12 @@ static apr_status_t inflate_out_filter(ap_filter_t *f, /* * Only work on main request, not subrequests, - * that are not a 204 response with no content + * that are not responses with no content (204/304), * and not a partial response to a Range request, * and only when Content-Encoding ends in gzip. */ - if (!ap_is_initial_req(r) || (r->status == HTTP_NO_CONTENT) || + if (!ap_is_initial_req(r) || + AP_STATUS_IS_HEADER_ONLY(r->status) || (apr_table_get(r->headers_out, "Content-Range") != NULL) || (check_gzip(r, r->headers_out, r->err_headers_out) == 0) ) { diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index 2887136caf7..0610154d681 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -1251,7 +1251,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f, } else if (ctx->headers_sent) { /* Eat body if response must not have one. */ - if (r->header_only || r->status == HTTP_NO_CONTENT) { + if (r->header_only || AP_STATUS_IS_HEADER_ONLY(r->status)) { apr_brigade_cleanup(b); return APR_SUCCESS; } @@ -1356,12 +1356,15 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f, basic_http_header_check(r, &protocol); ap_set_keepalive(r); - if (r->chunked) { - apr_table_mergen(r->headers_out, "Transfer-Encoding", "chunked"); + if (AP_STATUS_IS_HEADER_ONLY(r->status)) { + apr_table_unset(r->headers_out, "Transfer-Encoding"); apr_table_unset(r->headers_out, "Content-Length"); + r->content_type = r->content_encoding = NULL; + r->content_languages = NULL; + r->clength = r->chunked = 0; } - - if (r->status == HTTP_NO_CONTENT) { + else if (r->chunked) { + apr_table_mergen(r->headers_out, "Transfer-Encoding", "chunked"); apr_table_unset(r->headers_out, "Content-Length"); } @@ -1440,7 +1443,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f, } ctx->headers_sent = 1; - if (r->header_only || r->status == HTTP_NO_CONTENT) { + if (r->header_only || AP_STATUS_IS_HEADER_ONLY(r->status)) { apr_brigade_cleanup(b); goto out; } diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c index 97182dc3d74..928596c29bd 100644 --- a/modules/http/http_protocol.c +++ b/modules/http/http_protocol.c @@ -254,9 +254,8 @@ AP_DECLARE(int) ap_set_keepalive(request_rec *r) */ if ((r->connection->keepalive != AP_CONN_CLOSE) && !r->expecting_100 - && ((r->status == HTTP_NOT_MODIFIED) - || (r->status == HTTP_NO_CONTENT) - || r->header_only + && (r->header_only + || AP_STATUS_IS_HEADER_ONLY(r->status) || apr_table_get(r->headers_out, "Content-Length") || ap_find_last_token(r->pool, apr_table_get(r->headers_out, @@ -1239,26 +1238,22 @@ AP_DECLARE(void) ap_send_error_response(request_rec *r, int recursive_error) ap_run_insert_error_filter(r); - /* - * It's possible that the Location field might be in r->err_headers_out - * instead of r->headers_out; use the latter if possible, else the - * former. - */ - if (location == NULL) { - location = apr_table_get(r->err_headers_out, "Location"); - } /* We need to special-case the handling of 204 and 304 responses, * since they have specific HTTP requirements and do not include a * message body. Note that being assbackwards here is not an option. */ - if (status == HTTP_NOT_MODIFIED) { + if (AP_STATUS_IS_HEADER_ONLY(status)) { ap_finalize_request_protocol(r); return; } - if (status == HTTP_NO_CONTENT) { - ap_finalize_request_protocol(r); - return; + /* + * It's possible that the Location field might be in r->err_headers_out + * instead of r->headers_out; use the latter if possible, else the + * former. + */ + if (location == NULL) { + location = apr_table_get(r->err_headers_out, "Location"); } if (!r->assbackwards) { diff --git a/modules/http2/h2_from_h1.c b/modules/http2/h2_from_h1.c index dd6ad90302e..d69c53c21be 100644 --- a/modules/http2/h2_from_h1.c +++ b/modules/http2/h2_from_h1.c @@ -209,10 +209,18 @@ static h2_headers *create_response(h2_task *task, request_rec *r) /* determine the protocol and whether we should use keepalives. */ ap_set_keepalive(r); - if (r->chunked) { + if (AP_STATUS_IS_HEADER_ONLY(r->status)) { + apr_table_unset(r->headers_out, "Transfer-Encoding"); apr_table_unset(r->headers_out, "Content-Length"); + r->content_type = r->content_encoding = NULL; + r->content_languages = NULL; + r->clength = r->chunked = 0; } - + else if (r->chunked) { + apr_table_mergen(r->headers_out, "Transfer-Encoding", "chunked"); + apr_table_unset(r->headers_out, "Content-Length"); + } + ctype = ap_make_content_type(r, r->content_type); if (ctype) { apr_table_setn(r->headers_out, "Content-Type", ctype); diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index f7eaab3b9dc..6299e332be6 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1808,10 +1808,7 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) } /* send body - but only if a body is expected */ - if ((!r->header_only) && /* not HEAD request */ - (proxy_status != HTTP_NO_CONTENT) && /* not 204 */ - (proxy_status != HTTP_NOT_MODIFIED)) { /* not 304 */ - + if (!r->header_only && !AP_STATUS_IS_HEADER_ONLY(proxy_status)) { /* We need to copy the output headers and treat them as input * headers as well. BUT, we need to do this before we remove * TE, so that they are preserved accordingly for diff --git a/modules/test/mod_policy.c b/modules/test/mod_policy.c index 22184793e17..ded77c761da 100644 --- a/modules/test/mod_policy.c +++ b/modules/test/mod_policy.c @@ -308,9 +308,8 @@ static apr_status_t policy_keepalive_out_filter(ap_filter_t *f, if (result != policy_ignore && r->connection->keepalive != AP_CONN_CLOSE && !r->expecting_100 && !ap_status_drops_connection(r->status)) { - if (!((r->status == HTTP_NOT_MODIFIED) - || (r->status == HTTP_NO_CONTENT) - || r->header_only + if (!(r->header_only + || AP_STATUS_IS_HEADER_ONLY(r->status) || apr_table_get(r->headers_out, "Content-Length") || ap_find_last_token(r->pool, apr_table_get(r->headers_out, "Transfer-Encoding"), "chunked") diff --git a/server/protocol.c b/server/protocol.c index e1c9eac6441..f98707472de 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -1930,7 +1930,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_length_filter( * such filters update or remove the C-L header, and just use it * if present. */ - if (!(r->header_only + if (!((r->header_only || AP_STATUS_IS_HEADER_ONLY(r->status)) && !r->bytes_sent && (r->sent_bodyct || conf->http_cl_head_zero != AP_HTTP_CL_HEAD_ZERO_ENABLE