search

msimerson / mail-dmarc

Mail::DMARC, a complete DMARC implementation in Perl
Other
33 stars 23 forks source link

use MIME::Entity and MIME::Parser from MIME::Tools instead of Email::MIME #217

Open mirespace opened 9 months ago

mirespace commented 9 months ago

Hello and Happy New year,

as I raised issue #216 before the Xmas holidays (I was off that period, sorry), here I made the PR for it.

Could you take a look on it and share your opinion on this? Thank you.

mirespace commented 7 months ago

Hi,

After talking to the maintainer of Email::MIME, I understand that this PR could be unnecessary from a security point of view, but I was happy to see progress in exploring the use of this PR (thank you for that, Matt!).

I continue using this approach as:

I know this PR could still be improved if the part of the email composition is also fully switched (from Email::Simple).

Could I do something to help with this?

msimerson commented 7 months ago

it implies fewer dependencies

If one was to just look at this PR, and see one dependency replaced with four, they might conclude otherwise. Perhaps something a bit less squishy such as this table which compares the dependencies to be installed on an otherwise bare OS with only perl installed:

Delta OS Total Dependencies Ubuntu packages from CPAN
Email::MIME Ubuntu 20 N N N
MIME::Tools Ubuntu 20 N N N

Then I'd be inclined to do a similar comparison on another platform (like FreeBSD) and perhaps the results would indicate that making a substantial change like this is worth the risk. (Mail::DMARC is a dependency of SpamAssassin 4.0 so I wouldn't be surprised to see a large increase in installations).

The test coverage for packaging up an email are rather thin. If such a change were deemed advisable, I'd like to see a before-and-after test that compares the packaged email message before and after this PR.

I might also go back and read the latest DMARC RFC (I wrote this module before the RFC was published) and make sure what we're doing is still abiding by the RFC. That's probably out-of-scope for this PR.

marcbradshaw commented 7 months ago

I might also go back and read the latest DMARC RFC (I wrote this module before the RFC was published) and make sure what we're doing is still abiding by the RFC. That's probably out-of-scope for this PR.

DMARCbis is rapidly approaching done, and there will be changes to be made to be compliant with the new spec. But also clearly out of scope for this PR.

marcbradshaw commented 7 months ago

I think both MIME::Tools and Email::MIME are just fine for this use case, what I don't see is a compelling case to justify switching. The case made in #216 was uncertainty over if an issue had been fixed, and we addressed that. It has been fixed, and I'm also confident I could push any other critical fixes through should they arise.