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....@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@lists.mercurial-scm.org
https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@lists.mercurial-scm.org
https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to