Subject: Re: [PATCH v2 3/3] mailinfo: disallow NUL character in mail's header

From: Danh Doan <[email-masked]>
Date: Sun, 19 Apr 2020 21:24:23 +0700

On 2020-04-19 14:30:19+0200, Martin Ågren <[email-masked]> wrote:
> On Sun, 19 Apr 2020 at 13:05, Đoàn Trần Công Danh <[email-masked]> 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