Merge branch 'jk/daemon-fixes' into maint

Assorted fixes to "git daemon".

* jk/daemon-fixes:
  daemon: fix length computation in newline stripping
  t/lib-git-daemon: add network-protocol helpers
  daemon: handle NULs in extended attribute string
  daemon: fix off-by-one in logging extended attributes
  t/lib-git-daemon: record daemon log
  t5570: use ls-remote instead of clone for interp tests

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,14 +760,8 @@ static int execute(void)
 	alarm(0);
 
 	len = strlen(line);
-	if (pktlen != len)
+	if (len && line[len-1] == '\n')
-		loginfo("Extended attributes (%d bytes) exist <%.*s>",
+		line[len-1] = 0;
-			(int) pktlen - len,
-			(int) pktlen - len, line + len + 1);
-	if (len && line[len-1] == '\n') {
-		line[--len] = 0;
-		pktlen--;
-	}
 
 	/* parse additional args hidden behind a NUL byte */
 	if (len != pktlen)

t/lib-git-daemon.sh

@@ -32,7 +32,8 @@ LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-${this_test#t}}
 
 GIT_DAEMON_PID=
 GIT_DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
-GIT_DAEMON_URL=git://127.0.0.1:$LIB_GIT_DAEMON_PORT
+GIT_DAEMON_HOST_PORT=127.0.0.1:$LIB_GIT_DAEMON_PORT
+GIT_DAEMON_URL=git://$GIT_DAEMON_HOST_PORT
 
 start_git_daemon() {
 	if test -n "$GIT_DAEMON_PID"
@@ -53,11 +54,19 @@ start_git_daemon() {
 		"$@" "$GIT_DAEMON_DOCUMENT_ROOT_PATH" \
 		>&3 2>git_daemon_output &
 	GIT_DAEMON_PID=$!
+	>daemon.log
 	{
-		read line <&7
+		read -r line <&7
-		echo >&4 "$line"
+		printf "%s\n" "$line"
-		cat <&7 >&4 &
+		printf >&4 "%s\n" "$line"
-	} 7<git_daemon_output &&
+		(
+			while read -r line <&7
+			do
+				printf "%s\n" "$line"
+				printf >&4 "%s\n" "$line"
+			done
+		) &
+	} 7<git_daemon_output >>"$TRASH_DIRECTORY/daemon.log" &&
 
 	# Check expected output
 	if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
@@ -90,3 +99,25 @@ stop_git_daemon() {
 	GIT_DAEMON_PID=
 	rm -f git_daemon_output
 }
+
+# A stripped-down version of a netcat client, that connects to a "host:port"
+# given in $1, sends its stdin followed by EOF, then dumps the response (until
+# EOF) to stdout.
+fake_nc() {
+	if ! test_declared_prereq FAKENC
+	then
+		echo >&4 "fake_nc: need to declare FAKENC prerequisite"
+		return 127
+	fi
+	perl -Mstrict -MIO::Socket::INET -e '
+		my $s = IO::Socket::INET->new(shift)
+			or die "unable to open socket: $!";
+		print $s <STDIN>;
+		$s->shutdown(1);
+		print <$s>;
+	' "$@"
+}
+
+test_lazy_prereq FAKENC '
+	perl -MIO::Socket::INET -e "exit 0"
+'

t/t5570-git-daemon.sh

@@ -167,23 +167,48 @@ test_expect_success 'access repo via interpolated hostname' '
 	git init --bare "$repo" &&
 	git push "$repo" HEAD &&
 	>"$repo"/git-daemon-export-ok &&
-	rm -rf tmp.git &&
 	GIT_OVERRIDE_VIRTUAL_HOST=localhost \
-		git clone --bare "$GIT_DAEMON_URL/interp.git" tmp.git &&
+		git ls-remote "$GIT_DAEMON_URL/interp.git" &&
-	rm -rf tmp.git &&
 	GIT_OVERRIDE_VIRTUAL_HOST=LOCALHOST \
-		git clone --bare "$GIT_DAEMON_URL/interp.git" tmp.git
+		git ls-remote "$GIT_DAEMON_URL/interp.git"
 '
 
 test_expect_success 'hostname cannot break out of directory' '
-	rm -rf tmp.git &&
 	repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/../escape.git" &&
 	git init --bare "$repo" &&
 	git push "$repo" HEAD &&
 	>"$repo"/git-daemon-export-ok &&
 	test_must_fail \
 		env GIT_OVERRIDE_VIRTUAL_HOST=.. \
-		git clone --bare "$GIT_DAEMON_URL/escape.git" tmp.git
+		git ls-remote "$GIT_DAEMON_URL/escape.git"
+'
+
+test_expect_success 'daemon log records all attributes' '
+	cat >expect <<-\EOF &&
+	Extended attribute "host": localhost
+	Extended attribute "protocol": version=1
+	EOF
+	>daemon.log &&
+	GIT_OVERRIDE_VIRTUAL_HOST=localhost \
+		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
+'
+
+test_expect_success FAKENC 'hostname interpolation works after LF-stripping' '
+	{
+		printf "git-upload-pack /interp.git\n\0host=localhost" | packetize
+		printf "0000"
+	} >input &&
+	fake_nc "$GIT_DAEMON_HOST_PORT" <input >output &&
+	depacketize <output >output.raw &&
+
+	# just pick out the value of master, which avoids any protocol
+	# particulars
+	perl -lne "print \$1 if m{^(\\S+) refs/heads/master}" <output.raw >actual &&
+	git -C "$repo" rev-parse master >expect &&
+	test_cmp expect actual
 '
 
 stop_git_daemon

t/test-lib-functions.sh

@@ -1020,3 +1020,37 @@ nongit () {
 		"$@"
 	)
 }
+
+# convert stdin to pktline representation; note that empty input becomes an
+# empty packet, not a flush packet (for that you can just print 0000 yourself).
+packetize() {
+	cat >packetize.tmp &&
+	len=$(wc -c <packetize.tmp) &&
+	printf '%04x%s' "$(($len + 4))" &&
+	cat packetize.tmp &&
+	rm -f packetize.tmp
+}
+
+# Parse the input as a series of pktlines, writing the result to stdout.
+# Sideband markers are removed automatically, and the output is routed to
+# stderr if appropriate.
+#
+# NUL bytes are converted to "\\0" for ease of parsing with text tools.
+depacketize () {
+	perl -e '
+		while (read(STDIN, $len, 4) == 4) {
+			if ($len eq "0000") {
+				print "FLUSH\n";
+			} else {
+				read(STDIN, $buf, hex($len) - 4);
+				$buf =~ s/\0/\\0/g;
+				if ($buf =~ s/^[\x2\x3]//) {
+					print STDERR $buf;
+				} else {
+					$buf =~ s/^\x1//;
+					print $buf;
+				}
+			}
+		}
+	'
+}