svn commit: r1878367 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

[yl...@apache.org]

Author: ylavic
Date: Mon Jun  1 15:40:03 2020
New Revision: 1878367

URL: http://svn.apache.org/viewvc?rev=1878367&view=rev
Log:
mod_proxy_http: put request/response splitting checks under the same comment.

If stream_reqbody() detects a Content-Length vs bytes streamed mismatch it
means that a module's filter played bad games, such inconsistency on the client
connection would have been caught by ap_http_filter().

So move AH01087 logic to AH01086, under the same comment and log message, and
always return HTTP_INTERNAL_SERVER because the client is not the culprit here.

Modified:
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1878367&r1=1878366&r2=1878367&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Jun  1 15:40:03 2020
@@ -364,20 +364,26 @@ static int stream_reqbody(proxy_http_req
                 }
             }
             else if (rb_method == RB_STREAM_CL
-                     && bytes_streamed > req->cl_val) {
-                /* C-L < bytes streamed?!?
-                 * We will error out after the body is completely
-                 * consumed, but we can't stream more bytes at the
-                 * back end since they would in part be interpreted
-                 * as another request!  If nothing is sent, then
-                 * just send nothing.
+                     && (bytes_streamed > req->cl_val
+                         || (seen_eos && bytes_streamed < req->cl_val))) {
+                /* C-L != bytes streamed?!?
                  *
-                 * Prevents HTTP Response Splitting.
+                 * Prevent HTTP Request/Response Splitting.
+                 *
+                 * We can't stream more (or less) bytes at the back end since
+                 * they could be interpreted in separate requests (more bytes
+                 * now would start a new request, less bytes would make the
+                 * first bytes of the next request be part of the current one).
+                 *
+                 * It can't happen from the client connection here thanks to
+                 * ap_http_filter(), but some module's filter may be playing
+                 * bad games, hence the HTTP_INTERNAL_SERVER_ERROR.
                  */
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01086)
-                              "read more bytes of request body than expected "
+                              "read %s bytes of request body than expected "
                               "(got %" APR_OFF_T_FMT ", expected "
                               "%" APR_OFF_T_FMT ")",
+                              bytes_streamed > req->cl_val ? "more" : "less",
                               bytes_streamed, req->cl_val);
                 return HTTP_INTERNAL_SERVER_ERROR;
             }
@@ -403,13 +409,6 @@ static int stream_reqbody(proxy_http_req
         }
     } while (!seen_eos);
 
-    if (rb_method == RB_STREAM_CL && bytes_streamed != req->cl_val) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01087)
-                      "client %s given Content-Length did not match"
-                      " number of body bytes read", r->connection->client_ip);
-        return HTTP_BAD_REQUEST;
-    }
-
     return OK;
 }