[b4] b4: fix from header not using real name

Commit Message

Justin Stitt July 27, 2023, 6:47 p.m. UTC

When using `b4 send` there seems to be an issue with the real name not
being included in the From: header.

Here's an example:
| From:	[e-mail]

Whereas, something like the following is preferred:
| From: Justin Stitt <[e-mail]>

This patch fixes this issue and achieves the preferred behavior (above)
by properly using both parts of a user's `from` field from their config.

A .gitconfig like this now properly works
| [sendemail]
|   from = Justin Stitt <[e-mail]>

It should be noted that myself and Nick (reported this issue) use an
internal smtpserver called sendgmr. I'm not sure if this is a non-issue
on other smtpservers but the fix I've outlined in this patch looks
universal for locally-ran smtp commands.

Reported-by: Nick Desaulniers <[e-mail]>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
Signed-off-by: Justin Stitt <[e-mail]>
---
 b4/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 099c9b47b39b6076752b8c757872080fad8fae56
change-id: 20230727-fix-from-header-c81940de575b

Best regards,
--
Justin Stitt <[e-mail]>

Comments

Justin Stitt July 27, 2023, 11:07 p.m. UTC | #1

On Thu, Jul 27, 2023 at 06:47:36PM +0000, Justin Stitt wrote:
> When using `b4 send` there seems to be an issue with the real name not
> being included in the From: header.
>
> Here's an example:
> | From:	[e-mail]
>
> Whereas, something like the following is preferred:
> | From: Justin Stitt <[e-mail]>
>
> This patch fixes this issue and achieves the preferred behavior (above)
> by properly using both parts of a user's `from` field from their config.
>
> A .gitconfig like this now properly works
> | [sendemail]
> |   from = Justin Stitt <[e-mail]>
>
> It should be noted that myself and Nick (reported this issue) use an
> internal smtpserver called sendgmr. I'm not sure if this is a non-issue
> on other smtpservers but the fix I've outlined in this patch looks
> universal for locally-ran smtp commands.

I wonder if this breaks other non-sendgmr use cases and as such this
patch should take on a different form. One such idea is checking
specifically for sendgmr usage and adjusting the -f flag as such.

>
> Reported-by: Nick Desaulniers <[e-mail]>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
> Signed-off-by: Justin Stitt <[e-mail]>
> ---
>  b4/__init__.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/b4/__init__.py b/b4/__init__.py
> index b974642..760ba33 100644
> --- a/b4/__init__.py
> +++ b/b4/__init__.py
> @@ -3259,7 +3259,7 @@ def get_smtp(dryrun: bool = False) -> Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL
>          else:
>              envpair = email.utils.parseaddr(fromaddr)
>          if envpair[1]:
> -            smtp += ['-f', envpair[1]]
> +            smtp += ['-f', f"{envpair[0]} <{envpair[1]}>"]
>          return smtp, fromaddr
>
>      encryption = sconfig.get('smtpencryption')
>
> ---
> base-commit: 099c9b47b39b6076752b8c757872080fad8fae56
> change-id: 20230727-fix-from-header-c81940de575b
>
> Best regards,
> --
> Justin Stitt <[e-mail]>
>

Kees Cook July 27, 2023, 11:20 p.m. UTC | #2

On July 27, 2023 11:47:36 AM PDT, Justin Stitt <[e-mail]> wrote:
>When using `b4 send` there seems to be an issue with the real name not
>being included in the From: header.
>
>Here's an example:
>| From:	[e-mail]
>
>Whereas, something like the following is preferred:
>| From: Justin Stitt <[e-mail]>

Hmm. There are actually two "From" fields. There's envelope sender "From" (which is what -f sets), and the in-body "From:" field. I wonder if b4 isn't constructing the in-body From: field, and when missing, the MUAs add it based on the -f flag?

>This patch fixes this issue and achieves the preferred behavior (above)
>by properly using both parts of a user's `from` field from their config.
>
>A .gitconfig like this now properly works
>| [sendemail]
>|   from = Justin Stitt <[e-mail]>

