Re: [PATCH] quiltimport: fix backslash expansion in patch subjects

[Junio C Hamano <gitster@pobox.com>]

Sasha Levin <sashal@kernel.org> writes:

> echo interprets backslash sequences, so a patch with "\0" in its

"Some implementations of echo"; I think it is in XSI but the
plain vanilla POSIX makes it "implementation-defined".

> subject has that expanded into a NUL byte, which git commit-tree
> rejects.
>
> Use printf '%s\n' instead, which doesn't interpret the string.
>
> Also quote $tmp_dir to handle paths with spaces.
>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  git-quiltimport.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index eb34cda409..38302d28c9 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
>  # Find the initial commit
>  commit=$(git rev-parse HEAD)
>  
> -mkdir $tmp_dir || exit 2
> +mkdir "$tmp_dir" || exit 2
>  while read patch_name level garbage <&3
>  do
>  	case "$patch_name" in ''|'#'*) continue;; esac
> @@ -101,7 +101,7 @@ do
>  		echo "$patch_name doesn't exist. Skipping."
>  		continue
>  	fi
> -	echo $patch_name
> +	printf '%s\n' "$patch_name"

Unquoted "$patch_name" in the original fed to "echo" is doing more
than just backslash 0.

    patch_name="My   casual patch"

would be split into three tokens, runs of multiple spaces in the
original will be squashed into one, and "My casual patch" would have
been the result, which people may have appreciated as cleaning up a
sloppy original patch title.

The updated version will give completely different result, losing
the "cleaning up" feature and parrotting the garbage input to
garbage output.

So I am not 100% convinced that this change would not result in
robbing Peter to pay Paul.

Likewise for the next hunk.

Thanks.

> @@ -142,14 +142,14 @@ do
>  	SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
>  	export GIT_AUTHOR_DATE SUBJECT
>  	if [ -z "$SUBJECT" ] ; then
> -		SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
> +		SUBJECT=$(printf '%s' "$patch_name" | sed -e 's/.patch$//')
>  	fi
>  
>  	if [ -z "$dry_run" ] ; then
>  		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
>  		tree=$(git write-tree) &&
> -		commit=$( { echo "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
> +		commit=$( { printf '%s\n' "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
>  		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
>  	fi
>  done 3<"$QUILT_SERIES"
> -rm -rf $tmp_dir || exit 5
> +rm -rf "$tmp_dir" || exit 5
>
> base-commit: 795c338de725e13bd361214c6b768019fc45a2c1