On Wed, Sep 18, 2013 at 3:36 PM, Yann Ylavic <[email protected]> wrote:
> On Wed, Sep 18, 2013 at 3:02 PM, Jim Jagielski <[email protected]> wrote: > >> But why also drop the (!lenp) check? >> >> Should it be else >> >> >> >> >> >> if ((f->r->proxyreq != PROXYREQ_RESPONSE) || (!lenp)) { >> > > I don't see in the draft where the Content-Length has to be taken into > account whenever a Transfer-Encoding is specified. > If the latter is "valid", the former MUST be ignored/stripped : > > If a message is received with both a Transfer-Encoding and a > Content-Length header field, the Transfer-Encoding overrides the > Content-Length. Such a message might indicate an attempt to > perform request or response smuggling (bypass of security-related > checks on message routing or content) and thus ought to be > handled as an error. A sender MUST remove the received Content- > Length field prior to forwarding such a message downstream. > > > So I don't see the point for the Content-Length, if it is a request > whether or not a Content-Length is given a 400 is replied, if it is a > response the Content-Length must be ignored (and stripped, what the patch > does not, but I'm not sure if the Content-Length of the fake request_rec > used here by mod_proxy is forwarded to the client, I'll check that!). > However, if the case "ought to be handled as an error", it should rather be (in ap_http_filter) : if ((f->r->proxyreq != PROXYREQ_RESPONSE) || lenp) { and (in ap_read_request) : if ((strcasecmp(tenc, "chunked") != 0 // fast path && !ap_find_last_token(r->pool, tenc, "chunked")) || apr_table_get(r->headers_in, "Content-Length")) {