Perhaps what's needed is this (which is what I've been using forever):

[sendemail]
	envelopesender = auto

Nick Desaulniers Aug. 1, 2023, 4:12 p.m. UTC | #3

On Thu, Jul 27, 2023 at 4:20 PM Kees Cook <[e-mail]> wrote:
>
> On July 27, 2023 11:47:36 AM PDT, Justin Stitt <[e-mail]> wrote:
> >When using `b4 send` there seems to be an issue with the real name not
> >being included in the From: header.
> >
> >Here's an example:
> >| From:        [e-mail]
> >
> >Whereas, something like the following is preferred:
> >| From: Justin Stitt <[e-mail]>
>
> Hmm. There are actually two "From" fields. There's envelope sender "From" (which is what -f sets), and the in-body "From:" field. I wonder if b4 isn't constructing the in-body From: field, and when missing, the MUAs add it based on the -f flag?
>
> >This patch fixes this issue and achieves the preferred behavior (above)
> >by properly using both parts of a user's `from` field from their config.
> >
> >A .gitconfig like this now properly works
> >| [sendemail]
> >|   from = Justin Stitt <[e-mail]>
>
> Perhaps what's needed is this (which is what I've been using forever):
>
> [sendemail]
>         envelopesender = auto

Thanks for the tip; I'll give that a shot. Do you have `from` set
under `[sendemail]` as well or no?

>
>
>
> --
> Kees Cook

Kees Cook Aug. 1, 2023, 4:44 p.m. UTC | #4

On Tue, Aug 01, 2023 at 09:12:35AM -0700, Nick Desaulniers wrote:
> On Thu, Jul 27, 2023 at 4:20 PM Kees Cook <[e-mail]> wrote:
> >
> > On July 27, 2023 11:47:36 AM PDT, Justin Stitt <[e-mail]> wrote:
> > >When using `b4 send` there seems to be an issue with the real name not
> > >being included in the From: header.
> > >
> > >Here's an example:
> > >| From:        [e-mail]
> > >
> > >Whereas, something like the following is preferred:
> > >| From: Justin Stitt <[e-mail]>
> >
> > Hmm. There are actually two "From" fields. There's envelope sender "From" (which is what -f sets), and the in-body "From:" field. I wonder if b4 isn't constructing the in-body From: field, and when missing, the MUAs add it based on the -f flag?
> >
> > >This patch fixes this issue and achieves the preferred behavior (above)
> > >by properly using both parts of a user's `from` field from their config.
> > >
> > >A .gitconfig like this now properly works
> > >| [sendemail]
> > >|   from = Justin Stitt <[e-mail]>
> >
> > Perhaps what's needed is this (which is what I've been using forever):
> >
> > [sendemail]
> >         envelopesender = auto
> 
> Thanks for the tip; I'll give that a shot. Do you have `from` set
> under `[sendemail]` as well or no?

I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
not clear to me what's actually going wrong here. It still looks to me
like either "b4 send" isn't writing a "From:" header, or Justin (and
your?) MUA is removing/replacing it when it processes the "-f" argument.

FWIW, my MUA is postfix. (Though actually, I guess, it's the MTA, b4 or
git is the MUA... whatever. The thing I hand off to is postfix.)

For the relevant ~/.gitconfig settings, I have:

[user]
        email = [e-mail]
        name = Kees Cook
[sendemail]
        envelopesender = auto
        confirm = auto

Konstantin Ryabitsev Aug. 1, 2023, 5:04 p.m. UTC | #5

On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> not clear to me what's actually going wrong here. It still looks to me
> like either "b4 send" isn't writing a "From:" header, or Justin (and
> your?) MUA is removing/replacing it when it processes the "-f" argument.

I'm pretty sure this is the side-effect of using sendgmr, which forces the
in-header From: to match what was passed in the -f parameter. It's just
another way in which it differs from /bin/sendmail.

-K

Justin Stitt Aug. 1, 2023, 8:07 p.m. UTC | #6

