Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Max Kirillov <max@max630.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Max Kirillov" <max@max630.net>,
	"Jelmer Vernooij" <jelmer@jelmer.uk>,
	git@vger.kernel.org, "Jeff King" <peff@peff.net>
Subject: [PATCH v3] http-backend: allow empty CONTENT_LENGTH
Date: Sat,  8 Sep 2018 08:42:24 +0300
Message-ID: <20180908054224.21856-1-max@max630.net> (raw)
In-Reply-To: <20180908001940.GB225427@aiede.svl.corp.google.com>

Before 817f7dc223, CONTENT_LENGTH variable was never considered,
http-backend was just reading request body from standard input until EOF
when it, or a command started by it, needed it.

Then it was discovered that some HTTP do not close standard input, instead
expecting CGI scripts to obey CONTENT_LENGTH. In 817f7dc223, behavior
was changed to consider the CONTENT_LENGTH variable when it is set. Case
of unset CONTENT_LENGTH was kept to mean "read until EOF" which is not
compliant to the RFC3875 (which treats it as empty body), but
practically is used when client uses chunked encoding to submit big
request.

Case of empty CONTENT_LENGTH has slept through this conditions.
Apparently, it is used for GET requests, and RFC3875 does specify that
it also means empty body. Current implementation, however, fails to
parse it and aborts the request.

Fix the case of empty CONTENT_LENGTH to also be treated as "read until EOF".
It does not actually matter what does it mean because body is never read
anyway, it just should not cause parse error. Add a test for the case.

Reported-By: Jelmer Vernooij <jelmer@jelmer.uk>
Signed-off-by: Max Kirillov <max@max630.net>
---
Provided more thorough message, also fix test (it did not test actually the error before)

There will be more versions later, at least the one which suggested by Jeff

PS: did I write v2, it should be v3 of course!
 http-backend.c                         |  2 +-
 t/t5562-http-backend-content-length.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/http-backend.c b/http-backend.c
index e88d29f62b..a1230d7ead 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -353,7 +353,7 @@ static ssize_t get_content_length(void)
 	ssize_t val = -1;
 	const char *str = getenv("CONTENT_LENGTH");
 
-	if (str && !git_parse_ssize_t(str, &val))
+	if (str && *str && !git_parse_ssize_t(str, &val))
 		die("failed to parse CONTENT_LENGTH: %s", str);
 	return val;
 }
diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
index 057dcb85d6..b28c3c4765 100755
--- a/t/t5562-http-backend-content-length.sh
+++ b/t/t5562-http-backend-content-length.sh
@@ -152,4 +152,15 @@ test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
 	grep "fatal:.*CONTENT_LENGTH" err
 '
 
+test_expect_success 'empty CONTENT_LENGTH' '
+	env \
+		QUERY_STRING="/repo.git/info/refs?service=git-receive-pack" \
+		PATH_TRANSLATED="$PWD"/.git/info/refs \
+		GIT_HTTP_EXPORT_ALL=TRUE \
+		REQUEST_METHOD=GET \
+		CONTENT_LENGTH="" \
+		git http-backend <empty_body >act.out 2>act.err &&
+	verify_http_result "200 OK"
+'
+
 test_done
-- 
2.17.0.1185.g782057d875


  parent reply index

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-08  0:19 [PATCH v2] " Jonathan Nieder
2018-09-08  5:35 ` Max Kirillov
2018-09-08  5:42 ` Max Kirillov [this message]
2018-09-10  5:17   ` [PATCH v3] " Jonathan Nieder
2018-09-10 20:36     ` Max Kirillov
2018-09-11  4:06       ` Jonathan Nieder
2018-09-11 20:33         ` [PATCH v2] http-backend test: make empty CONTENT_LENGTH test more realistic Max Kirillov
  -- strict thread matches above, loose matches on Subject: below --
2018-09-06  6:10 CONTENT_LENGTH can no longer be empty Jonathan Nieder
2018-09-06 19:35 ` [PATCH] http-backend: allow empty CONTENT_LENGTH Max Kirillov
2018-09-06 21:54   ` Junio C Hamano
2018-09-07  3:27     ` Max Kirillov
2018-09-07  3:38       ` Jeff King
2018-09-07  4:20         ` Max Kirillov
2018-09-07  4:59         ` Max Kirillov
2018-09-07  9:49           ` Junio C Hamano
2018-09-08  5:41             ` Max Kirillov
2018-09-09  4:40             ` Max Kirillov
2018-09-06 22:45   ` Jonathan Nieder
2018-09-07  3:36   ` [PATCH v2] " Max Kirillov
2018-09-09  4:10     ` [PATCH v4] " Max Kirillov
2018-09-10  5:25       ` Jonathan Nieder
2018-09-10 13:17         ` Jeff King
2018-09-10 16:37           ` Junio C Hamano
2018-09-10 18:46             ` Jeff King
2018-09-10 20:53         ` [PATCH] http-backend: Treat empty CONTENT_LENGTH as zero Max Kirillov
2018-09-10 21:22           ` Jonathan Nieder
2018-09-11  1:55             ` Jeff King
2018-09-11  2:20               ` Jonathan Nieder
2018-09-11  2:30                 ` Jeff King
2018-09-11  1:58           ` Jeff King
2018-09-11  3:42           ` [PATCH] http-backend: treat " Jonathan Nieder
2018-09-11  4:03             ` Jonathan Nieder
2018-09-11 18:15               ` Junio C Hamano
2018-09-11 18:27                 ` Junio C Hamano
2018-09-12  5:56                 ` Jeff King
2018-09-12  6:26                   ` Jonathan Nieder
2018-09-12 16:10                   ` Junio C Hamano
2018-09-11  4:18             ` Junio C Hamano
2018-09-11  4:29               ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180908054224.21856-1-max@max630.net \
    --to=max@max630.net \
    --cc=git@vger.kernel.org \
    --cc=jelmer@jelmer.uk \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git