From: Junio C Hamano <gitster@pobox.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/3] mailinfo.c::convert_to_utf8: reuse strlen info
Date: Mon, 20 Apr 2020 12:59:37 -0700 [thread overview]
Message-ID: <xmqqv9luq66u.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <e3e542d38818b762f8d6d6b50bc42e01b070772b.1587289680.git.congdanhqx@gmail.com> ("Đoàn Trần Công Danh"'s message of "Sun, 19 Apr 2020 18:00:41 +0700")
Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
> index ddd35498db..98cda32d0a 100755
> --- a/t/t4254-am-corrupt.sh
> +++ b/t/t4254-am-corrupt.sh
> @@ -3,6 +3,36 @@
> test_description='git am with corrupt input'
> . ./test-lib.sh
>
> +write_nul_patch() {
Style: SP on both sides of (), i.e.
write_nul_patch () {
But isn't this misnamed? You are interested in injecting '\0' byte
in the e-mail headers and bodies, not necessarily part of the patch,
but "nul-patch" somehow hints readers that we are writing out a Null
Patch (something that does not do anything, perhaps?).
sample_mbox_with_nul is the best alternative I can come up with
offhand, which is not great either, but at least it does not say
patch.
> + space=' '
> + qNUL=
> + case "$1" in
> + subject) qNUL='=00' ;;
> + esac
Style: case/esac aligns with case arms, i.e.
case "$1" in
subject) qNUL='=00' ;;
esac
> + cat <<-EOF
> + From ec7364544f690c560304f5a5de9428ea3b978b26 Mon Sep 17 00:00:00 2001
> + From: A U Thor <author@example.com>
> + Date: Sun, 19 Apr 2020 13:42:07 +0700
> + Subject: [PATCH] =?ISO-8859-1?q?=C4=CB${qNUL}=D1=CF=D6?=
> + MIME-Version: 1.0
> + Content-Type: text/plain; charset=ISO-8859-1
> + Content-Transfer-Encoding: 8bit
> +
> + EOF
Since the above does have ${qNUL} interpolated, not quoting <<-EOF
is correct. Good.
> + if test "$1" = body
> + then
> + printf "%s\0%s\n" abc def
> + fi
OK. So we won't be able to inject NUL byte in both header and body
at the same time. If you wanted to allow it, you could write
case ",$1," in
*,subject,*) qNUL="=00" ;;
esac
in the early part, and then rewrite this one like so:
case ",$1," in
*,body,*) printf "..." ;;
esac
Then those callers who want to ask for both can say
sample_mbox_with_nul subject,body
> + cat <<-\EOF
> + ---
> + diff --git a/afile b/afile
> + new file mode 100644
> + index 0000000000..e69de29bb2
> + --$space
> + 2.26.1
> + EOF
Doesn't this want to interpolate $space in the output? I think you
want to say <<-EOF, without quoting.
cd t && sh t4254-am-corrupt.sh -d && cat trash*.t4254-*/body.patch
tells me that "--$space" is left in the output, not "-- ".
> +}
> +
> test_expect_success setup '
> # Note the missing "+++" line:
> cat >bad-patch.diff <<-\EOF &&
> @@ -32,4 +62,18 @@ test_expect_success 'try to apply corrupted patch' '
> test_i18ncmp expected actual
> '
>
> +test_expect_success "NUL in commit message's body" '
> + test_when_finished "git am --abort" &&
> + write_nul_patch body >body.patch &&
> + test_must_fail git am body.patch 2>err &&
> + grep "a NUL byte in commit log message not allowed" err
> +'
> +
> +test_expect_failure "NUL in commit message's header" '
> + test_when_finished "git am --abort" &&
> + write_nul_patch subject >subject.patch &&
> + test_must_fail git am subject.patch 2>err &&
> + grep "a NUL byte in Subject is not allowed" err
> +'
> +
> test_done
next prev parent reply other threads:[~2020-04-20 19:59 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-18 3:54 [PATCH] mailinfo.c::convert_to_utf8: reuse strlen info Đoàn Trần Công Danh
2020-04-18 19:56 ` Martin Ågren
2020-04-18 20:18 ` [PATCH 0/6] strbuf: simplify `strbuf_attach()` usage Martin Ågren
2020-04-18 20:18 ` [PATCH 1/6] am: use `strbuf_attach()` correctly Martin Ågren
2020-04-18 20:18 ` [PATCH 2/6] strbuf_attach: correctly pass in `strlen() + 1` for `alloc` Martin Ågren
2020-04-18 20:18 ` [PATCH 3/6] strbuf: use `strbuf_attach()` correctly Martin Ågren
2020-04-18 20:18 ` [PATCH 4/6] fast-import: avoid awkward use of `strbuf_attach()` Martin Ågren
2020-04-18 20:18 ` [PATCH 5/6] rerere: " Martin Ågren
2020-04-18 20:18 ` [PATCH 6/6] strbuf: simplify `strbuf_attach()` usage Martin Ågren
2020-04-19 4:44 ` [PATCH 0/6] " Martin Ågren
2020-04-19 12:32 ` [PATCH 0/4] strbuf: fix doc for `strbuf_attach()` and avoid it Martin Ågren
2020-04-19 12:32 ` [PATCH 1/4] strbuf: fix doc for `strbuf_attach()` Martin Ågren
2020-04-20 17:30 ` Junio C Hamano
2020-04-21 18:44 ` Martin Ågren
2020-04-19 12:32 ` [PATCH 2/4] strbuf: introduce `strbuf_attachstr_len()` Martin Ågren
2020-04-19 12:32 ` [PATCH 3/4] strbuf: introduce `strbuf_attachstr()` Martin Ågren
2020-04-20 19:39 ` Junio C Hamano
2020-04-21 18:47 ` Martin Ågren
2020-04-19 12:32 ` [PATCH 4/4] strbuf_attach: prefer `strbuf_attachstr_len()` Martin Ågren
2020-04-18 23:12 ` [PATCH] mailinfo.c::convert_to_utf8: reuse strlen info Junio C Hamano
2020-04-19 2:48 ` Danh Doan
2020-04-19 4:34 ` Martin Ågren
2020-04-19 5:32 ` Junio C Hamano
2020-04-19 11:00 ` [PATCH v2 0/3] mailinfo: disallow and complains about NUL character Đoàn Trần Công Danh
2020-04-19 11:00 ` [PATCH v2 1/3] t4254: merge 2 steps of a single test Đoàn Trần Công Danh
2020-04-19 12:25 ` Martin Ågren
2020-04-19 14:17 ` Danh Doan
2020-04-19 11:00 ` [PATCH v2 2/3] mailinfo.c::convert_to_utf8: reuse strlen info Đoàn Trần Công Danh
2020-04-19 12:29 ` Martin Ågren
2020-04-19 14:16 ` Danh Doan
2020-04-20 19:59 ` Junio C Hamano [this message]
2020-04-20 23:53 ` Danh Doan
2020-04-19 11:00 ` [PATCH v2 3/3] mailinfo: disallow NUL character in mail's header Đoàn Trần Công Danh
2020-04-19 12:30 ` Martin Ågren
2020-04-19 14:24 ` Danh Doan
2020-04-20 23:54 ` [PATCH v3 0/3] Disallow NUL character in mailinfo Đoàn Trần Công Danh
2020-04-20 23:54 ` [PATCH v3 1/3] t4254: merge 2 steps of a single test Đoàn Trần Công Danh
2020-04-20 23:54 ` [PATCH v3 2/3] mailinfo.c: avoid strlen on strings that can contains NUL Đoàn Trần Công Danh
2020-04-20 23:54 ` [PATCH v3 3/3] mailinfo: disallow NUL character in mail's header Đoàn Trần Công Danh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqv9luq66u.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=congdanhqx@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).