{"thread":{"id":"65169","subject":"[PATCH] quiltimport: fix backslash expansion in patch subjects","startedAt":"2026-03-08T16:55:35Z","lastAt":"2026-03-09T01:31:18Z","messageCount":5,"participants":["Sasha Levin","Ben Knoble","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"538204","messageId":"20260308165531.40655-1-sashal@kernel.org","threadId":"65169","inReplyTo":null,"subject":"[PATCH] quiltimport: fix backslash expansion in patch subjects","fromName":"Sasha Levin","fromEmail":"sashal@kernel.org","sentAt":"2026-03-08T16:55:31Z","receivedAt":"2026-03-08T16:55:35Z","isPatch":true,"sender":{"key":"sashal@kernel.org","avatar":null},"body":"echo interprets backslash sequences, so a patch with \"\\0\" in its\nsubject has that expanded into a NUL byte, which git commit-tree\nrejects.\n\nUse printf '%s\\n' instead, which doesn't interpret the string.\n\nAlso quote $tmp_dir to handle paths with spaces.\n\nSigned-off-by: Sasha Levin <sashal@kernel.org>\n---\n git-quiltimport.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-quiltimport.sh b/git-quiltimport.sh\nindex eb34cda409..38302d28c9 100755\n--- a/git-quiltimport.sh\n+++ b/git-quiltimport.sh\n@@ -79,7 +79,7 @@ tmp_info=\"$tmp_dir/info\"\n # Find the initial commit\n commit=$(git rev-parse HEAD)\n \n-mkdir $tmp_dir || exit 2\n+mkdir \"$tmp_dir\" || exit 2\n while read patch_name level garbage <&3\n do\n \tcase \"$patch_name\" in ''|'#'*) continue;; esac\n@@ -101,7 +101,7 @@ do\n \t\techo \"$patch_name doesn't exist. Skipping.\"\n \t\tcontinue\n \tfi\n-\techo $patch_name\n+\tprintf '%s\\n' \"$patch_name\"\n \tgit mailinfo $MAILINFO_OPT \"$tmp_msg\" \"$tmp_patch\" \\\n \t\t<\"$QUILT_PATCHES/$patch_name\" >\"$tmp_info\" || exit 3\n \ttest -s \"$tmp_patch\" || {\n@@ -142,14 +142,14 @@ do\n \tSUBJECT=$(sed -ne 's/Subject: //p' \"$tmp_info\")\n \texport GIT_AUTHOR_DATE SUBJECT\n \tif [ -z \"$SUBJECT\" ] ; then\n-\t\tSUBJECT=$(echo $patch_name | sed -e 's/.patch$//')\n+\t\tSUBJECT=$(printf '%s' \"$patch_name\" | sed -e 's/.patch$//')\n \tfi\n \n \tif [ -z \"$dry_run\" ] ; then\n \t\tgit apply --index -C1 ${level:+\"$level\"} \"$tmp_patch\" &&\n \t\ttree=$(git write-tree) &&\n-\t\tcommit=$( { echo \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n+\t\tcommit=$( { printf '%s\\n' \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n \t\tgit update-ref -m \"quiltimport: $patch_name\" HEAD $commit || exit 4\n \tfi\n done 3<\"$QUILT_SERIES\"\n-rm -rf $tmp_dir || exit 5\n+rm -rf \"$tmp_dir\" || exit 5\n\nbase-commit: 795c338de725e13bd361214c6b768019fc45a2c1\n-- \n2.51.0\n\n"},{"id":"538214","messageId":"92EC21F4-52E1-4FE8-A1B8-5878D6CC654C@gmail.com","threadId":"65169","inReplyTo":"20260308165531.40655-1-sashal@kernel.org","subject":"Re: [PATCH] quiltimport: fix backslash expansion in patch subjects","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-03-08T20:56:57Z","receivedAt":"2026-03-08T20:57:09Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"\n> Le 8 mars 2026 à 13:01, Sasha Levin <sashal@kernel.org> a écrit :\n> \n> echo interprets backslash sequences, so a patch with \"\\0\" in its\n> subject has that expanded into a NUL byte, which git commit-tree\n> rejects.\n\nEcho _shouldn’t_ do that without flags like -e depending on your implementation, but I wouldn’t put it past echo ;) hence my preference for printf in scripts.\n\nMore interestingly, though: do the patch-names or subjects being adjusted below contain such a sequence? They look like user input, so possibly (and we can’t be sure they don’t).\n\nIt might be nice to say more about what “rejects” means (errors confusingly? Truncates user input?), but otherwise I expect this is reasonable.\n\n> Use printf '%s\\n' instead, which doesn't interpret the string.\n> \n> Also quote $tmp_dir to handle paths with spaces.\n> \n> Signed-off-by: Sasha Levin <sashal@kernel.org>\n> ---\n> git-quiltimport.sh | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-quiltimport.sh b/git-quiltimport.sh\n> index eb34cda409..38302d28c9 100755\n> --- a/git-quiltimport.sh\n> +++ b/git-quiltimport.sh\n> @@ -79,7 +79,7 @@ tmp_info=\"$tmp_dir/info\"\n> # Find the initial commit\n> commit=$(git rev-parse HEAD)\n> \n> -mkdir $tmp_dir || exit 2\n> +mkdir \"$tmp_dir\" || exit 2\n\nWe prefer to leave such “while at it” changes in separate patches, so perhaps a preliminary cleanup “quote variable expansions to handle whitespace” or some such step would help?\n\n(I didn’t look past the context to see if tmp_dir may have whitespace or shell meta characters.)\n\n> while read patch_name level garbage <&3\n> do\n>    case \"$patch_name\" in ''|'#'*) continue;; esac\n> @@ -101,7 +101,7 @@ do\n>        echo \"$patch_name doesn't exist. Skipping.\"\n>        continue\n>    fi\n> -    echo $patch_name\n> +    printf '%s\\n' \"$patch_name\"\n\nDoes this go to commit-tree? Or just protecting the output for the user’s terminal?\n\n>    git mailinfo $MAILINFO_OPT \"$tmp_msg\" \"$tmp_patch\" \\\n>        <\"$QUILT_PATCHES/$patch_name\" >\"$tmp_info\" || exit 3\n>    test -s \"$tmp_patch\" || {\n> @@ -142,14 +142,14 @@ do\n>    SUBJECT=$(sed -ne 's/Subject: //p' \"$tmp_info\")\n>    export GIT_AUTHOR_DATE SUBJECT\n>    if [ -z \"$SUBJECT\" ] ; then\n> -        SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')\n> +        SUBJECT=$(printf '%s' \"$patch_name\" | sed -e 's/.patch$//')\n\nInteresting. I think POSIX sh  supports the ${x#suffix} expansion, which could avoid sed. I think it unlikely the “.” in the RE is intended to match any character rather than a literal dot. But should be done a separate patch and could be left for another series if you wanted (assuming my memory of supported expansions is correct).\n\nImportantly, this does get fed to commit-tree below…\n\n>    fi\n> \n>    if [ -z \"$dry_run\" ] ; then\n>        git apply --index -C1 ${level:+\"$level\"} \"$tmp_patch\" &&\n>        tree=$(git write-tree) &&\n> -        commit=$( { echo \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n> +        commit=$( { printf '%s\\n' \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n>        git update-ref -m \"quiltimport: $patch_name\" HEAD $commit || exit 4\n>    fi\n\n… so may need protected as described. Neat. \n\n> done 3<\"$QUILT_SERIES\"\n> -rm -rf $tmp_dir || exit 5\n> +rm -rf \"$tmp_dir\" || exit 5\n\nDitto for cleanup. \n\n> base-commit: 795c338de725e13bd361214c6b768019fc45a2c1\n> --\n> 2.51.0\n\nThanks"},{"id":"538225","messageId":"aa4JIWrkgNAPnXA5@laps","threadId":"65169","inReplyTo":"92EC21F4-52E1-4FE8-A1B8-5878D6CC654C@gmail.com","subject":"Re: [PATCH] quiltimport: fix backslash expansion in patch subjects","fromName":"Sasha Levin","fromEmail":"sashal@kernel.org","sentAt":"2026-03-08T23:41:21Z","receivedAt":"2026-03-08T23:41:22Z","isPatch":true,"sender":{"key":"sashal@kernel.org","avatar":null},"body":"Thanks for the review Ben! I'll send a v2 based on your review.\n\nMore explanation below:\n\nOn Sun, Mar 08, 2026 at 04:56:57PM -0400, Ben Knoble wrote:\n>> Le 8 mars 2026 à 13:01, Sasha Levin <sashal@kernel.org> a écrit :\n>>\n>> echo interprets backslash sequences, so a patch with \"\\0\" in its\n>> subject has that expanded into a NUL byte, which git commit-tree\n>> rejects.\n>\n>Echo _shouldn’t_ do that without flags like -e depending on your implementation, but I wouldn’t put it past echo ;) hence my preference for printf in scripts.\n\nRight, this is just dash being \"funny\" and interprets backslash sequences in\necho by default.\n\nsasha@laps:~$ dash -c 'echo \"hello \\0 world\"' | xxd\n00000000: 6865 6c6c 6f20 0020 776f 726c 640a       hello . world.\nsasha@laps:~$ bash -c 'echo \"hello \\0 world\"' | xxd\n00000000: 6865 6c6c 6f20 5c30 2077 6f72 6c64 0a    hello \\0 world.\n\n>More interestingly, though: do the patch-names or subjects being adjusted below contain such a sequence? They look like user input, so possibly (and we can’t be sure they don’t).\n\nYes, this was hit in practice. A patch in the Linux kernel stable queue was\ntitled \"selftests: tc_actions: don't dump 2MB of \\0 to stdout\". The literal \\0\nin the subject caused git-quiltimport to abort mid-import under dash.\n\n>It might be nice to say more about what “rejects” means (errors confusingly? Truncates user input?), but otherwise I expect this is reasonable.\n\ncommit_tree_extended() in commit.c does:\n\n     if (memchr(msg, '\\0', msg_len))\n         return error(\"a NUL byte in commit log message not allowed.\");\n\nSo commit-tree errors out, which triggers the \"|| exit 4\" in the script and\naborts the entire quiltimport. Everything after the offending patch in the\nseries is silently lost. I'll add it to the commit message.\n\n>> Use printf '%s\\n' instead, which doesn't interpret the string.\n>>\n>> Also quote $tmp_dir to handle paths with spaces.\n>>\n>> Signed-off-by: Sasha Levin <sashal@kernel.org>\n>> ---\n>> git-quiltimport.sh | 10 +++++-----\n>> 1 file changed, 5 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/git-quiltimport.sh b/git-quiltimport.sh\n>> index eb34cda409..38302d28c9 100755\n>> --- a/git-quiltimport.sh\n>> +++ b/git-quiltimport.sh\n>> @@ -79,7 +79,7 @@ tmp_info=\"$tmp_dir/info\"\n>> # Find the initial commit\n>> commit=$(git rev-parse HEAD)\n>>\n>> -mkdir $tmp_dir || exit 2\n>> +mkdir \"$tmp_dir\" || exit 2\n>\n>We prefer to leave such “while at it” changes in separate patches, so perhaps a preliminary cleanup “quote variable expansions to handle whitespace” or some such step would help?\n>\n>(I didn’t look past the context to see if tmp_dir may have whitespace or shell meta characters.)\n\nWill do: I'll split the $tmp_dir quoting into a preliminary cleanup patch.\n\n>> while read patch_name level garbage <&3\n>> do\n>>    case \"$patch_name\" in ''|'#'*) continue;; esac\n>> @@ -101,7 +101,7 @@ do\n>>        echo \"$patch_name doesn't exist. Skipping.\"\n>>        continue\n>>    fi\n>> -    echo $patch_name\n>> +    printf '%s\\n' \"$patch_name\"\n>\n>Does this go to commit-tree? Or just protecting the output for the user’s terminal?\n\nJust terminal output. It doesn't feed into commit-tree, but under dash a patch\nname with backslash sequences would still produce garbled output, so worth\nfixing alongside.\n\n>>    git mailinfo $MAILINFO_OPT \"$tmp_msg\" \"$tmp_patch\" \\\n>>        <\"$QUILT_PATCHES/$patch_name\" >\"$tmp_info\" || exit 3\n>>    test -s \"$tmp_patch\" || {\n>> @@ -142,14 +142,14 @@ do\n>>    SUBJECT=$(sed -ne 's/Subject: //p' \"$tmp_info\")\n>>    export GIT_AUTHOR_DATE SUBJECT\n>>    if [ -z \"$SUBJECT\" ] ; then\n>> -        SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')\n>> +        SUBJECT=$(printf '%s' \"$patch_name\" | sed -e 's/.patch$//')\n>\n>Interesting. I think POSIX sh  supports the ${x#suffix} expansion, which could avoid sed. I think it unlikely the “.” in the RE is intended to match any character rather than a literal dot. But should be done a separate patch and could be left for another series if you wanted (assuming my memory of supported expansions is correct).\n>\n>Importantly, this does get fed to commit-tree below…\n\nGood catch! ${patch_name%.patch} is POSIX and already used throughout git's\nshell scripts.\n\n>>    fi\n>>\n>>    if [ -z \"$dry_run\" ] ; then\n>>        git apply --index -C1 ${level:+\"$level\"} \"$tmp_patch\" &&\n>>        tree=$(git write-tree) &&\n>> -        commit=$( { echo \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n>> +        commit=$( { printf '%s\\n' \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n>>        git update-ref -m \"quiltimport: $patch_name\" HEAD $commit || exit 4\n>>    fi\n>\n>… so may need protected as described. Neat.\n>\n>> done 3<\"$QUILT_SERIES\"\n>> -rm -rf $tmp_dir || exit 5\n>> +rm -rf \"$tmp_dir\" || exit 5\n>\n>Ditto for cleanup.\n\nack!\n\n-- \nThanks,\nSasha\n"},{"id":"538229","messageId":"xmqqa4wh96qr.fsf@gitster.g","threadId":"65169","inReplyTo":"20260308165531.40655-1-sashal@kernel.org","subject":"Re: [PATCH] quiltimport: fix backslash expansion in patch subjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T00:38:52Z","receivedAt":"2026-03-09T00:38:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sasha Levin <sashal@kernel.org> writes:\n\n> echo interprets backslash sequences, so a patch with \"\\0\" in its\n\n\"Some implementations of echo\"; I think it is in XSI but the\nplain vanilla POSIX makes it \"implementation-defined\".\n\n> subject has that expanded into a NUL byte, which git commit-tree\n> rejects.\n>\n> Use printf '%s\\n' instead, which doesn't interpret the string.\n>\n> Also quote $tmp_dir to handle paths with spaces.\n>\n> Signed-off-by: Sasha Levin <sashal@kernel.org>\n> ---\n>  git-quiltimport.sh | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-quiltimport.sh b/git-quiltimport.sh\n> index eb34cda409..38302d28c9 100755\n> --- a/git-quiltimport.sh\n> +++ b/git-quiltimport.sh\n> @@ -79,7 +79,7 @@ tmp_info=\"$tmp_dir/info\"\n>  # Find the initial commit\n>  commit=$(git rev-parse HEAD)\n>  \n> -mkdir $tmp_dir || exit 2\n> +mkdir \"$tmp_dir\" || exit 2\n>  while read patch_name level garbage <&3\n>  do\n>  \tcase \"$patch_name\" in ''|'#'*) continue;; esac\n> @@ -101,7 +101,7 @@ do\n>  \t\techo \"$patch_name doesn't exist. Skipping.\"\n>  \t\tcontinue\n>  \tfi\n> -\techo $patch_name\n> +\tprintf '%s\\n' \"$patch_name\"\n\nUnquoted \"$patch_name\" in the original fed to \"echo\" is doing more\nthan just backslash 0.\n\n    patch_name=\"My   casual patch\"\n\nwould be split into three tokens, runs of multiple spaces in the\noriginal will be squashed into one, and \"My casual patch\" would have\nbeen the result, which people may have appreciated as cleaning up a\nsloppy original patch title.\n\nThe updated version will give completely different result, losing\nthe \"cleaning up\" feature and parrotting the garbage input to\ngarbage output.\n\nSo I am not 100% convinced that this change would not result in\nrobbing Peter to pay Paul.\n\nLikewise for the next hunk.\n\nThanks.\n\n> @@ -142,14 +142,14 @@ do\n>  \tSUBJECT=$(sed -ne 's/Subject: //p' \"$tmp_info\")\n>  \texport GIT_AUTHOR_DATE SUBJECT\n>  \tif [ -z \"$SUBJECT\" ] ; then\n> -\t\tSUBJECT=$(echo $patch_name | sed -e 's/.patch$//')\n> +\t\tSUBJECT=$(printf '%s' \"$patch_name\" | sed -e 's/.patch$//')\n>  \tfi\n>  \n>  \tif [ -z \"$dry_run\" ] ; then\n>  \t\tgit apply --index -C1 ${level:+\"$level\"} \"$tmp_patch\" &&\n>  \t\ttree=$(git write-tree) &&\n> -\t\tcommit=$( { echo \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n> +\t\tcommit=$( { printf '%s\\n' \"$SUBJECT\"; echo; cat \"$tmp_msg\"; } | git commit-tree $tree -p $commit) &&\n>  \t\tgit update-ref -m \"quiltimport: $patch_name\" HEAD $commit || exit 4\n>  \tfi\n>  done 3<\"$QUILT_SERIES\"\n> -rm -rf $tmp_dir || exit 5\n> +rm -rf \"$tmp_dir\" || exit 5\n>\n> base-commit: 795c338de725e13bd361214c6b768019fc45a2c1\n"},{"id":"538243","messageId":"aa4i5TDt7uPYw9Jq@laps","threadId":"65169","inReplyTo":"xmqqa4wh96qr.fsf@gitster.g","subject":"Re: [PATCH] quiltimport: fix backslash expansion in patch subjects","fromName":"Sasha Levin","fromEmail":"sashal@kernel.org","sentAt":"2026-03-09T01:31:17Z","receivedAt":"2026-03-09T01:31:18Z","isPatch":true,"sender":{"key":"sashal@kernel.org","avatar":null},"body":"On Sun, Mar 08, 2026 at 05:38:52PM -0700, Junio C Hamano wrote:\n>Sasha Levin <sashal@kernel.org> writes:\n>\n>> echo interprets backslash sequences, so a patch with \"\\0\" in its\n>\n>\"Some implementations of echo\"; I think it is in XSI but the\n>plain vanilla POSIX makes it \"implementation-defined\".\n\nI had to Google this one :)\n\nI'll reword to \"Some implementations of echo (notably dash) interpret backslash\nsequences by default; POSIX leaves this behavior implementation-defined.\"\n\n>> subject has that expanded into a NUL byte, which git commit-tree\n>> rejects.\n>>\n>> Use printf '%s\\n' instead, which doesn't interpret the string.\n>>\n>> Also quote $tmp_dir to handle paths with spaces.\n>>\n>> Signed-off-by: Sasha Levin <sashal@kernel.org>\n>> ---\n>>  git-quiltimport.sh | 10 +++++-----\n>>  1 file changed, 5 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/git-quiltimport.sh b/git-quiltimport.sh\n>> index eb34cda409..38302d28c9 100755\n>> --- a/git-quiltimport.sh\n>> +++ b/git-quiltimport.sh\n>> @@ -79,7 +79,7 @@ tmp_info=\"$tmp_dir/info\"\n>>  # Find the initial commit\n>>  commit=$(git rev-parse HEAD)\n>>\n>> -mkdir $tmp_dir || exit 2\n>> +mkdir \"$tmp_dir\" || exit 2\n>>  while read patch_name level garbage <&3\n>>  do\n>>  \tcase \"$patch_name\" in ''|'#'*) continue;; esac\n>> @@ -101,7 +101,7 @@ do\n>>  \t\techo \"$patch_name doesn't exist. Skipping.\"\n>>  \t\tcontinue\n>>  \tfi\n>> -\techo $patch_name\n>> +\tprintf '%s\\n' \"$patch_name\"\n>\n>Unquoted \"$patch_name\" in the original fed to \"echo\" is doing more\n>than just backslash 0.\n>\n>    patch_name=\"My   casual patch\"\n>\n>would be split into three tokens, runs of multiple spaces in the\n>original will be squashed into one, and \"My casual patch\" would have\n>been the result, which people may have appreciated as cleaning up a\n>sloppy original patch title.\n>\n>The updated version will give completely different result, losing\n>the \"cleaning up\" feature and parrotting the garbage input to\n>garbage output.\n>\n>So I am not 100% convinced that this change would not result in\n>robbing Peter to pay Paul.\n\npatch_name is read from the series file via:\n\n     while read patch_name level garbage <&3\n\nSince read splits on IFS, patch_name only ever gets the first\nwhitespace delimited token. So in the normal case it can never\ncontain internal whitespace, and the word splitting in echo is a\nnoop:\n\n     $ printf 'my-patch.patch -p1\\n' |\n       while read name level garbage; do\n         echo \"name=[$name]\"\n       done\n     name=[my-patch.patch]\n\nThe only way patch_name gets internal spaces is if the series file\nuses \"\\ \" escaping, but in that case the spaces should be (??)\nintentional.\n\n>Likewise for the next hunk.\n\nSame reasoning applies to the SUBJECT fallback, I think...\n\n     SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')\n\npatch_name is still the same single token from read. And the old path was\nalready quoted:\n\n     echo \"$SUBJECT\"\n\nSo the only issue there was backslash interpretation, rather than splitting?\nsimilar to 47be066026 (\"rebase -i: do not \"echo\" random user-supplied\nstrings\")?\n\nI need a beer.\n\n-- \nThanks,\nSasha\n"}]}