daemon: handle NULs in extended attribute string

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/daemon.c b/daemon.c
index d78afc8e96d29e75ef13e858fc23912b165afb14..652f89b6e761646c75e5a549780e6d2087e36d27 100644 (file)
--- 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 359af3994af904e6cc5951a6c19fb9e7e9b82d4c..b556469db6583d96fcb4642acbf100628b5b2374 100755 (executable)
--- 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
 '