On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
<[e-mail]> wrote:
>
> On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > not clear to me what's actually going wrong here. It still looks to me
> > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > your?) MUA is removing/replacing it when it processes the "-f" argument.
>
> I'm pretty sure this is the side-effect of using sendgmr, which forces the
> in-header From: to match what was passed in the -f parameter. It's just
> another way in which it differs from /bin/sendmail.

Do you think a patch to b4 specifically catered to `sendgmr` is the
right call here? If not, I can hold onto my patch locally and use it
for my purposes + anyone else who wants it.

>
> -K

Nick Desaulniers Aug. 1, 2023, 10:20 p.m. UTC | #7

On Tue, Aug 1, 2023 at 9:44 AM Kees Cook <[e-mail]> wrote:
>
> On Tue, Aug 01, 2023 at 09:12:35AM -0700, Nick Desaulniers wrote:
> > On Thu, Jul 27, 2023 at 4:20 PM Kees Cook <[e-mail]> wrote:
> > >
> > > On July 27, 2023 11:47:36 AM PDT, Justin Stitt <[e-mail]> wrote:
> > > >When using `b4 send` there seems to be an issue with the real name not
> > > >being included in the From: header.
> > > >
> > > >Here's an example:
> > > >| From:        [e-mail]
> > > >
> > > >Whereas, something like the following is preferred:
> > > >| From: Justin Stitt <[e-mail]>
> > >
> > > Hmm. There are actually two "From" fields. There's envelope sender "From" (which is what -f sets), and the in-body "From:" field. I wonder if b4 isn't constructing the in-body From: field, and when missing, the MUAs add it based on the -f flag?
> > >
> > > >This patch fixes this issue and achieves the preferred behavior (above)
> > > >by properly using both parts of a user's `from` field from their config.
> > > >
> > > >A .gitconfig like this now properly works
> > > >| [sendemail]
> > > >|   from = Justin Stitt <[e-mail]>
> > >
> > > Perhaps what's needed is this (which is what I've been using forever):
> > >
> > > [sendemail]
> > >         envelopesender = auto
> >

Sending via "/usr/bin/sendgmr -i -f auto"
  [PATCH] word-at-a-time: use the same return type for has_zero
regardless of endianness
CRITICAL: Error running /usr/bin/sendgmr -i -f auto: F0801
15:18:46.325579       1 sendgmr.go:277] Send failed: sending email
failed (permanent failure): PERM_FAILURE: PERM_FAIL_SENDER_DENIED With
GMR service:selector "_flex_", the envelope from address can only be
{LDAP}[e-mail].

> > Thanks for the tip; I'll give that a shot. Do you have `from` set
> > under `[sendemail]` as well or no?
>
> I don't, no. I also don't use "b4 send" (just "git send-email"), so it's

Well, we're trying to get that working with sendgmr + b4, not postfix
+ git send email.

> not clear to me what's actually going wrong here. It still looks to me
> like either "b4 send" isn't writing a "From:" header, or Justin (and
> your?) MUA is removing/replacing it when it processes the "-f" argument.
>
> FWIW, my MUA is postfix. (Though actually, I guess, it's the MTA, b4 or
> git is the MUA... whatever. The thing I hand off to is postfix.)
>
> For the relevant ~/.gitconfig settings, I have:
>
> [user]
>         email = [e-mail]
>         name = Kees Cook
> [sendemail]
>         envelopesender = auto
>         confirm = auto
>
>
> --
> Kees Cook

Nick Desaulniers Aug. 1, 2023, 10:25 p.m. UTC | #8

On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
<[e-mail]> wrote:
>
> On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > not clear to me what's actually going wrong here. It still looks to me
> > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > your?) MUA is removing/replacing it when it processes the "-f" argument.
>
> I'm pretty sure this is the side-effect of using sendgmr, which forces the
> in-header From: to match what was passed in the -f parameter. It's just
> another way in which it differs from /bin/sendmail.

Going back and forth between the maintainer of sendgmr here...

I don't think you ever responded to my question of `-t`:
https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9

