[PATCH] patchbomb: do not send bare LFs in emails

[Antonio Muci]

Hi Raphaël,

thanks for the guidance, and for putting the change through CI.

I managed to also run the tests locally, via command line, and I confirm 
that the tests are failing for me, too. I should have checked them 
before submitting the change: sorry for that.

I am no expert in Email standards, I've just read Python's documentation 
for the email module (which is detailed, but sometimes not so clear) and 
had a glimpse at the RFCs (5322, 2047). For example RFC 5322 reads: 
"Messages are divided into lines of characters. A line is a series of 
characters that is delimited with the two characters carriage-return and 
line-feed".

I hacked around _process_out_line() in run-tests.py, adding a 
print(out_line) at the start of the function, in order to see the 
captured command output. Before my paths, there are no LFs ("\r"), after 
the change they are there. This is no big news, but I wanted to bypass 
some of the postprocessing run-tests.py performs on command's output.

My current hunches:
- the tests might be wrong, because at least for the email headers we 
can say that they should be separated by "\r\n" and not just "\n";
- the error (552, b'Message contains bare LF and is violating 822.bis 
section 2.3') is coming from my ISP's SMTP server. It seems a reasonable 
error.

I understand that Mercurial and TortoiseHg have been using an email 
based workflow for a long time now, and no one ever reported this 
problem. Yet there are some indications that the current way of 
generating email patches is not standards-compliant.

Do exist anyone in the team who is more knowledgeable in email standards 
and can give us some advice?

Thanks
Antonio


On 14/11/24 00:50, Pierre-Yves David wrote:
>
> Thanks, I imported this to Heptapod here
>
> https://foss.heptapod.net/mercurial/mercurial-devel/-/pipelines/92371
>
> On 11/13/24 22:38, Antonio Muci via Mercurial-devel wrote:
>> # HG changeset patch
>> # User Antonio Muci<a.mux at inwind.it>
>> # Date 1728298075 -7200
>> #      Mon Oct 07 12:47:55 2024 +0200
>> # Branch stable
>> # Node ID d1da72aa71c3dffc4a9002b413105dceed8d56ce
>> # Parent  124c944b71b293057649cb1983ee4a9e0d8eeb37
>> patchbomb: do not send bare LFs in emails
>>
>> Before this change, on some systems, sending emails caused the following error:
>>      abort: (552, b'Message contains bare LF and is violating 822.bis section 2.3')
>>
>> The reason is that Python's default email policy keeps compatibility with Python
>> 3.2, and that version is not standards compliant.
>>
>> The standard library contains a standard compliant email.policy.SMTP class. See
>> https://docs.python.org/3.13/library/email.policy.html#email.policy.SMTP
>>      > Suitable for serializing messages in conformance with the email RFCs. Like
>>      > default, but with linesep set to \r\n, which is RFC compliant.
>>
>> The policy, however, needs to be explicitly opted in.
>>
>> These changes should get rid of the above error when sending email and should be
>> future proof too.
>>
>> Note that we also no longer need to explicitly pass mangle_from_=False, because
>> it is subsumed in the Policy object. See the documentation at
>> https://docs.python.org/3.13/library/email.generator.html#email.generator.Generator
>>      > *mangle_from_* defaults to the value of the mangle_from_ setting of the
>>      > policy (which is True for the compat32 policy and False for all others)
>>
>> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>> --- a/hgext/patchbomb.py
>> +++ b/hgext/patchbomb.py
>> @@ -78,6 +78,7 @@ import email.encoders as emailencoders
>>   import email.mime.base as emimebase
>>   import email.mime.multipart as emimemultipart
>>   import email.utils as eutil
>> +import email.policy as epolicy
>>   import os
>>   import socket
>>   
>> @@ -985,7 +986,7 @@ def email(ui, repo, *revs, **opts):
>>           if opts.get(b'test'):
>>               ui.status(_(b'displaying '), subj, b' ...\n')
>>               ui.pager(b'email')
>> -            generator = mail.Generator(ui, mangle_from_=False)
>> +            generator = mail.Generator(ui, policy=epolicy.SMTP)
>>               try:
>>                   generator.flatten(m, False)
>>                   ui.write(b'\n')
>> @@ -1000,7 +1001,7 @@ def email(ui, repo, *revs, **opts):
>>                   # Exim does not remove the Bcc field
>>                   del m['Bcc']
>>               fp = stringio()
>> -            generator = mail.Generator(fp, mangle_from_=False)
>> +            generator = mail.Generator(fp, policy=epolicy.SMTP)
>>               generator.flatten(m, False)
>>               alldests = to + bcc + cc
>>               sendmail(sender_addr, alldests, fp.getvalue())
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at lists.mercurial-scm.org
>> https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
> -- 
> Pierre-Yves David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20241115/1c45bb6b/attachment.htm>