Failing SwiftMailer test after upgrade to v3

[driesvints]

Hi all, I hope this is the right place to ask this question as I couldn't immediately find something in the contributing guidelines. Appreciating if anyone could answer a question I have about a failing SwiftMailer test after upgrading to v3.

I've tried patching SwiftMailer to v3 which mostly went super smooth and didn't require any code changes (thanks!). But one test is now failing for EmailValidator v3:

https://github.com/swiftmailer/swiftmailer/blob/master/tests/unit/Swift/Mime/Headers/IdentificationHeaderTest.php#L118

With the following message:

1) Swift_Mime_Headers_IdentificationHeaderTest::testIdRightCanBeDotAtom
Swift_RfcComplianceException: Invalid ID given <a@b.c+&%$.d>

/Users/driesvints/Sites/swiftmailer/lib/classes/Swift/Mime/Headers/IdentificationHeader.php:183
/Users/driesvints/Sites/swiftmailer/lib/classes/Swift/Mime/Headers/IdentificationHeader.php:128
/Users/driesvints/Sites/swiftmailer/lib/classes/Swift/Mime/Headers/IdentificationHeader.php:99
/Users/driesvints/Sites/swiftmailer/tests/unit/Swift/Mime/Headers/IdentificationHeaderTest.php:118

The exception is being thrown here:

https://github.com/swiftmailer/swiftmailer/blob/master/lib/classes/Swift/Mime/Headers/IdentificationHeader.php#L183

I guess my main question is why a@b.c+&%$.d isn't a valid email address anymore in EmailValidator v3? I'm not very familiar with all of the rules from RFC 2822 so I don't immediately see what's wrong here.

[egulias]

Hello @driesvints , it is the right place to ask!
The test is failing because v3 of the validator validates the domain part (after the @) against RFC1035 which is actually what is used in Real Life (r) and generated lots of confusion in user-land (as you can read in the changelog https://github.com/egulias/EmailValidator/blob/3.x/CHANGELOG.md#breacking-changes).
Now, the test is validating an "ID" from the header which can be an email address or any other thing. Similar to symfony/symfony#39685 , which used this validator's use of the more open definition for the domain part to validate other parts of the email (i.e: ID, address list, etc), which is also failing.

I'm working to add that into the next minor version (#195 ) , the challenge is to mix validations of different type of specs.
Meanwhile you can stick to 2.1 as I still give it maintenance.

Hope this helps to solve your question!

[driesvints]

Hey @egulias thank you for the very thorough answer! I'll keep an eye out on that issue and the Symfony PR to see what they'll do. Nothing is urgent here. Thanks for your work! 👍

[cdjenkins]

I added swiftmailer ("swiftmailer/swiftmailer": "^6.0") to my composer.json. This installed version 2.1.25 of your library. Now one of my unit tests fails. Now $validator->assert('d@dcom'); should throw an exception, but now doesn't. Any ideas why?

[driesvints]

@cdjenkins maybe best that you create a separate issue for that so this one stays at the topic on hand?

[egulias]

Hi @cdjenkins , please open a new issue so we keep topics scoped. Happy to help you in the new one.

[cdjenkins]

will do

[cdjenkins]

added my concern as a new issue.

[egulias]

@driesvints Github did it's magic and closed it. v3.1 has been released, please close the issue once you fix Swiftmailer with the new version.
Thanks!

[driesvints]

Hey @egulias. I have no idea how it's possible but the test fails again with the newly tagged v3.1.0.

1) Swift_Mime_Headers_IdentificationHeaderTest::testIdRightCanBeDotAtom
Swift_RfcComplianceException: Invalid ID given <a@b.c+&%$.d>

I see some extra commits have been added to the PR after I tested it. Did something from those break it again?

[driesvints]

I tried again with the exact commit from before I tested and now the test is failing again. I have no idea why it was passing for me before... I'm sure I required the correct PR branch...

[driesvints]

Nvm, false alarm. I forgot to make use of the new class. Everything's working now 😅 Sorry about that.

Thanks again for helping out here!

[driesvints]

Here's the SwiftMailer PR: swiftmailer/swiftmailer#1334

[driesvints]

PR was merged and released.