Specifically NOT REFLECT MODE.

>
> -K

Justin Stitt Aug. 1, 2023, 10:27 p.m. UTC | #9

On Tue, Aug 1, 2023 at 3:25 PM Nick Desaulniers <[e-mail]> wrote:
>
> On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
> <[e-mail]> wrote:
> >
> > On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > > not clear to me what's actually going wrong here. It still looks to me
> > > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > > your?) MUA is removing/replacing it when it processes the "-f" argument.
> >
> > I'm pretty sure this is the side-effect of using sendgmr, which forces the
> > in-header From: to match what was passed in the -f parameter. It's just
> > another way in which it differs from /bin/sendmail.
>
> Going back and forth between the maintainer of sendgmr here...
>
> I don't think you ever responded to my question of `-t`:
> https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
FWIW, I tried manually adding `-t` from inside of b4 and it did _not_
fix the issue. sendgmr still botched the From: header.

>
> Specifically NOT REFLECT MODE.
>
> >
> > -K
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

Nick Desaulniers Aug. 1, 2023, 10:34 p.m. UTC | #10

+Sterling (maintainer of sendgmr)

On Tue, Aug 1, 2023 at 3:27 PM Justin Stitt <[e-mail]> wrote:
>
> On Tue, Aug 1, 2023 at 3:25 PM Nick Desaulniers <[e-mail]> wrote:
> >
> > On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
> > <[e-mail]> wrote:
> > >
> > > On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > > > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > > > not clear to me what's actually going wrong here. It still looks to me
> > > > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > > > your?) MUA is removing/replacing it when it processes the "-f" argument.
> > >
> > > I'm pretty sure this is the side-effect of using sendgmr, which forces the
> > > in-header From: to match what was passed in the -f parameter. It's just
> > > another way in which it differs from /bin/sendmail.
> >
> > Going back and forth between the maintainer of sendgmr here...
> >
> > I don't think you ever responded to my question of `-t`:
> > https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
> FWIW, I tried manually adding `-t` from inside of b4 and it did _not_
> fix the issue. sendgmr still botched the From: header.

Hmm...I should probably reopen our internal ticket for this then...

>
> >
> > Specifically NOT REFLECT MODE.
> >
> > >
> > > -K
> >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers

Nick Desaulniers Aug. 3, 2023, 5:47 p.m. UTC | #11

On Tue, Aug 1, 2023 at 3:27 PM Justin Stitt <[e-mail]> wrote:
>
> On Tue, Aug 1, 2023 at 3:25 PM Nick Desaulniers <[e-mail]> wrote:
> >
> > On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
> > <[e-mail]> wrote:
> > >
> > > On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > > > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > > > not clear to me what's actually going wrong here. It still looks to me
> > > > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > > > your?) MUA is removing/replacing it when it processes the "-f" argument.
> > >
> > > I'm pretty sure this is the side-effect of using sendgmr, which forces the
> > > in-header From: to match what was passed in the -f parameter. It's just
> > > another way in which it differs from /bin/sendmail.
> >
> > Going back and forth between the maintainer of sendgmr here...
> >
> > I don't think you ever responded to my question of `-t`:
> > https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
> FWIW, I tried manually adding `-t` from inside of b4 and it did _not_
> fix the issue. sendgmr still botched the From: header.

Hmm...that was not my experience.  Using the patch to b4 that you sent me:
```
 b4/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/b4/__init__.py b/b4/__init__.py
index b974642..993d6f6 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -3259,7 +3259,7 @@ def get_smtp(dryrun: bool = False) ->
Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL
         else:
             envpair = email.utils.parseaddr(fromaddr)
         if envpair[1]:
-            smtp += ['-f', envpair[1]]
+            smtp += ['-t']
         return smtp, fromaddr

     encryption = sconfig.get('smtpencryption')
```

