From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gitconfig: fix read/write out of bound
To: Omar Polo <op@omarpolo.com>
Cc: James Cook <falsifian@falsifian.org>, gameoftrees@openbsd.org
Date: Tue, 21 Feb 2023 14:46:35 +0100

On Tue, Feb 21, 2023 at 09:20:33AM +0100, Omar Polo wrote:
> James Cook <falsifian@falsifian.org> writes:
> > got still emits a "syntax error" warning if my ~/.gitconfig has a
> > comment line with whitespace before the initial #, but it seems to be
> > harmless. Example .gitconfig triggering the error message:
> > 
> > [user]
> > 	#comment
> > 	name = James Cook
> > 	email = falsifian@falsifian.org
> 
> This adjust the parser to look for comments after trimming the lines,
> which makes such "indented" comments work.  It also adds a test case
> for it.
> 
> There's a separate, but small, change that I'm bundling here and it's
> to log gitconfig.c error to stderr instead that on stdout.
> 
> ok?

Yep, ok.

> diff /home/op/w/gotd
> commit - 45e6b2f427b11e0fc760c10ee96eae3a6a5f06e7
> path + /home/op/w/gotd
> blob - 836551957b7ea94b0769a73ed8ac1f103def1278
> file + lib/gitconfig.c
> --- lib/gitconfig.c
> +++ lib/gitconfig.c
> @@ -55,8 +55,8 @@
>  #define LOG_DBG(x)
>  #endif
>  
> -#define log_print printf
> -#define log_error printf
> +#define log_print(...) fprintf(stderr, __VA_ARGS__)
> +#define log_error(...) fprintf(stderr, __VA_ARGS__)
>  
>  #ifdef GITCONFIG_DEBUG
>  static void
> @@ -222,10 +222,6 @@ conf_parse_line(char **section, struct got_gitconfig *
>  	size_t	 i;
>  	int	 j;
>  
> -	/* Lines starting with '#' or ';' are comments.  */
> -	if (*line == '#' || *line == ';')
> -		return NULL;
> -
>  	/* '[section]' parsing...  */
>  	if (*line == '[') {
>  		for (i = 1; i < sz; i++)
> @@ -248,6 +244,10 @@ conf_parse_line(char **section, struct got_gitconfig *
>  		sz--;
>  	}
>  
> +	/* Lines starting with '#' or ';' are comments.  */
> +	if (*line == '#' || *line == ';')
> +		return NULL;
> +
>  	/* Deal with assignments.  */
>  	for (i = 0; i < sz; i++)
>  		if (line[i] == '=') {
> blob - 62bcb9f3f02f060aa7096a6fd0d8eb9f6ded5382
> file + regress/cmdline/commit.sh
> --- regress/cmdline/commit.sh
> +++ regress/cmdline/commit.sh
> @@ -982,6 +982,9 @@ test_commit_gitconfig_author() {
>  
>  	# retry with spaces in the git config
>  	ed -s "$testroot/repo/.git/config" <<EOF
> +/^\[user/ a
> +    # it's me!
> +.
>  ,s/	/    /g
>  wq
>  EOF
> @@ -989,13 +992,24 @@ EOF
>  
>  	# unset in a subshell to avoid affecting our environment
>  	(unset GOT_IGNORE_GITCONFIG && cd "$testroot/wt" && \
> -		got commit -m 'test gitconfig author again' >/dev/null)
> +		got commit -m 'test gitconfig author again' \
> +		>/dev/null 2>$testroot/stderr)
>  	ret=$?
>  	if [ $ret -ne 0 ]; then
>  		test_done "$testroot" "$ret"
>  		return 1
>  	fi
>  
> +	# shouldn't have triggered any parsing error
> +	echo -n > $testroot/stderr.expected
> +	cmp -s $testroot/stderr.expected $testroot/stderr
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u $testroot/stderr.expected $testroot/stderr
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
>  	(cd "$testroot/repo" && got log -l1 | grep ^from: > $testroot/stdout)
>  	ret=$?
>  	if [ $ret -ne 0 ]; then
> 
>