threads / discuss / 44415

send-email garbled header with trailing doublequote in email

Subject: send-email garbled header with trailing doublequote in email

## tl;dr

9 messages between Nov 2, 2016 and Nov 4, 2016.

replies: 8people: 3as markdown or json

Andrea Arcangeli· Nov 2, 2016, 20:27 UTC · lore
Hello,

send-email gets confused with one trailing " at the end of the email. Details and how to reproduce below, it breaks also with upstream git version 2.10.2.dirty.

Feel free to CC me if you need any further info.

Thanks, Andrea

----- Forwarded message from Andrea Arcangeli <aarcange@redhat.com> -----
Date: Wed, 2 Nov 2016 21:07:02 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: linux-mm@kvack.org
Subject: Re: [PATCH 00/33] userfaultfd tmpfs/hugetlbfs/non-cooperative
User-Agent: Mutt/1.7.1 (2016-10-04)
FYI: apparently I hit a git bug in this submit... reproducible with
the below command:
git send-email -1 --to '"what ever" <--your--@--email--.com>"'
after replacing --your--@email--.com with your own email.
*snip*
Dry-OK. Log says:
To: "what ever" " <--your--@--email--.com>
*snip*
X-Mailer: git-send-email 2.7.3
Result: OK

It's not ok if the --dry-run outputs the above with a fine header, but the actual header in the email data is different. Of course I tested --dry-run twice and it was fine like the above is fine as well.

*snip*
----- End forwarded message -----
Jeff King· Nov 2, 2016, 21:11 UTC · re: Andrea Arcangeli · lore

Re: send-email garbled header with trailing doublequote in email

On Wed, Nov 02, 2016 at 09:27:09PM +0100, Andrea Arcangeli wrote:
> send-email gets confused with one trailing " at the end of the
> email. Details and how to reproduce below, it breaks also with
> upstream git version 2.10.2.dirty.

I'm not quite sure what happened, and what you wanted to happen. In your example:

> FYI: apparently I hit a git bug in this submit... reproducible with
> the below command:
> 
> git send-email -1 --to '"what ever" <--your--@--email--.com>"'

The "to" address is slightly bogus here because of the extra double-quote. That gets turned into a slightly bogus rfc2822 header:

> *snip*
> Dry-OK. Log says:
> To: "what ever" " <--your--@--email--.com>

Which is funny, but matches what we do with other addresses that are invalid according to the rfc. E.g., there was a discussion recently on:

  Stable <stable@vger.kernel.org> [4.8+]
which has historically been converted to:
  "Stable [4.8+]" <stable@vger.kernel.org>

