All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danh Doan <congdanhqx@gmail.com>
To: "Martin Ågren" <martin.agren@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] mailinfo: disallow NUL character in mail's header
Date: Sun, 19 Apr 2020 21:24:23 +0700	[thread overview]
Message-ID: <20200419142423.GC28207@danh.dev> (raw)
In-Reply-To: <CAN0heSpZR6HtntK80DhQxtRQZo21GBd3uAKrsJxz_opyun8n4g@mail.gmail.com>

On 2020-04-19 14:30:19+0200, Martin Ågren <martin.agren@gmail.com> wrote:
> On Sun, 19 Apr 2020 at 13:05, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> > --- a/t/t4254-am-corrupt.sh
> > +++ b/t/t4254-am-corrupt.sh
> > @@ -69,9 +69,11 @@ test_expect_success "NUL in commit message's body" '
> >         grep "a NUL byte in commit log message not allowed" err
> >  '
> >
> > -test_expect_failure "NUL in commit message's header" '
> > +test_expect_success "NUL in commit message's header" '
> >         test_when_finished "git am --abort" &&
> >         write_nul_patch subject >subject.patch &&
> > +       test_must_fail git mailinfo msg patch <subject.patch 2>err &&
> > +       grep "a NUL byte in Subject is not allowed" err &&
> >         test_must_fail git am subject.patch 2>err &&
> >         grep "a NUL byte in Subject is not allowed" err
> >  '
> 
> We used to fail for some reason and it's sort of clear from the size
> of the test what that reason is. Now that we flip "failure" to "success"
> we can add some more stuff here that works. Makes sense. Of course,
> somewhere is a line for stuffing too much into the test, i.e., you'll
> reach a point where you should have separate tests, but I think this is
> ok.

I'm not really keen to check the "git-mailinfo" failure here.
Since, another developer may come and decide that we should keep the
mailinfo as is, i.e. keep NUL character in Subject, Author, Email,
etc... And let's git-commit complains instead.

Adding a check for git-mailinfo here may limit future development.
Other's feedback is welcomed.

-- 
Danh

  reply	other threads:[~2020-04-19 14:24 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
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 [this message]
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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200419142423.GC28207@danh.dev \
    --to=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=martin.agren@gmail.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.