Re: [PATCH] http-backend: treat empty CONTENT_LENGTH as zero
- Date: Mon, 10 Sep 2018 21:03:43 -0700
- From: Jonathan Nieder <jrnieder@xxxxxxxxx>
- Subject: Re: [PATCH] http-backend: treat empty CONTENT_LENGTH as zero
Kicking off the reviews: ;-)
Jonathan Nieder wrote:
> --- a/http-backend.c
> +++ b/http-backend.c
> @@ -350,10 +350,25 @@ static ssize_t read_request_fixed_len(int fd, ssize_t req_len, unsigned char **o
> static ssize_t get_content_length(void)
> + /*
> + * According to RFC 3875, an empty or missing
> + * CONTENT_LENGTH means "no body", but RFC 3875
> + * precedes HTTP/1.1 and chunked encoding. Apache and
> + * its imitators leave CONTENT_LENGTH unset for
Which imitators? Maybe this should just say "Apache leaves [...]".
> + * chunked requests, for which we should use EOF to
> + * detect the end of the request.
> + */
> + str = getenv("HTTP_TRANSFER_ENCODING");
> + if (str && !strcmp(str, "chunked"))
RFC 2616 says Transfer-Encoding is a list of transfer-codings applied,
in the order that they were applied, and that "chunked" is always
applied last. That means a transfer-encoding like
Transfer-Encoding: identity chunked
would be permitted, or e.g.
Transfer-Encoding: gzip chunked
Does that means we should be using a check like
str && (!strcmp(str, "chunked") || ends_with(str, " chunked"))
That said, a quick search of codesearch.debian.net mostly finds
examples using straight comparison, so maybe the patch is fine as-is.