Re: [PATCH v5 1/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2017, 03:46 UTC
Message-ID: <xmqqo9npitx7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171126015448.24111-2-max@max630.net>

Max Kirillov <max@max630.net> writes:

> Author: Florian Manschwetus <manschwetus@cs-software-gmbh.de>

This should read "From: ...";

Show 17 quoted lines

> Date: Wed, 30 Mar 2016 09:08:56 +0000
>
> http-backend reads whole input until EOF. However, the RFC 3875 specifies
> that a script must read only as many bytes as specified by CONTENT_LENGTH
> environment variable. Web server may exercise the specification by not closing
> the script's standard input after writing content. In that case http-backend
> would hang waiting for the input. The issue is known to happen with
> IIS/Windows, for example.
>
> Make http-backend read only CONTENT_LENGTH bytes, if it's defined, rather than
> the whole input until EOF. If the variable is not defined, keep older behavior
> of reading until EOF because it is used to support chunked transfer-encoding.
>
> Signed-off-by: Florian Manschwetus <manschwetus@cs-software-gmbh.de>
> [mk: fixed trivial build failures and polished style issues]
>
> Signed-off-by: Max Kirillov <max@max630.net>

There shouldn't be a blank line before your sign-off.

The above are only for future reference; I can fix them up before queuing if there isn't any other issues in this patch.

Show 8 quoted lines

> +ssize_t git_env_ssize_t(const char *k, ssize_t val)
> +{
> +	const char *v = getenv(k);
> +	if (v && !git_parse_ssize_t(v, &val))
> +		die("failed to parse %s", k);
> +	return val;
> +}
> +

If this were passing "v" that is a string the caller obtains from any source (and the callee does not care about where it came from), that would be a different story, but as a public interface that is part of the config.c layer, "k" that has to be the name of the environment variable sticks out like a sore thumb.

If we were to add one more public function to the interface, I suspect that exposing the existing git_parse_ssize_t() and have the caller implement the above function for its use would be a much better way to go.