Re: [PATCH] http-backend: Treat empty CONTENT_LENGTH as zero

[Jeff King <peff@peff.net>]

On Mon, Sep 10, 2018 at 07:20:28PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> > On Mon, Sep 10, 2018 at 02:22:21PM -0700, Jonathan Nieder wrote:
> 
> >> Thanks.  I am wondering if we should go all the way and do
> >>
> >> 	ssize_t val;
> >> 	const char *str = getenv("CONTENT_LENGTH");
> >>
> >> 	if (!str || !*str)
> >> 		return 0;
> >> 	if (!git_parse_ssize_t(str, &val))
> >> 		die(...);
> >> 	return val;
> >>
> >> That would match the RFC, but it seems to make t5510-fetch.sh hang,
> [...]
> >> Do you know why?
> >
> > Yes. :)
> >
> > It's due to this comment in the patch you are replying to:
> >
> > +       if (!str) {
> > +               /*
> > +                * RFC3875 says this must mean "no body", but in practice we
> > +                * receive chunked encodings with no CONTENT_LENGTH. Tell the
> > +                * caller to read until EOF.
> > +                */
> > +               val = -1;
> 
> Ah!  So "in practice" includes "in Apache".  An old discussion[1] on
> Apache's httpd-users list agrees.
> 
> The question then becomes: what does IIS do for zero-length requests?
> Does any other web server fail to support "read until EOF" in general?
> 
> The CGI standard does not cover chunked encoding so we can't lean on
> the standard for advice.  It's not clear to me yet whether this patch
> improves on what's in "master".

I'd note that the case in question (no CONTENT_LENGTH at all) is not
changed between this patch and master. It's only the case of
CONTENT_LENGTH set to an empty string. But I agree that it is not clear
to me whether it is actually improving anything in practice.

-Peff