From: Stefan Sperling <stsp@stsp.name>
Subject: gotsh serve_read error handling
To: gameoftrees@openbsd.org
Date: Sat, 21 Jan 2023 11:06:30 +0100

serve_read() is not jumping into the error path when a read error occurs.
Fix this and adjust some tests accordingly which shows that the errors
returned now are more reasonable.

The same problem exists in serve_write() but that an be fixed separately.

ok?

diff 695bc1ecccf24ece2b0b8a03a5f0d26ad5116a0e 11d907f5f1f58d1a983e27eb0de37b5d524427dd
commit - 695bc1ecccf24ece2b0b8a03a5f0d26ad5116a0e
commit + 11d907f5f1f58d1a983e27eb0de37b5d524427dd
blob - 33fddb5601425a93824e79798b4bd2ee24a574a3
blob + 043a4dd74cbffb62ce90eba9ec362db3ad6c61f5
--- lib/serve.c
+++ lib/serve.c
@@ -896,7 +896,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
 		buf[0] = '\0';
 		err = got_pkt_readpkt(&n, infd, buf, sizeof(buf), chattygot);
 		if (err)
-			break;
+			goto done;
 		if (n == 0) {
 			if (curstate != STATE_EXPECT_WANT &&
 			    curstate != STATE_EXPECT_MORE_WANT &&
blob - d01cd1bae40ee00fa47c8d4205b83088b4f36a73
blob + 96339d43615d5d8fad7e4a300f64c7db2dc9cea8
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -24,6 +24,7 @@ export GOT_IGNORE_GITCONFIG=1
 export GOT_LOG_DEFAULT_LIMIT=0
 export GOT_TEST_ROOT="/tmp"
 export GOT_IGNORE_GITCONFIG=1
+export GOT_VERSION_STR=`got --version | cut -d ' ' -f2`
 
 export MALLOC_OPTIONS=S
 
blob - 2896db4ceb362abfd70987b33dcf5f158b418c91
blob + 113aa4cf7bd50234a0d5ea376652934c0ef7b007
--- regress/gotd/request_bad.sh
+++ regress/gotd/request_bad.sh
@@ -93,12 +93,16 @@ test_request_bad_length_empty() {
 		| ssh ${GOTD_DEVUSER}@127.0.0.1 git-upload-pack '/test-repo' \
 		> $testroot/stdout 2>$testroot/stderr
 
-	printf "00000008NAK\n0021ERR read: Bad file descriptor" \
+	echo -n '006c0000000000000000000000000000000000000000 ' \
 		> $testroot/stdout.expected
+	printf "capabilities^{}\0" >> $testroot/stdout.expected
+	echo -n " agent=got/${GOT_VERSION_STR} ofs-delta side-band-64k" \
+		>> $testroot/stdout.expected
+	echo -n '00000018ERR packet too short' >> $testroot/stdout.expected
 
-	echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected
+	echo "gotsh: packet too short" > $testroot/stderr.expected
 
-	cmp -s $testroot/stdout.expected $testroot/stdout 0 108
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		echo "unexpected stdout" >&2
@@ -125,12 +129,16 @@ test_request_bad_length_small() {
 		| ssh ${GOTD_DEVUSER}@127.0.0.1 git-upload-pack '/test-repo' \
 		> $testroot/stdout 2>$testroot/stderr
 
-	printf "00000008NAK\n0021ERR read: Bad file descriptor" \
+	echo -n '006c0000000000000000000000000000000000000000 ' \
 		> $testroot/stdout.expected
+	printf "capabilities^{}\0" >> $testroot/stdout.expected
+	echo -n " agent=got/${GOT_VERSION_STR} ofs-delta side-band-64k" \
+		>> $testroot/stdout.expected
+	echo -n '00000018ERR packet too short' >> $testroot/stdout.expected
 
-	echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected
+	echo "gotsh: packet too short" > $testroot/stderr.expected
 
-	cmp -s $testroot/stdout.expected $testroot/stdout 0 108
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		echo "unexpected stdout" >&2
@@ -157,12 +165,17 @@ test_request_bad_length_large() {
 		| ssh ${GOTD_DEVUSER}@127.0.0.1 git-upload-pack '/test-repo' \
 		> $testroot/stdout 2>$testroot/stderr
 
-	printf "00000008NAK\n0021ERR read: Bad file descriptor" \
+	echo -n '006c0000000000000000000000000000000000000000 ' \
 		> $testroot/stdout.expected
+	printf "capabilities^{}\0" >> $testroot/stdout.expected
+	echo -n " agent=got/${GOT_VERSION_STR} ofs-delta side-band-64k" \
+		>> $testroot/stdout.expected
+	echo -n '0000001eERR unexpected end of file' \
+		>> $testroot/stdout.expected
 
-	echo "gotsh: read: Bad file descriptor" > $testroot/stderr.expected
+	echo "gotsh: unexpected end of file" > $testroot/stderr.expected
 
-	cmp -s $testroot/stdout.expected $testroot/stdout 0 108
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		echo "unexpected stdout" >&2