Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically populate the Message-Id header. #81

Closed
sh4d0v1 opened this issue Aug 26, 2019 · 6 comments
Closed

Automatically populate the Message-Id header. #81

sh4d0v1 opened this issue Aug 26, 2019 · 6 comments
Labels
2.enhancement A request for a new feature or altered behaviour. area:message Issue relates to the MIME e-mail Message object representation.

Comments

@sh4d0v1
Copy link

sh4d0v1 commented Aug 26, 2019

Sending mails with marrow.mailer results in mails without message ids (MID) for my own mailserver and a big one I tested against. (while a third mail server adds its own message id)

The (valid) message id I try to set via message._id doesn't get sent.

Mails without a message id get very much higher spam ratings and is against the standard. I see the code referring to generating a message id if there is none set (which I assume to be default) though, so I am unsure why no message id is received.

@sh4d0v1 sh4d0v1 changed the title Can't import Mailer object no message id Aug 26, 2019
@amcgregor
Copy link
Member

amcgregor commented Aug 26, 2019

SMTP servers in general add Message-ID if it is missing. Reference the local_header_rewrite_clients and always_add_missing_headers directives for Postfix. Notably, an ID is generated by Marrow Mailer, albeit it isn't directly used except in logging. I'll admit: that's a bug! However, an easy one to "work around":

Just set the header.

Screen Shot 2019-08-26 at 2 57 41 PM

Note that the above works, correctly, because a conformant ID will be generated automatically and stored ("cached") on first access. Assignment of an explicit _id value will also work. But, notably, the _id (or id) attributes are not utilized by the SMTP transport other than for logging.

@amcgregor amcgregor changed the title no message id Automatically populate the Message-Id header. Aug 26, 2019
@amcgregor amcgregor added 2.enhancement A request for a new feature or altered behaviour. area:message Issue relates to the MIME e-mail Message object representation. labels Aug 26, 2019
amcgregor added a commit that referenced this issue Sep 4, 2019
@amcgregor
Copy link
Member

amcgregor commented Sep 4, 2019

I don't suppose you could give the develop branch a go? It should be entirely compatible with your own fix (test that, too? ;) and render manual application of the Message-Id unnecessary. Building the MIME message (e.g. for SMTP delivery) will automatically add it if missing.

The reasons this is not performed in the constant declaration at the top of prepare_headers is:

  • That I do wish to permit manual specification of that (don't break people's fixes for this long-standing issue).
  • It is not performed much earlier to ensure the ID generation is truly lazy, waiting until the last moment (delivery) to generate if left unspecified.

(Edited to add: apologies for the commit spam there, it's been a long day. Latest run is all good: https://travis-ci.org/marrow/mailer/builds/580962733)

amcgregor added a commit that referenced this issue Sep 4, 2019
@sh4d0v1
Copy link
Author

sh4d0v1 commented Sep 5, 2019

Thank you for the fix.
Are you sure this isn't ready for the master branch? Most people don't pull again without expecting breaking changes anyway (worst case here is two message id headers, so not even a breaking change). Yet in develop branch nobody will recognize it but people will continue building on the master branch and possibly keep running into this issue.

Sidenote: Most SMTP servers should add their own message id if there is not one already there, but it is not default config in postfix, which I guess most - especially the big - mail providers use.

@amcgregor
Copy link
Member

amcgregor commented Sep 5, 2019

Are you sure this isn't ready for the master branch?

There's a release process, I follow it.

…but people will continue building on the master branch…

It is generally good practice to utilize stable, published, release versions. I'm, however, asking the reporter of an issue to check to see if the issue initially reported is actually corrected by the "fix". Otherwise it's not a "fix", it's "just more dead weight", and is absolutely pointless to release, breaking or otherwise. ;P If actually fixed, I can consider rolling a release (after writing a regression test): master branch merge + actual Pypi package release.

Edited to add: and you seem to have given no indication on if you have tested it or not, successfully or not. :/

@sh4d0v1
Copy link
Author

sh4d0v1 commented Sep 5, 2019

Sorry, I misunderstood your former comment.

I tested the develop branch and your fix works. A message id is added to the header without having to append it manually.

@amcgregor
Copy link
Member

amcgregor commented Sep 5, 2019

Awesome, thanks! :D

I'll see what else I can complete for this release, with an estimated release time for this patch (4.0.3) scheduled for Friday. There's a much larger breaking change (breaking at the packaging level itself, not just code using the package) I'll also see about finalizing for this week-end (5.0). I'm aiming for the major bump to be API compatible (no major changes there, just in packaging).

Have a great day!

Edited to add: if you depend on this Message-Id fix, I'd strongly recommend pinning your version numbers a la marrow.mongo~=4.0.3. (4.0.x where x>=3)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.enhancement A request for a new feature or altered behaviour. area:message Issue relates to the MIME e-mail Message object representation.
Projects
None yet
Development

No branches or pull requests

2 participants