git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 4/6] daemon: handle NULs in extended attribute string
Date: Wed, 24 Jan 2018 19:56:20 -0500	[thread overview]
Message-ID: <20180125005619.GD26850@sigill.intra.peff.net> (raw)
In-Reply-To: <20180125005447.GA26661@sigill.intra.peff.net>

If we receive a request with extended attributes after the
NUL, we try to write those attributes to the log. We do so
with a "%s" format specifier, which will only show
characters up to the first NUL.

That's enough for printing a "host=" specifier. But since
dfe422d04d (daemon: recognize hidden request arguments,
2017-10-16) we may have another NUL, followed by protocol
parameters, and those are not logged at all.

Let's cut out the attempt to show the whole string, and
instead log when we parse individual attributes. We could
leave the "extended attributes (%d bytes) exist" part of the
log, which in theory could alert us to attributes that fail
to parse. But anything we don't parse as a "host=" parameter
gets blindly added to the "protocol" attribute, so we'd see
it in that part of the log.

Signed-off-by: Jeff King <peff@peff.net>
---
 daemon.c              | 9 ++++-----
 t/t5570-git-daemon.sh | 8 +++++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/daemon.c b/daemon.c
index d78afc8e96..652f89b6e7 100644
--- a/daemon.c
+++ b/daemon.c
@@ -597,6 +597,7 @@ static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
 		if (strncasecmp("host=", extra_args, 5) == 0) {
 			val = extra_args + 5;
 			vallen = strlen(val) + 1;
+			loginfo("Extended attribute \"host\": %s", val);
 			if (*val) {
 				/* Split <host>:<port> at colon. */
 				char *host;
@@ -647,9 +648,11 @@ static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,
 		}
 	}
 
-	if (git_protocol.len > 0)
+	if (git_protocol.len > 0) {
+		loginfo("Extended attribute \"protocol\": %s", git_protocol.buf);
 		argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=%s",
 				 git_protocol.buf);
+	}
 	strbuf_release(&git_protocol);
 }
 
@@ -757,10 +760,6 @@ static int execute(void)
 	alarm(0);
 
 	len = strlen(line);
-	if (pktlen != len)
-		loginfo("Extended attributes (%d bytes) exist <%.*s>",
-			(int) pktlen - len - 1,
-			(int) pktlen - len - 1, line + len + 1);
 	if (len && line[len-1] == '\n') {
 		line[--len] = 0;
 		pktlen--;
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 359af3994a..b556469db6 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -183,13 +183,15 @@ test_expect_success 'hostname cannot break out of directory' '
 		git ls-remote "$GIT_DAEMON_URL/escape.git"
 '
 
-test_expect_success 'daemon log records hostnames' '
+test_expect_success 'daemon log records all attributes' '
 	cat >expect <<-\EOF &&
-	Extended attributes (15 bytes) exist <host=localhost>
+	Extended attribute "host": localhost
+	Extended attribute "protocol": version=1
 	EOF
 	>daemon.log &&
 	GIT_OVERRIDE_VIRTUAL_HOST=localhost \
-		git ls-remote "$GIT_DAEMON_URL/interp.git" &&
+		git -c protocol.version=1 \
+			ls-remote "$GIT_DAEMON_URL/interp.git" &&
 	grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&
 	test_cmp expect actual
 '
-- 
2.16.1.273.gfdaa03aa74


  parent reply	other threads:[~2018-01-25  0:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-25  0:54 [PATCH 0/6] off-by-one errors in git-daemon Jeff King
2018-01-25  0:55 ` [PATCH 1/6] t5570: use ls-remote instead of clone for interp tests Jeff King
2018-01-25  0:55 ` [PATCH 2/6] t/lib-git-daemon: record daemon log Jeff King
2018-01-25 11:56   ` Lucas Werkmeister
2018-01-25 19:08     ` Jeff King
2018-01-25  0:56 ` [PATCH 3/6] daemon: fix off-by-one in logging extended attributes Jeff King
2018-01-25  0:56 ` Jeff King [this message]
2018-01-25  0:58 ` [PATCH 5/6] t/lib-git-daemon: add network-protocol helpers Jeff King
2018-01-25  0:58 ` [PATCH 6/6] daemon: fix length computation in newline stripping Jeff King
2018-01-25 21:38   ` Eric Sunshine
2018-01-26 18:52     ` Jeff King
2018-01-25 18:46 ` [PATCH 0/6] off-by-one errors in git-daemon Junio C Hamano
2018-01-25 19:16   ` Jeff King

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=20180125005619.GD26850@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).