Fixed chunked request forwarding in ICAP presence. ICAP prohibits forwarding of hop-by-hop headers in HTTP headers. If the virgin request has a "Transfer-Encoding: chunked" header, the ICAP server will not receive it. Thus, when the ICAP server responds with a 200 OK and what it thinks is a copy of the HTTP request, the adapted request will be missing the Transfer-Encoding header. One the server side, Squid used to test whether the request had a Transfer-Encoding header to determine whether request chunking is needed when talking to the next HTTP hop. That test would fail in ICAP presence. This change implements a more direct/robust check: if we do not know the request content length, we chunk the request. We also no longer forward the Content-Length header if we are chunking. It should not really be there in most cases, but an explicit check is safer and may also prevent request smuggling attacks via Connection: Content-Length tricks. === modified file 'src/http.cc' --- src/http.cc 2011-04-05 21:08:43 +0000 +++ src/http.cc 2011-04-05 22:24:15 +0000 @@ -1949,40 +1949,47 @@ if (!Config.onoff.via) hdr_out->addEntry(e->clone()); break; case HDR_RANGE: case HDR_IF_RANGE: case HDR_REQUEST_RANGE: /** \par Range:, If-Range:, Request-Range: * Only pass if we accept ranges */ if (!we_do_ranges) hdr_out->addEntry(e->clone()); break; case HDR_PROXY_CONNECTION: // SHOULD ignore. But doing so breaks things. break; + case HDR_CONTENT_LENGTH: + // pass through unless we chunk; also, keeping this away from default + // prevents request smuggling via Connection: Content-Length tricks + if (!flags.chunked_request) + hdr_out->addEntry(e->clone()); + break; + case HDR_X_FORWARDED_FOR: case HDR_CACHE_CONTROL: /** \par X-Forwarded-For:, Cache-Control: * handled specially by Squid, so leave off for now. * append these after the loop if needed */ break; case HDR_FRONT_END_HTTPS: /** \par Front-End-Https: * Pass thru only if peer is configured with front-end-https */ if (!flags.front_end_https) hdr_out->addEntry(e->clone()); break; default: /** \par default. * pass on all other header fields * which are NOT listed by the special Connection: header. */ @@ -2071,42 +2078,42 @@ debugs(11,3, HERE << "cannot send request to closing FD " << fd); assert(closeHandler != NULL); return false; } typedef CommCbMemFunT TimeoutDialer; AsyncCall::Pointer timeoutCall = JobCallback(11, 5, TimeoutDialer, this, HttpStateData::httpTimeout); commSetTimeout(fd, Config.Timeout.lifetime, timeoutCall); flags.do_next_read = 1; maybeReadVirginBody(); if (orig_request->body_pipe != NULL) { if (!startRequestBodyFlow()) // register to receive body data return false; typedef CommCbMemFunT 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) + // use chunked encoding if we do not know the length + if (orig_request->content_length < 0) flags.chunked_request = 1; } else { assert(!requestBodySource); typedef CommCbMemFunT Dialer; requestSender = JobCallback(11,5, Dialer, this, HttpStateData::wroteLast); } if (_peer != NULL) { if (_peer->options.originserver) { flags.proxying = 0; flags.originpeer = 1; } else { flags.proxying = 1; flags.originpeer = 0; } } else { flags.proxying = 0; flags.originpeer = 0; }