In fact, it is not even git that does this, but rather what Mail::Address happens to output (though git has fallback routines if that module isn't available that do the same thing).

But in either case, in my test, the actual email address is still extracted correctly and fed to the MTA, so the mail is delivered.

So I'm not sure what you wanted to happen that didn't. Did you want --dry-run to complain about the bogus address? Did the message not get delivered for you? Something else?

-Peff
Andrea Arcangeli· Nov 2, 2016, 21:38 UTC · re: Jeff King · lore

Re: send-email garbled header with trailing doublequote in email

Hello,
On Wed, Nov 02, 2016 at 05:11:18PM -0400, Jeff King wrote:
> In fact, it is not even git that does this, but rather what
> Mail::Address happens to output (though git has fallback routines if
> that module isn't available that do the same thing).

If it's something in perl it should be fixed there indeed. I didn't debug what exactly was wrong, so I sent it here first.

> But in either case, in my test, the actual email address is still
> extracted correctly and fed to the MTA, so the mail is delivered.

The email is delivered to the right email for me too, but to see the problem you need to check the email itself, and look how the To: field looks in the actual email in your mail client or web interface.

Don't you see whatever@yourhostname without spaces and @yourhostname instead of the email domain?

If you still can't reproduce, maybe it's a different perl Mail::Address, I'm using dev-perl/MailTools-2.140.0

From who receives the email it just looks garbled, but --dry-run showed a correct To/Cc list so it was undetectable that it would end up garbled during the real email delivery.

> So I'm not sure what you wanted to happen that didn't. Did you want
> --dry-run to complain about the bogus address? Did the message not get
> delivered for you? Something else?

If --dry-run complained and failed instead of passing and then sending an email with garbled To/Cc list, it'd be more than enough. Either that or it should generate a mail header that matches the output of --dry-run so the review of --dry-run becomes meaningful again.

Thanks, Andrea

Jeff King· Nov 2, 2016, 22:04 UTC · re: Andrea Arcangeli · lore

Re: send-email garbled header with trailing doublequote in email

On Wed, Nov 02, 2016 at 10:38:05PM +0100, Andrea Arcangeli wrote:
Show 9 quoted lines
> > But in either case, in my test, the actual email address is still
> > extracted correctly and fed to the MTA, so the mail is delivered.
> 
> The email is delivered to the right email for me too, but to see the
> problem you need to check the email itself, and look how the To: field
> looks in the actual email in your mail client or web interface.
> 
> Don't you see whatever@yourhostname without spaces and @yourhostname
> instead of the email domain?
Nope, it looks exactly as --dry-run reports it.
To see exactly what is being sent out, try:
-- >8 --

cat >/tmp/foo <<\EOF #!/bin/sh echo "args: $*" sed 's/^/stdin: /' EOF

chmod +x /tmp/foo
git send-email --smtp-server=/tmp/foo --to=whatever
-- 8< --

Mine shows the same header as --dry-run. Which makes sense, because the code is literally just dumping the $header variable, which is the same thing that gets sent to the sendmail binary (or the SMTP server if that is in use).

So my guess would be that either an MTA in the routing path is garbling it (or possibly mailing list software). or maybe even the eventual MUA, though it sounds like you checked the raw bytes.

> If you still can't reproduce, maybe it's a different perl
> Mail::Address, I'm using dev-perl/MailTools-2.140.0
Mine is 2.13, which I would imagine is comparable.
> If --dry-run complained and failed instead of passing and then sending
> an email with garbled To/Cc list, it'd be more than enough. Either
> that or it should generate a mail header that matches the output of
> --dry-run so the review of --dry-run becomes meaningful again.

OK. I think we get that first part right. The problem is that the garbling is such that somebody in the middle is unhappy with it (which makes sense; it's syntactically bogus). So the tool is there to see the problem in --dry-run, but of course it's rather subtle.

In theory we should be able to detect and complain about bogus syntax like this, but right now we don't re-parse the addresses at all. We rely on Mail::Address to produce valid output, and it doesn't seem to be doing so here.

-Peff
Andrea Arcangeli· Nov 2, 2016, 22:29 UTC · re: Jeff King · lore

Re: send-email garbled header with trailing doublequote in email

On Wed, Nov 02, 2016 at 06:04:37PM -0400, Jeff King wrote:
> Nope, it looks exactly as --dry-run reports it.
My sendmail is postfix 3.1.0.
Show 15 quoted lines
> To see exactly what is being sent out, try:
> 
> -- >8 --
> 
> cat >/tmp/foo <<\EOF
> #!/bin/sh
> echo "args: $*"
> sed 's/^/stdin: /'
> EOF
> 
> chmod +x /tmp/foo
> 
> git send-email --smtp-server=/tmp/foo --to=whatever
> 
> -- 8< --
Right it's the same as --dry-run:
stdin: To: "what ever" " <.....>

There's not my hostname and not removed space. If I add more addresses they also go in the second line with a leading space and they're not cut.

So this must be postfix then that out of the blue decided to garble it in a strange way while parsing the input... The removal of all whitespaces s/what ever/whatever/ especially I've no idea how it decided to do so.

Can you reproduce with postfix as sendmail at least? If you can reproduce also see what happens if you add another --to.

Jeff King· Nov 3, 2016, 14:18 UTC · re: Andrea Arcangeli · lore

Re: send-email garbled header with trailing doublequote in email

On Wed, Nov 02, 2016 at 11:29:01PM +0100, Andrea Arcangeli wrote:
Show 7 quoted lines
> So this must be postfix then that out of the blue decided to garble it
> in a strange way while parsing the input... The removal of all
> whitespaces s/what ever/whatever/ especially I've no idea how it
> decided to do so.
> 
> Can you reproduce with postfix as sendmail at least? If you can
> reproduce also see what happens if you add another --to.

Yes, I can easily reproduce without using git at all by installing postfix in local-delivery mode and running:

sendmail peff@sigill.intra.peff.net <<\EOF
From: Jeff King <peff@peff.net>
To: "what ever" " <peff@sigill.intra.peff.net>
Subject: patch

This is the body EOF

Many MTAs do this kind of header-rewriting. I don't necessarily agree with it as a general concept, but the real problem is the syntactically bogus header. The munging that postfix does makes things worse, but I can see why it is confused and does what it does (the whole email is inside a double-quoted portion that is never closed, so it probably thinks there is no hostname portion at all).

So git is possibly at fault for passing along a bogus address. OTOH, the user is perhaps at fault for providing the bogus address to git in the first place. GIGO. :)

I think if any change were to be made, it would be to recognize this bogosity and either clean it up or abort. That ideally would happen via Mail::Address so git does not have to add a bunch of ad-hoc "is this valid rfc822" checks. Reading the manpage for that module, though, it says:

  [we do not handle all of rfc2822]
  Often requests are made to the maintainers of this code improve this
  situation, but this is not a good idea, where it will break zillions
  of existing applications.  If you wish for a fully RFC2822 compliant
  implementation you may take a look at Mail::Message::Field::Full, part
  of MailBox.

So it's possible that switching to a more robust module would improve things.

-Peff
demerphq· Nov 3, 2016, 14:33 UTC · re: Jeff King · lore

Re: send-email garbled header with trailing doublequote in email

On 3 November 2016 at 15:18, Jeff King <peff@peff.net> wrote:
Show 47 quoted lines
> On Wed, Nov 02, 2016 at 11:29:01PM +0100, Andrea Arcangeli wrote:
>
>> So this must be postfix then that out of the blue decided to garble it
>> in a strange way while parsing the input... The removal of all
>> whitespaces s/what ever/whatever/ especially I've no idea how it
>> decided to do so.
>>
>> Can you reproduce with postfix as sendmail at least? If you can
>> reproduce also see what happens if you add another --to.
>
> Yes, I can easily reproduce without using git at all by installing
> postfix in local-delivery mode and running:
>
> sendmail peff@sigill.intra.peff.net <<\EOF
> From: Jeff King <peff@peff.net>
> To: "what ever" " <peff@sigill.intra.peff.net>
> Subject: patch
>
> This is the body
> EOF
>
> Many MTAs do this kind of header-rewriting. I don't necessarily agree
> with it as a general concept, but the real problem is the syntactically
> bogus header. The munging that postfix does makes things worse, but I
> can see why it is confused and does what it does (the whole email is
> inside a double-quoted portion that is never closed, so it probably
> thinks there is no hostname portion at all).
>
> So git is possibly at fault for passing along a bogus address. OTOH, the
> user is perhaps at fault for providing the bogus address to git in the
> first place. GIGO. :)
>
> I think if any change were to be made, it would be to recognize this
> bogosity and either clean it up or abort. That ideally would happen via
> Mail::Address so git does not have to add a bunch of ad-hoc "is this
> valid rfc822" checks. Reading the manpage for that module, though, it
> says:
>
>   [we do not handle all of rfc2822]
>   Often requests are made to the maintainers of this code improve this
>   situation, but this is not a good idea, where it will break zillions
>   of existing applications.  If you wish for a fully RFC2822 compliant
>   implementation you may take a look at Mail::Message::Field::Full, part
>   of MailBox.
>
> So it's possible that switching to a more robust module would improve
> things.

There is an RFC2822 compliant email address validator in the perl test suite if you guys want to use it. We use it to test recursive patterns.

http://perl5.git.perl.org/perl.git/blob/HEAD:/t/re/reg_email.t
Yves
-- 
perl -Mre=debug -e "/just|another|perl|hacker/"
Andrea Arcangeli· Nov 4, 2016, 00:03 UTC · re: Jeff King · lore

Re: send-email garbled header with trailing doublequote in email

On Thu, Nov 03, 2016 at 10:18:48AM -0400, Jeff King wrote:
> bogus header. The munging that postfix does makes things worse, but I
I agree it makes things worse.
> can see why it is confused and does what it does (the whole email is
> inside a double-quoted portion that is never closed, so it probably
> thinks there is no hostname portion at all).

I would see it too, if it actually sent the email to the garbled email address it generated, but it actually got the right email address (because it sent the email to the right address), but it decided to show a different email address in the mail header than the one it sent the email to. But I figure this is the wrong list for such questions :).

