http: Enforce consistently no response body with both 204 and 304 statuses.
authorYann Ylavic <ylavic@apache.org>
Mon, 30 Jul 2018 13:08:23 +0000 (13:08 +0000)
committerYann Ylavic <ylavic@apache.org>
Mon, 30 Jul 2018 13:08:23 +0000 (13:08 +0000)
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

CHANGES
include/httpd.h
modules/filters/mod_brotli.c
modules/filters/mod_deflate.c
modules/http/http_filters.c
modules/http/http_protocol.c
modules/http2/h2_from_h1.c
modules/proxy/mod_proxy_http.c
modules/test/mod_policy.c
server/protocol.c

diff --git a/CHANGES b/CHANGES
index 952e220839ae6c799b724852237582727b0d1d57..5643a51b75b1e0770a569a8bd29ff06b1e603c88 100644 (file)
--- 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]
 
index 45ad9765b1877dbd3d86732275690bab69fb3d27..ff620aefeca1c12e5e66ce466d4c4cd77591a3fd 100644 (file)
@@ -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)
 /** @} */
 
 /**
index dd91f6e703c479580c984c31069699be54020c41..024f6b760ea172b5f59f399c62c4b6657dc8182f 100644 (file)
@@ -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);
index 3ee78c7d0b08ead66add96d9d7fafcd2e9a2512e..ab586d27c976ff964b5db27faf73cca645ec715a 100644 (file)
@@ -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)
            ) {
index 2887136caf75c1b33b4ea7f97624dfe85e6e267e..0610154d681b9b68023d564310da1882d8a5e9a2 100644 (file)
@@ -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;
     }
index 97182dc3d740b085e49e32b8bd12b73fc20efe74..928596c29bd930211ff889b7b937922a8f261207 100644 (file)
@@ -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) {
index dd6ad90302e0bfe513e0fe5b934ceaa49c693a33..d69c53c21be51d1da262670e44b522bd5195877b 100644 (file)
@@ -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);
index f7eaab3b9dcbb8525c38f58105b1bb25a119ceda..6299e332be666e3d7afce6ae2de68b186b5e64fd 100644 (file)
@@ -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
index 22184793e17dc59b0615f8458f242a28a6170399..ded77c761da92f740c31ef6302393659af90f552 100644 (file)
@@ -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")
index e1c9eac6441f3cdfd847663ef883f5c69ee11131..f98707472de1e48e07bc46780220fd1048e26e5b 100644 (file)
@@ -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