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.

Sign up for GitHub

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

Jump to bottom

Add support for RFC6530, using it only when required. #3000

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+146 −3
Open

Add support for RFC6530, using it only when required. #3000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 66 additions & 1 deletion src/PHPMailer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,14 @@ class PHPMailer
*/
protected $ReplyToQueue = [];

/**
* Whether the need for SMTPUTF8 has been detected. Set by
* preSend() if necessary.
*
* @var bool
*/
public $UseSMTPUTF8 = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One key problem that has stopped me pursuing SMTPUTF8 support in PHPMailer is that you can't tell ahead of time if the server supports it, so while you might provide UTF8 addresses that are valid during message construction, if the server doesn't support it it's not going to work. RFC6531 says the downgrade method is up to the MSA (i.e. PHPMailer), but how it should be handled is not defined, and it's not a simple problem to solve. Unfortunately the only reliable approach is to avoid using it altogether, which is where I've been sitting!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part of 6531 is IMO not quite as well written as it could be.

When you walk through the various cases, the only one where a downgrade is both possible and better than an available alternative is MSA-time downgrade of the sender address. That's not a terribly relevant case for phpmailer (like for for mikel-mail, smtplib, javamail, or Sendgrid, Mailchimp, etc).

IMO, the kind of software that typically calls phpmailer is best served by using noreply@asciidomain as sender address and trusting the caller to supply the actually desired recipient address. Some phpmailer users might be seen as MSAs that could respond to errors by picking a different sender address, but that decision is IMO solidly outside phpmailer's scope.

Perhaps I should comment about how others do it... Ruby, Python and Java all have libraries that raise exceptions when something doesn't work. If the server refuses to play because we don't have the right password, they throw an exception. If the server won't relay because of a misconfiguration, another exception. The javamail/net-smtp/smtplib user is expected to catch the exceptions. I grepped around github to see what callers responded to those exceptions and didn't find any that distinguished between missing SMTPUTF8 and other exceptions.

So if it's bulk mail being generated from a database query, missing SMTPUTF8 will just add the relevant address to the list of undeliverable addresses. If it's a contact form, missing SMTPUTF8 will show up as the same error as if the server rejects the message (e.g., Postfix can reject unknown domains at injection time).


/**
* The array of attachments.
*
Expand Down Expand Up @@ -1363,6 +1371,7 @@ public function getLastMessageID()
* * `pcre` Use old PCRE implementation;
* * `php` Use PHP built-in FILTER_VALIDATE_EMAIL;
* * `html5` Use the pattern given by the HTML5 spec for 'email' type form input elements.
* * `eai` Use a pattern similar to the HTML5 spec for 'email' and to firefox, extended to support EAI (RFC6530).
* * `noregex` Don't use a regex: super fast, really dumb.
* Alternatively you may pass in a callable to inject your own validator, for example:
*
Expand Down Expand Up @@ -1434,6 +1443,17 @@ public static function validateAddress($address, $patternselect = null)
'[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/sD',
$address
);
case 'eai':
/*
* This is the pattern used in the HTML5 spec for validation of 'email' type form input elements, modified to accept unicode email addresses. This is also more lenient than Firefox' html5 spec, in order to make the regex faster.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coincidentally, I wrote the pattern in the HTML5 spec!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, did you? In that case I wouldn't mind having a chat about how it might be updated to cover EAI, if you have a spare half hour sometime. That github issue isn't going anywhere.

*
* @see https://html.spec.whatwg.org/#e-mail-state-(type=email)
*/
return (bool) preg_match(
'/^[a-zA-Z0-9.!#$%&\'*+\/=?^_`{|}~\x80-\xff-]+@[a-zA-Z0-9\x80-\xff](?:[a-zA-Z0-9\x80-\xff-]{0,61}' .
'[a-zA-Z0-9\x80-\xff])?(?:\.[a-zA-Z0-9\x80-\xff](?:[a-zA-Z0-9\x80-\xff-]{0,61}[a-zA-Z0-9\x80-\xff])?)*$/sD',
$address
);
case 'php':
default:
return filter_var($address, FILTER_VALIDATE_EMAIL) !== false;
Expand Down Expand Up @@ -1567,9 +1587,24 @@ public function preSend()
$this->error_count = 0; //Reset errors
$this->mailHeader = '';

//The code below tries to support full use of unicode,
//while remaining compatible with legacy SMTP servers to
//the greatest degree possible: If the message uses
//unicode in the localparts of any addresses, it is sent
//using SMTPUTF8. If not, it it sent using
//pynycode-encoded domains and plain SMTP.
if (static::CHARSET_UTF8 === strtolower($this->CharSet) &&
($this->anyAddressHasUnicodeLocalpart($this->RecipientsQueue) ||
$this->anyAddressHasUnicodeLocalpart(array_keys($this->all_recipients)) ||
$this->anyAddressHasUnicodeLocalpart($this->ReplyToQueue) ||
$this->addressHasUnicodeLocalpart($this->From))) {
$this->UseSMTPUTF8 = true;
}
//Dequeue recipient and Reply-To addresses with IDN
foreach (array_merge($this->RecipientsQueue, $this->ReplyToQueue) as $params) {
$params[1] = $this->punyencodeAddress($params[1]);
if (!$this->UseSMTPUTF8) {
$params[1] = $this->punyencodeAddress($params[1]);
}
call_user_func_array([$this, 'addAnAddress'], $params);
}
if (count($this->to) + count($this->cc) + count($this->bcc) < 1) {
Expand Down Expand Up @@ -2163,6 +2198,7 @@ public function smtpConnect($options = null)
$this->smtp->setDebugLevel($this->SMTPDebug);
$this->smtp->setDebugOutput($this->Debugoutput);
$this->smtp->setVerp($this->do_verp);
$this->smtp->setSMTPUTF8($this->UseSMTPUTF8);
if ($this->Host === null) {
$this->Host = 'localhost';
}
Expand Down Expand Up @@ -4271,6 +4307,35 @@ public static function isValidHost($host)
return filter_var('http://' . $host, FILTER_VALIDATE_URL) !== false;
}

