Add DSN mail option #2157
Add DSN mail option #2157
Conversation
It can be used to support the section 4.3 and 4.4 regarding
RET={FULL|HDRS} and ENVID=xyzIDxyz of the RFC:
https://tools.ietf.org/html/rfc3461
|
While this could be useful for some, I see a few issues with this implementation.
|
|
Regarding 1, it follows the same design than the support of recipient() https://github.com/PHPMailer/PHPMailer/blob/master/src/SMTP.php#L912 ; RFC6533 is a work to be done, but it shall be part of another pull request serie. Regarding 2, you are right, I'll think about a better API, close to the support of $dsn (https://github.com/PHPMailer/PHPMailer/blob/master/src/PHPMailer.php#L375). Regarding 3, of course it is widely supported for a while. On the sender side, for instance, postfix does support it too: https://manpages.debian.org/testing/postfix/sendmail.1.en.html |
|
Regarding a DSN/ENVID reference, see: https://github.com/mozilla/releases-comm-central/blob/master/mailnews/compose/src/nsSmtpProtocol.cpp#L149 |
|
Another consideration – the DSN extension to MAIL FROM should only be used if the DSN extension is listed in the server's capabilities, otherwise you're likely to have problems with servers that don't support DSNs. You can check this by calling |
The previous commit added the support of DSN thru a loose support of DSN for the MAIL FROM command. This commit checks that only FULL|HDRS and the optionaly ENVID can be used which prevent any wrong syntax from the users.
|
WIP with this previous commit. I'll provide an update using getServerExt() |
Only DSN capable servers should get the optional MAIL FROM commands. Suggested-by: Marcus Bointon (@Synchro)
|
@Synchro thanks for your comments. See enclosed the updates of this pull request. |
|
Dam'd ! I forgot php-cs-fixer, hold on please. I'll post an update. |
Apply php-cs-fixer --diff --dry-run --verbose fix
|
done |
Add some basic unit tests of the DSN run: php dsn.php
|
@Synchro : please do you have some comments about this serie ? I included the features you requested. thank you, |
|
Please, do you have some comments ? |
src/SMTP.php
Outdated
| @@ -947,7 +947,7 @@ public function mail($from, $ret = '') | |||
| $useVerp = ($this->do_verp ? ' XVERP' : ''); | |||
|
|
|||
| $useRet = ''; | |||
| if (!empty($ret)) { | |||
| if ($this->getServerExt('DSN') && !empty($ret)) { | |||
| $useRet = self::dsnize($ret); | |||
| if (!is_string($useRet)) | |||
There was a problem hiding this comment.
I think it would be better to make this more explicit:
if ($useRet === false)
|
Sorry, lots of other things going on. For the tests, could you reformulate that as tests within the test suite (in |
|
OK, I'll give a try by end of the week. |
It can be used to support the section 4.3 and 4.4 regarding
RET={FULL|HDRS} and ENVID=xyzIDxyz of the RFC:
https://tools.ietf.org/html/rfc3461