Do not send chunked requests without a "Transfer-Encoding: chunked" header

or with a "Content-Length: 0" header.

Whether we are sending a chunked request depends not just on whether the
received request was chunked (condition A) but also on whether we still do not
know the request body size (condition B). The old code added the
"Transfer-Encoding: chunked" header if (A && B) but chunked the request body
if (A). This resulted in malformed requests with chunked request bodies but
without the "Transfer-Encoding: chunked" header.

When adding the Transfer-Encoding field, the old code also considered zero
Content-Length as "unknown", which was, apparently wrong. This resulted in the
"Content-Length: 0" header sent with a chunked encoded [empty] body, violating
HTTP rules. I am not 100% sure we never use zero request->content_length value
to mark "unknown" length though, so this may need more work.

based on lp 3p2-plus branch, r10827.

diff --git a/src/http.cc b/src/http.cc
index 5923c71c275a76f3533791e97e7ec30e43b34c18..597ad6c4f61cfc9cb0e91d240586632393a48277 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1814,11 +1814,9 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
             hdr_out->putStr(HDR_FRONT_END_HTTPS, "On");
     }
 
-    if (orig_request->header.chunked() && orig_request->content_length <= 0) {
-        /* Preserve original chunked encoding unless we learned the length.
-         * Do not just copy the original value so that if the client-side
-         * starts decode other encodings, this code may remain valid.
-         */
+    if (flags.chunked_request) {
+        // Do not just copy the original value so that if the client-side
+        // starts decode other encodings, this code may remain valid.
         hdr_out->putStr(HDR_TRANSFER_ENCODING, "chunked");
     }
 
@@ -2081,6 +2079,11 @@ HttpStateData::sendRequest()
         typedef CommCbMemFunT<HttpStateData, CommIoCbParams> Dialer;
         requestSender = JobCallback(11,5,
                                     Dialer, this, HttpStateData::sentRequestBody);
+
+        Must(!flags.chunked_request);
+        // Preserve original chunked encoding unless we learned the length.
+        if (orig_request->header.chunked() && orig_request->content_length < 0)
+            flags.chunked_request = 1;
     } else {
         assert(!requestBodySource);
         typedef CommCbMemFunT<HttpStateData, CommIoCbParams> Dialer;
@@ -2137,7 +2140,7 @@ bool
 HttpStateData::getMoreRequestBody(MemBuf &buf)
 {
     // parent's implementation can handle the no-encoding case
-    if (!request->header.chunked())
+    if (!flags.chunked_request)
         return ServerStateData::getMoreRequestBody(buf);
 
     MemBuf raw;
@@ -2248,10 +2251,9 @@ HttpStateData::doneSendingRequestBody()
     debugs(11,5, HERE << "doneSendingRequestBody: FD " << fd);
 
     // do we need to write something after the last body byte?
-    const bool chunked = request->header.chunked();
-    if (chunked && finishingChunkedRequest())
+    if (flags.chunked_request && finishingChunkedRequest())
         return;
-    if (!chunked && finishingBrokenPost())
+    if (!flags.chunked_request && finishingBrokenPost())
         return;
 
     sendComplete();

diff --git a/src/structs.h b/src/structs.h
index 5298668c30f1a0a9e0bb30cbbf7ac9f4b0da5234..2e83590908b6103634e20d70a187bd31e6bd429c 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -768,7 +768,8 @@ struct _http_state_flags {
     unsigned int request_sent:1;
     unsigned int do_next_read:1;
     unsigned int consume_body_data:1;
-    unsigned int chunked:1;
+    unsigned int chunked:1; ///< reading a chunked response; TODO: rename
+    unsigned int chunked_request:1; ///< writing a chunked request
     unsigned int sentLastChunk:1; ///< do not try to write last-chunk again
 };