CRM-20982 Configure the SMTP mailer to identify itself to the SMTP se…

[kenwest]

…rver in EHLO/HELO commands as: SERVER_NAME when invoked from the web server; else CIVICRM_UF_BASEURL when invoked from the CLI; and falling back to 'localhost' as the last resort.

Overview

Improve the likelihood CiviMails will be delivered. When connecting to some SMTP servers, CiviMail will identify itself in a way that (a) may prevent the server verifying the sender and (b) resembles a spammer. This is a simple fix to improve the way CiviMail identifies itself.

Before

1. Configure emails to be sent using SMTP
2. Send a CiviMail using the command line
3. The mailer will identify itself to the SMTP server as "EHLO localhost"
4. In some cases the SMTP server will reject the email

After

1. Configure emails to be sent using SMTP
2. Send a CiviMail using the command line
3. The mailer will identify itself to the SMTP server as "EHLO example.com"
4. The SMTP server will accept the email

Technical Details

Configure the SMTP mailer to identify itself to the SMTP server in EHLO/HELO commands as: SERVER_NAME when invoked from the web server; else CIVICRM_UF_BASEURL when invoked from the CLI; and falling back to 'localhost' as the last resort.

Comments

In my experience using G Suite for Non-profits, Google rejects these emails when the mailer identifies itself using "EHLO localhost". The problem rights itself when we replace "localhost" with our host name.

[totten]

This is a nicely written PR summary! (OK, maybe I'm biased because I know what EHLO is. But still, thanks for filling that out.)

[seamuslee001]

This seems to make sense to me, Just thinking outloud is there a way to mock this up in a unit test? I don't think that should be a blocker to this PR going through just may help

[kenwest]

I had a look at the unit tests.

• civicrm-core/tests/phpunit/CiviTest/CiviMailUtils.php - sets emails to be written to a DB rather than sent over the wire.
• civicrm-core/tests/phpunit/CRM/Utils/MailTest.php - calls a static function

Any unit test would require ...

• setting the 'mailing_backend' configuration to suit SMTP
• calling CRM_Utils_Mail::createMailer
• checking the attributes of the mailer

I'm not familiar with PHPUnit, so I could modify an existing test, but don't feel competent to write one from scratch.

I assume the PHPUnit tests run via a CLI?

[eileenmcnaughton]

@seamuslee001 this has stalled on the unit test. I would love to see a unit test but at first glance it feels like this is a case where the unit test is relatively hard & the change is potentially one with a lower return on writing the test (it's not in a complex piece of code or in use by multiple pathways).

What are your thoughts on merging it given it has languished for a month?

[seamuslee001]

I think its safe to merge without a unit test, it will be a difficult unit test to write.

[eileenmcnaughton]

Thanks @seamuslee001

[eileenmcnaughton]

& thanks @kenwest for the PR & the rather high-quality write up on it