/**
* Check whether the supplied address uses unicode in the localpart.
*
* @return bool
*/
protected function addressHasUnicodeLocalpart($address)
{
return (bool) preg_match( '/[\x80-\xFF].*@/', $address);
arnt marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Check whether any of the supplied addresses use unicode in the
* localpart.
*
* @return bool
*/
protected function anyAddressHasUnicodeLocalpart($addresses)
{
foreach ($addresses as $address) {
if (is_array($address)) {
$address = $address[0];
}
if ($this->addressHasUnicodeLocalpart($address)) {
return true;
}
}
return false;
}

/**
* Get an error message in the current language.
*
Expand Down
42 changes: 40 additions & 2 deletions src/SMTP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,15 @@ class SMTP
*/
public $do_verp = false;

/**
* Whether to use SMTPUTF8.
*
* @see https://www.rfc-editor.org/rfc/rfc6531
*
* @var bool
*/
public $do_smtputf8 = false;

/**
* The timeout value for connection, in seconds.
* Default of 5 minutes (300sec) is from RFC2821 section 4.5.3.2.
Expand Down Expand Up @@ -905,7 +914,15 @@ protected function parseHelloFields($type)
* $from. Returns true if successful or false otherwise. If True
* the mail transaction is started and then one or more recipient
* commands may be called followed by a data command.
* Implements RFC 821: MAIL <SP> FROM:<reverse-path> <CRLF>.
* Implements RFC 821: MAIL <SP> FROM:<reverse-path> <CRLF> and
* two extensions, namely XVERP and SMTPUTF8.
*
* The server's EHLO response is not checked. If use of either
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned this earlier. You can check if the mail server supports an extension by looking in the SMTP::$server_caps array after EHLO has happened, like this:

array_key_exists('SMTPUTF8', $this->server_caps)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is easy, the next line is the question. I considered throwing an exception, but decided against it for the moment.

  • I didn't want to have SMTPUTF8 behave differently from XVERP in that function
  • Changing XVERP would change existing functionality (that noone should depend on, but...)
  • Callers can disable exceptions anyway

I could of course call setError(), like how sendCommand() would do when the MAIL FROM fails. Shall I?

* extensions is enabled even though the server does not support
* that, mail submission will fail.
*
* XVERP is documented at https://www.postfix.org/VERP_README.html
* and SMTPUTF8 is specified in RFC 6531.
*
* @param string $from Source address of this message
*
Expand All @@ -914,10 +931,11 @@ protected function parseHelloFields($type)
public function mail($from)
{
$useVerp = ($this->do_verp ? ' XVERP' : '');
$useSmtputf8 = ($this->do_smtputf8 ? ' SMTPUTF8' : '');

return $this->sendCommand(
'MAIL FROM',
'MAIL FROM:<' . $from . '>' . $useVerp,
'MAIL FROM:<' . $from . '>' . $useSmtputf8 . $useVerp,
250
);
}
Expand Down Expand Up @@ -1352,6 +1370,26 @@ public function getVerp()
return $this->do_verp;
}

/**
* Enable or disable use of SMTPUTF8.
*
* @param bool $enabled
*/
public function setSMTPUTF8($enabled = false)
{
$this->do_smtputf8 = $enabled;
}

/**
* Get SMTPUTF8 use.
*
* @return bool
*/
public function getSMTPUTF8()
{
return $this->do_smtputf8;
}

/**
* Set error messages and codes.
*
Expand Down
40 changes: 40 additions & 0 deletions test/PHPMailer/PHPMailerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,46 @@ public function testDuplicateIDNRemoved()
);
}

/**
* Test SMTPUTF8 usage, including when it is not to be used.
*/
public function testUnsuppoortedSmtpUTF8()
{
$this->Mail->CharSet = PHPMailer::CHARSET_ISO88591;
self::assertFalse($this->Mail->addAddress('spın̈altap@example.com', ''));
}

/**
* Test SMTPUTF8 usage, including when it is not to be used.
*/
public function testSmtpUTF8()
{
//No reason to use SMTPUTF8
$this->Mail->isSMTP();
$this->Mail->addAddress('foo@example.com', '');
$this->Mail->preSend();

//Using a punycoded domain is enough
self::assertFalse($this->Mail->UseSMTPUTF8);
$this->Mail->addAddress('foo@spın̈altap.example', '');
$this->Mail->preSend();
self::assertFalse($this->Mail->UseSMTPUTF8);

//Need to use SMTPUTF8, and can.
$this->Mail->CharSet = PHPMailer::CHARSET_UTF8;
PHPMailer::$validator = 'eai';
self::assertTrue($this->Mail->addAddress('spın̈altap@example.com', ''));
$this->Mail->preSend();
self::assertTrue($this->Mail->UseSMTPUTF8);

//If using SMTPUTF8, then the To header should contain
//unicode@unicode, for better rendering by clients like Mac
//Outlook.
$this->Mail->addAddress('spın̈altap@spın̈altap.invalid', '');
$this->Mail->preSend();
self::assertStringContainsString("spın̈altap@spın̈altap.invalid", $this->Mail->createHeader());
}

/**
* Test SMTP Xclient options
*/
Expand Down