On Fri, Aug 29, 2025 at 11:15:17AM +0200, Stefan Sperling wrote:
> If the patch file lacks a trailing newline just before end-of-file
> then 'got patch' will exit with an "unexpected privsep message" error.
> 
> Regression test and fix below.
> 
> ok?

Imrpoved version which fixes a function name in an error I overlooked
("malloc" -> "strdup") and reorders the checks in recv_patch() for
cosmetic reasons.


M  lib/patch.c                              |  11+  7-
M  libexec/got-read-patch/got-read-patch.c  |   2+  2-
M  regress/cmdline/patch.sh                 |  50+  0-

3 files changed, 63 insertions(+), 9 deletions(-)

commit - a906b277aa501d5897960b8e9f4466450d91dcac
commit + 2a1120ba5e4469c33e2fb13129ed303f45cefe40
blob - edb26e01b7ce7ae6f0582ad4b4cb1a56c500e3d4
blob + 1c80144c8b6b71c24e7500863b0dc3c3f28ff965
--- lib/patch.c
+++ lib/patch.c
@@ -164,13 +164,13 @@ pushline(struct got_patch_hunk *h, const char *line, s
 		h->cap = newcap;
 	}
 
-	if ((t = malloc(len - 1)) == NULL)
-		return got_error_from_errno("malloc");
-	memcpy(t, line + 1, len - 1);	/* skip the line type */
+	t = strdup(line + 1); /* skip the line type */
+	if (t == NULL)
+		return got_error_from_errno("strdup");
 
 	h->lines[h->len].mode = *line;
 	h->lines[h->len].line = t;
-	h->lines[h->len].len = len - 2;	/* line type and trailing NUL */
+	h->lines[h->len].len = strlen(t);
 	h->len++;
 	return NULL;
 }
@@ -299,12 +299,16 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got
 				err = got_error(GOT_ERR_PRIVSEP_MSG);
 				goto done;
 			}
-			t = imsg.data;
 			/* at least one char */
-			if (datalen < 2 || t[datalen-1] != '\0') {
+			if (datalen < 2) {
 				err = got_error(GOT_ERR_PRIVSEP_MSG);
 				goto done;
 			}
+			t = imsg.data;
+			if (memchr(t, '\0', datalen) == NULL) {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				goto done;
+			}
 			if (*t != ' ' && *t != '-' && *t != '+' &&
 			    *t != '\\') {
 				err = got_error(GOT_ERR_PRIVSEP_MSG);
@@ -312,7 +316,7 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got
 			}
 
 			if (*t != '\\')
-				err = pushline(h, t, datalen);
+				err = pushline(h, t, strlen(t));
 			else if (lastmode == '-')
 				h->old_nonl = 1;
 			else if (lastmode == '+')
blob - 14537c5cce57d708e10787a44cfd196c34bb6263
blob + bbb4fdeb0e8ad7491f1ffe1cec7450ea6bb923c2
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -467,7 +467,7 @@ send_line(const char *line, size_t len)
 	}
 
 	iov[iovcnt].iov_base = (void *)line;
-	iov[iovcnt].iov_len = len;
+	iov[iovcnt].iov_len = len + 1; /* send NUL terminator */
 	iovcnt++;
 
 	if (imsg_composev(&ibuf, GOT_IMSG_PATCH_LINE, 0, 0, -1,
@@ -585,7 +585,7 @@ parse_hunk(FILE *fp, int *done)
 			goto done;
 		}
 
-		err = send_line(line, linelen);
+		err = send_line(line, strlen(line));
 		if (err)
 			goto done;
 
blob - e5504f42e6a08f89598536b78ff4c5ed56bfb461
blob + f41077edecd7463beacb0b7995d2081ff27c3bff
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -2147,6 +2147,55 @@ test_patch_commit_keywords() {
 	test_done $testroot $ret
 }
 
+test_patch_no_newline_at_end_of_patch() {
+	local testroot=`test_init patch_no_newline_at_end_of_patch`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done $testroot $ret
+		return 1
+	fi
+
+	cat <<EOF > $testroot/wt/patch
+--- alpha
++++ alpha
+@@ -1 +1 @@
+-alpha
++alpha is my favourite character
+EOF
+
+	head -n 4 $testroot/wt/patch > $testroot/wt/patch.noeol
+	tail -n 1 $testroot/wt/patch | tr -d '\n' >> $testroot/wt/patch.noeol
+	
+	(cd $testroot/wt && got patch < patch.noeol) > $testroot/stdout
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo 'M  alpha' > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done $testroot $ret
+		return 1
+	fi
+
+	echo 'alpha is my favourite character' > $testroot/wt/alpha.expected
+	cmp -s $testroot/wt/alpha.expected $testroot/wt/alpha
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/wt/alpha.expected $testroot/wt/alpha
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	test_done $testroot $ret
+}
+
 test_parseargs "$@"
 run_test test_patch_basic
 run_test test_patch_dont_apply
@@ -2179,3 +2228,4 @@ run_test test_patch_newfile_xbit_git_diff
 run_test test_patch_umask
 run_test test_patch_remove_binary_file
 run_test test_patch_commit_keywords
+run_test test_patch_no_newline_at_end_of_patch