From: Mark Jamsek <mark@jamsek.com>
Subject: Re: got rebase or diff library bug
To: Mark Jamsek <mark@jamsek.com>
Cc: Stefan Sperling <stsp@stsp.name>, Christian Weisgerber <naddy@mips.inka.de>, gameoftrees@openbsd.org
Date: Tue, 06 Aug 2024 00:58:53 +1000

Mark Jamsek <mark@jamsek.com> wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > On Mon, Aug 05, 2024 at 01:53:22AM +1000, Mark Jamsek wrote:
> > > Updated diff checks the buffer for the terminating newline--not the
> > > line returned from getline(3). This fixes an OOB read in the previous
> > > diff.
> > 
> > ok by me too (style: could avoid the reundant == 1 in boolean context)
> > 
> > > I still want to write a test for this case though.
> > 
> > Great!
> 
> This is the basic test reproduced from naddy's minimal recipe that I've
> been using to fix the bug naddy reported. It only covers the trivial
> diff3 of a histedit operation involving a malformed text file that lacks
> the terminating newline. I'm still trying to find a minimal repro to
> cover the path exercised in naddy's llvm case.

New diff now also includes a test exercising the path identified by
naddy's llvm case.


diffstat 581ac2a102c104327012657946baa4ad14ac649b 736b567efe910d81351462baff27557d28eb6540
 M  regress/cmdline/histedit.sh  |  145+  0-

1 file changed, 145 insertions(+), 0 deletions(-)

diff 581ac2a102c104327012657946baa4ad14ac649b 736b567efe910d81351462baff27557d28eb6540
commit - 581ac2a102c104327012657946baa4ad14ac649b
commit + 736b567efe910d81351462baff27557d28eb6540
blob - c7454905559296ba3c95edb66a8cf17f0700d11a
blob + eab70f0ce726ea1ddee09bde6fa789f70296de2c
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2758,6 +2758,150 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
+test_histedit_no_eof_newline() {
+	local testroot=$(test_init histedit_no_eof_newline)
+	local id_root=$(git_show_head "$testroot/repo")
+
+	got checkout "$testroot/repo" "$testroot/wt" > /dev/null
+
+	echo -n "fubar" >> $testroot/wt/alpha
+
+	(cd "$testroot/wt" && got commit -m 'no eof newline' > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cp $testroot/wt/alpha $testroot/content.expected
+
+	(cd $testroot/wt && got update -c :base:- > /dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "update to $id_root failed" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	cat > $testroot/editor.sh <<EOF
+#!/bin/sh
+ed -s "\$1" <<-EOF
+	,s/ eof / terminating /
+	w
+	EOF
+EOF
+	chmod +x $testroot/editor.sh
+
+	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
+	    VISUAL="$testroot/editor.sh" got histedit -m > /dev/null)
+
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "histedit -m failed" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	cp $testroot/wt/alpha $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat > $testroot/wt/beta <<-EOF
+	beta
+	2
+	3
+	4
+
+	6
+	7
+	8
+	EOF
+
+	(cd "$testroot/wt" && got commit -m 'padding beta' > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local id=$(git_show_head "$testroot/repo")
+
+	cat > $testroot/wt/beta <<-EOF
+	betazoid
+	3
+	4
+
+	7
+	8
+	a
+	b
+	c
+	d
+	e
+	f
+	0
+	1
+	2
+	3
+	4
+	5
+	6
+	7
+	8
+	9
+	EOF
+
+	echo -n "fubar" >> $testroot/wt/beta
+
+	(cd "$testroot/wt" && got commit -m 'beta no eof newline' > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cp $testroot/wt/beta $testroot/content.expected
+
+	(cd $testroot/wt && got update -c :base:- > /dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "update to $id failed" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
+	    VISUAL="$testroot/editor.sh" got histedit -m > /dev/null)
+
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "histedit -m failed" >&2
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	cp $testroot/wt/beta $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" 0
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -2786,3 +2930,4 @@ run_test test_histedit_umask
 run_test test_histedit_mesg_filemode_change
 run_test test_histedit_drop_only
 run_test test_histedit_conflict_revert
+run_test test_histedit_no_eof_newline


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68