> I think if any change were to be made, it would be to recognize this
> bogosity and either clean it up or abort. That ideally would happen via

If postfix continues to do what it does, a strict checking would be my preference of course, assuming it won't break anything that currently works...

If not it's fine too, as nothing particularly "unexpected" happened in git.

At this point double checking that what postfix does is legit is perhaps more worthwhile than adding strictness to git.

Thanks, Andrea

Jeff King· Nov 4, 2016, 00:11 UTC · re: Andrea Arcangeli · lore

Re: send-email garbled header with trailing doublequote in email

On Fri, Nov 04, 2016 at 01:03:51AM +0100, Andrea Arcangeli wrote:
Show 9 quoted lines
> > can see why it is confused and does what it does (the whole email is
> > inside a double-quoted portion that is never closed, so it probably
> > thinks there is no hostname portion at all).
> 
> I would see it too, if it actually sent the email to the garbled email
> address it generated, but it actually got the right email address
> (because it sent the email to the right address), but it decided to
> show a different email address in the mail header than the one it sent
> the email to. But I figure this is the wrong list for such questions :).

It has to do with the "envelope recipient" versus the RFC822 header. Git provides the envelope recipient on the command-line, and that is what is used in the SMTP conversation. In theory the MTA does not need to ever even look at the contents of the message itself. Some do not, but some have features like rewriting the in-message headers (e.g., to turn "to: user" into "to: user@hostname"). You can probably disable that header-rewriting feature in your config, but I don't know very much about configuring postfix.

-Peff

← back to recent threads