From: Sasha Levin <sashal@kernel.org>
Date: Mon, 09 Mar 2026 01:31:17 GMT
Subject: Re: [PATCH] quiltimport: fix backslash expansion in patch subjects
Message-ID: <aa4i5TDt7uPYw9Jq@laps>
In-Reply-To: <xmqqa4wh96qr.fsf@gitster.g>

On Sun, Mar 08, 2026 at 05:38:52PM -0700, Junio C Hamano wrote:
>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".

I had to Google this one :)

I'll reword to "Some implementations of echo (notably dash) interpret backslash
sequences by default; POSIX leaves this behavior 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.

patch_name is read from the series file via:

     while read patch_name level garbage <&3

Since read splits on IFS, patch_name only ever gets the first
whitespace delimited token. So in the normal case it can never
contain internal whitespace, and the word splitting in echo is a
noop:

     $ printf 'my-patch.patch -p1\n' |
       while read name level garbage; do
         echo "name=[$name]"
       done
     name=[my-patch.patch]

The only way patch_name gets internal spaces is if the series file
uses "\ " escaping, but in that case the spaces should be (??)
intentional.

>Likewise for the next hunk.

Same reasoning applies to the SUBJECT fallback, I think...

     SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')

patch_name is still the same single token from read. And the old path was
already quoted:

     echo "$SUBJECT"

So the only issue there was backslash interpretation, rather than splitting?
similar to 47be066026 ("rebase -i: do not "echo" random user-supplied
strings")?

I need a beer.

-- 
Thanks,
Sasha