Here was the recent result:
https://lore.kernel.org/llvm/[e-mail]/
(Note to followers, the very first line has:

From: Nick Desaulniers <[e-mail]>

compare that with v1:
https://lore.kernel.org/all/[e-mail]/

From: [e-mail]

This is a PITA because now patches are getting merged without my name on them.
example: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=79e8328e5acbe691bbde029a52c89d70dcbc22f3

author [e-mail] <[e-mail]>

Konstantine, can you please help?

In particular, I would love if you could address the question about
using `-t` rather than `-f <email addr>`, or provide concrete feedback
to Sterling (cc'ed) on what the expected behavior of the external
mailer is, such that Sterling can then provide feedback on whether
that's acceptable or not.



--
Thanks,
~Nick Desaulniers

Justin Stitt Aug. 3, 2023, 5:51 p.m. UTC | #12

On Thu, Aug 3, 2023 at 10:48 AM Nick Desaulniers
<[e-mail]> wrote:
>
> On Tue, Aug 1, 2023 at 3:27 PM Justin Stitt <[e-mail]> wrote:
> >
> > On Tue, Aug 1, 2023 at 3:25 PM Nick Desaulniers <[e-mail]> wrote:
> > >
> > > On Tue, Aug 1, 2023 at 10:05 AM Konstantin Ryabitsev
> > > <[e-mail]> wrote:
> > > >
> > > > On Tue, Aug 01, 2023 at 09:44:54AM -0700, Kees Cook wrote:
> > > > > I don't, no. I also don't use "b4 send" (just "git send-email"), so it's
> > > > > not clear to me what's actually going wrong here. It still looks to me
> > > > > like either "b4 send" isn't writing a "From:" header, or Justin (and
> > > > > your?) MUA is removing/replacing it when it processes the "-f" argument.
> > > >
> > > > I'm pretty sure this is the side-effect of using sendgmr, which forces the
> > > > in-header From: to match what was passed in the -f parameter. It's just
> > > > another way in which it differs from /bin/sendmail.
> > >
> > > Going back and forth between the maintainer of sendgmr here...
> > >
> > > I don't think you ever responded to my question of `-t`:
> > > https://bugzilla.kernel.org/show_bug.cgi?id=217332#c9
> > FWIW, I tried manually adding `-t` from inside of b4 and it did _not_
> > fix the issue. sendgmr still botched the From: header.
>
> Hmm...that was not my experience.  Using the patch to b4 that you sent me:

Sorry, I meant adding the `-t` flag in conjunction with the existing
`-f` flag did _not_ work. The standalone `-t`
flag certainly works as you've aptly demonstrated. Thanks for confirming.

To elaborate:

Works!
Sending via "/usr/bin/sendgmr -i -t"

Doesn't Work!
Sending via "/usr/bin/sendgmr -i -t -f [e-mail]"

Doesn't Work Either!
Sending via "/usr/bin/sendgmr -i -t -f Justin Stitt [e-mail]"

> ```
>  b4/__init__.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/b4/__init__.py b/b4/__init__.py
> index b974642..993d6f6 100644
> --- a/b4/__init__.py
> +++ b/b4/__init__.py
> @@ -3259,7 +3259,7 @@ def get_smtp(dryrun: bool = False) ->
> Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL
>          else:
>              envpair = email.utils.parseaddr(fromaddr)
>          if envpair[1]:
> -            smtp += ['-f', envpair[1]]
> +            smtp += ['-t']
>          return smtp, fromaddr
>
>      encryption = sconfig.get('smtpencryption')
> ```
>
> Here was the recent result:
> https://lore.kernel.org/llvm/[e-mail]/
> (Note to followers, the very first line has:
>
> From: Nick Desaulniers <[e-mail]>
>
> compare that with v1:
> https://lore.kernel.org/all/[e-mail]/
>
> From: [e-mail]
>
> This is a PITA because now patches are getting merged without my name on them.
> example: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=79e8328e5acbe691bbde029a52c89d70dcbc22f3
>
> author [e-mail] <[e-mail]>
>
> Konstantine, can you please help?
>
> In particular, I would love if you could address the question about
> using `-t` rather than `-f <email addr>`, or provide concrete feedback
> to Sterling (cc'ed) on what the expected behavior of the external
> mailer is, such that Sterling can then provide feedback on whether
> that's acceptable or not.
>
>
>
> --
> Thanks,
> ~Nick Desaulniers

Konstantin Ryabitsev Aug. 3, 2023, 6:17 p.m. UTC | #13

On Thu, Aug 03, 2023 at 10:47:49AM -0700, Nick Desaulniers wrote:
> In particular, I would love if you could address the question about
> using `-t` rather than `-f <email addr>`, or provide concrete feedback
> to Sterling (cc'ed) on what the expected behavior of the external
> mailer is, such that Sterling can then provide feedback on whether
> that's acceptable or not.

Reading the sendmail man page
(https://man7.org/linux/man-pages/man8/sendmail.8.html), I think the proper
way to do it is to:

if sendemail.from has just the email address, like:

    [sendemail]
    from = [e-mail]
    smtpserver = /some/path/sendmail-alike

We should run this command:

    /some/path/sendmail-alike -i -f [e-mail]

If we encounter the following (without envelopesender):

    [sendemail]
    from = Foo Quux <[e-mail]>
    smtpserver = /some/path/sendmail-alike

Then we should use the -F switch to pass the full name:

    /some/path/sendmail-alike -i -F "Foo Quux" -f [e-mail]

I'm not convinced that passing the following will work properly on all
/bin/sendmail implementations:

    /some/path/sendmail-alike -i -f "Foo Quux <[e-mail]>"

I am fairly certain that at least some clients will not properly handle such
situations, because I think -f is only supposed to have the email address, not
the full From-like header.

Now, the question is -- does sendgmr support the -F flag?

-K

Nick Desaulniers Aug. 3, 2023, 6:41 p.m. UTC | #14

On Thu, Aug 3, 2023 at 11:17 AM Konstantin Ryabitsev
<[e-mail]> wrote:
>
> On Thu, Aug 03, 2023 at 10:47:49AM -0700, Nick Desaulniers wrote:
> > In particular, I would love if you could address the question about
> > using `-t` rather than `-f <email addr>`, or provide concrete feedback
> > to Sterling (cc'ed) on what the expected behavior of the external
> > mailer is, such that Sterling can then provide feedback on whether
> > that's acceptable or not.
>
> Reading the sendmail man page
> (https://man7.org/linux/man-pages/man8/sendmail.8.html), I think the proper
> way to do it is to:
>
> if sendemail.from has just the email address, like:
>
>     [sendemail]
>     from = [e-mail]
>     smtpserver = /some/path/sendmail-alike
>
> We should run this command:
>
>     /some/path/sendmail-alike -i -f [e-mail]
>
> If we encounter the following (without envelopesender):
>
>     [sendemail]
>     from = Foo Quux <[e-mail]>
>     smtpserver = /some/path/sendmail-alike
>
> Then we should use the -F switch to pass the full name:
>
>     /some/path/sendmail-alike -i -F "Foo Quux" -f [e-mail]
>
> I'm not convinced that passing the following will work properly on all
> /bin/sendmail implementations:
>
>     /some/path/sendmail-alike -i -f "Foo Quux <[e-mail]>"
>
> I am fairly certain that at least some clients will not properly handle such
> situations, because I think -f is only supposed to have the email address, not
> the full From-like header.
>
> Now, the question is -- does sendgmr support the -F flag?

It does not currently, but we probably could add support for it.

What are your thoughts Konstantin on -t?

Justin Stitt Aug. 3, 2023, 6:41 p.m. UTC | #15

On Thu, Aug 3, 2023 at 11:17 AM Konstantin Ryabitsev
<[e-mail]> wrote:
>
> On Thu, Aug 03, 2023 at 10:47:49AM -0700, Nick Desaulniers wrote:
> > In particular, I would love if you could address the question about
> > using `-t` rather than `-f <email addr>`, or provide concrete feedback
> > to Sterling (cc'ed) on what the expected behavior of the external
> > mailer is, such that Sterling can then provide feedback on whether
> > that's acceptable or not.
>
> Reading the sendmail man page
> (https://man7.org/linux/man-pages/man8/sendmail.8.html), I think the proper
> way to do it is to:
>
> if sendemail.from has just the email address, like:
>
>     [sendemail]
>     from = [e-mail]
>     smtpserver = /some/path/sendmail-alike
>
> We should run this command:
>
>     /some/path/sendmail-alike -i -f [e-mail]
>
> If we encounter the following (without envelopesender):
>
>     [sendemail]
>     from = Foo Quux <[e-mail]>
>     smtpserver = /some/path/sendmail-alike
>
> Then we should use the -F switch to pass the full name:
>
>     /some/path/sendmail-alike -i -F "Foo Quux" -f [e-mail]
>
> I'm not convinced that passing the following will work properly on all
> /bin/sendmail implementations:
>
>     /some/path/sendmail-alike -i -f "Foo Quux <[e-mail]>"
>
> I am fairly certain that at least some clients will not properly handle such
> situations, because I think -f is only supposed to have the email address, not
> the full From-like header.
Working on a patch to address all of these points.

>
> Now, the question is -- does sendgmr support the -F flag?
>
> -K

Konstantin Ryabitsev Aug. 3, 2023, 7:32 p.m. UTC | #16

On Thu, Aug 03, 2023 at 11:41:51AM -0700, Nick Desaulniers wrote:
> > I am fairly certain that at least some clients will not properly handle such
> > situations, because I think -f is only supposed to have the email address, not
> > the full From-like header.
> >
> > Now, the question is -- does sendgmr support the -F flag?
> 
> It does not currently, but we probably could add support for it.

Per our IRC discussion, I think we stumbled on the solution that works for
your case and makes b4 more compatible with git-send-email. Specifically, we
only pass the -f switch if envelopesender is set.

> What are your thoughts Konstantin on -t?

I don't really see the point of it, as we already pass the list of recipients
as part of the command line invocation. The -t switch is just a convenience
flag for humans.

-K

Nick Desaulniers Aug. 3, 2023, 8:58 p.m. UTC | #17

On Thu, Aug 3, 2023 at 12:32 PM Konstantin Ryabitsev
<[e-mail]> wrote:
>
> On Thu, Aug 03, 2023 at 11:41:51AM -0700, Nick Desaulniers wrote:
> > > I am fairly certain that at least some clients will not properly handle such
> > > situations, because I think -f is only supposed to have the email address, not
> > > the full From-like header.
> > >
> > > Now, the question is -- does sendgmr support the -F flag?
> >
> > It does not currently, but we probably could add support for it.
>
> Per our IRC discussion, I think we stumbled on the solution that works for
> your case and makes b4 more compatible with git-send-email. Specifically, we
> only pass the -f switch if envelopesender is set.

Yep, thanks for
https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=8382882b9fb6c50737b6e19198a259850527d2a6
that seems to do it for us.  Please let us know if that gets reverted
and when that ships in a formal release.

We can probably close out
https://bugzilla.kernel.org/show_bug.cgi?id=217332 (I was not able to
do so).

>
> > What are your thoughts Konstantin on -t?
>
> I don't really see the point of it, as we already pass the list of recipients
> as part of the command line invocation. The -t switch is just a convenience
> flag for humans.

Hmm...given the above fix I guess it's kind of moot.

Thanks again for the fix.  Now I can start formally recommending the
usage of b4 more internally/to my colleagues.

I also look forward to `b4 prep --check` for running checkpatch on my series!

Patch

diff --git a/b4/__init__.py b/b4/__init__.py
index b974642..760ba33 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -3259,7 +3259,7 @@  def get_smtp(dryrun: bool = False) -> Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL
         else:
             envpair = email.utils.parseaddr(fromaddr)
         if envpair[1]:
-            smtp += ['-f', envpair[1]]
+            smtp += ['-f', f"{envpair[0]} <{envpair[1]}>"]
         return smtp, fromaddr
 
     encryption = sconfig.get('smtpencryption')