From: Stefan Sperling <stsp@stsp.name>
Subject: fix gotd rejecting multiple ref-update lines
To: gameoftrees@openbsd.org
Date: Wed, 18 Jan 2023 11:04:44 +0100

gotd will error out if a client sends more than one ref-update at a time.
This happens because of an error in server-side state validation.

Fix this and expand an existing regression test to cover the issue.

ok?

 fix an issue where multiple ref-updates are rejected by gotd
 
diff f4e8c21cb2dc6a468bae32a4dcf3a9e18f269527 aa14133319588c6ef4a25ab790712924d3a2a6e3
commit - f4e8c21cb2dc6a468bae32a4dcf3a9e18f269527
commit + aa14133319588c6ef4a25ab790712924d3a2a6e3
blob - 2b98e5b861a9809ce19718d8a1895fa536f61c7f
blob + 356daf210a6e33f99485b698040f4993bd37d9d3
--- gotd/session.c
+++ gotd/session.c
@@ -1059,7 +1059,9 @@ session_dispatch_listener(int fd, short events, void *
 			err = forward_want(client, &imsg);
 			break;
 		case GOTD_IMSG_REF_UPDATE:
-			if (client->state != GOTD_STATE_EXPECT_REF_UPDATE) {
+			if (client->state != GOTD_STATE_EXPECT_REF_UPDATE &&
+			    client->state !=
+			    GOTD_STATE_EXPECT_MORE_REF_UPDATES) {
 				err = got_error_msg(GOT_ERR_BAD_REQUEST,
 				    "unexpected ref-update-line received");
 				break;
blob - 2a84d048566d5046d46563c82ea71b7b67015c5c
blob + cb89c0c20a28919ad7363ff1bba5419fdad3c7f1
--- regress/gotd/repo_write.sh
+++ regress/gotd/repo_write.sh
@@ -20,7 +20,7 @@ test_send_basic() {
 test_send_basic() {
 	local testroot=`test_init send_basic 1`
 
-	ls -R ${GOTD_TEST_REPO} > $testroot/repo-list.before
+	ls -R ${GOTD_TEST_REPO}/objects/pack > $testroot/repo-list.before
 
 	got clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone
 	ret=$?
@@ -38,6 +38,15 @@ test_send_basic() {
 		test_done "$testroot" "1"
 		return 1
 	fi
+	# same for Git
+	git clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone3 \
+		>$testroot/stdout 2>$testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "git clone failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
 
 	got checkout -q $testroot/repo-clone $testroot/wt >/dev/null
 	ret=$?
@@ -52,8 +61,12 @@ test_send_basic() {
 	(cd $testroot/wt && got add psi/new > /dev/null)
 	echo "more alpha" >> $testroot/wt/alpha
 	(cd $testroot/wt && got commit -m 'make changes' > /dev/null)
+	(cd $testroot/wt && got branch newbranch >/dev/null)
+	echo "even more alpha" >> $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m 'more changes' > /dev/null)
+	got tag -r $testroot/repo-clone -m "tagging 1.0" 1.0 >/dev/null
 
-	got send -q -r $testroot/repo-clone
+	got send -b main -b newbranch -q -r $testroot/repo-clone -t 1.0
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		echo "got send failed unexpectedly" >&2
@@ -89,14 +102,25 @@ EOF
 		return 1
 	fi
 
+	# Verify that git pull works, too
+	(cd $testroot/repo-clone3 && git pull -q > $testroot/stdout \
+		2> $testroot/stderr)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "git pull failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
 	# sending to a repository should result in a new pack file
-	ls -R ${GOTD_TEST_REPO} > $testroot/repo-list.after
+	ls -R ${GOTD_TEST_REPO}/objects/pack > $testroot/repo-list.after
 	diff -u $testroot/repo-list.before $testroot/repo-list.after \
 		> $testroot/repo-list.diff
 	grep '^+[^+]' < $testroot/repo-list.diff > $testroot/repo-list.newlines
 	nplus=`wc -l < $testroot/repo-list.newlines | tr -d ' '`
 	if [ "$nplus" != "2" ]; then
-		echo "$nplus new files created"
+		echo "$nplus new files created:"
+		cat $testroot/repo-list.diff
 		test_done "$testroot" "$ret"
 		return 1
 	fi