-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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.
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
DKIM fails without To and ignores CC #1975
Comments
Good catch, thanks |
I solved the second problem by making the comparison case-insensitive in general. The first one is more difficult. Setting |
I tried to set $this->to[] to something but my attempts miserably failed. (It may still be possible though and I may just have missed the right syntax.) I guess that only works if you can set a value "undisclosed-recipients:;" that will be somehow recognised as a permissable To: value AND it has to match the DKIM signed value. |
It's set for SMTP sending on line 2392. I can't see anything that says that the To header must exist, but I'd guess that its absence might be problematic for some, and removing it unilaterally to work around a bug in
|
I think that we can conclude that an empty To: is not forbidden and we currently have no indication this will fail, but the syntax To: undisclosed-recipients:; is also allowed.
|
PHPMailer does set As I said, what you call the empty group doesn't matter; I just did some tests sending BCC-only emails from various places to see whether they include a To header.
One other problem: Spamassassin penalises 1.0 point for missing a To header.
RFC822 says (though not in a normative way):
Their example suggests that To can be omitted altogether, but if it's present it should not be empty; An empty group counts as "one address". RFC2822 describes that a little differently, and also uses an example of an empty |
Ah, I now see the undisclosed-recipients in createHeader(). To be honest, I am ok with an empty To as well as an undisclosed-recipients To, as long as the DKIM is ok. |
You can't add a To header in the message for I may not have understood what you meant – could you fork and make a PR with your suggested change? Going by the RFC, and what other mailers do, I think we should either omit To altogether or use undisclosed-recipients. Using an empty To goes against both of those things, with unknown consequences, so I don't think it's a good idea. |
Mail() wants a $to, $subject, $text and $headers. If mail() requires a $to value and thus creates a To: header then there is no way to leave out To:. This would probably require a To: DKIM header in any case, preventing an added To in transmission. I currently do not see where an empty To would violate RFC's or cause a problem. I did not fully follow the code from createHeader() to presend() and DKIM_add(). What I noticed it that there are separate places where undisclosed-recipients is added. It would conceptually make sense to do this early on in one place. That implies that this does not conflict with the separate handling of To and Subject for mail(). Similarly in an ideal world you would create the complete message with CRLF line endings and only at the very end convert for mail() on non-windows only to CR. Mail() has some differences with direct SMTP but I value the advantages. Many mail providers have a much larger limit for maximum number of mails per day for mail() then they do for SMTP. When you use mail() you do not need to expose the mail credentials in your application code. It is the provider that takes care of it. Mail() can only be accessed on localhost and not remotely, again reducing risks. It is said somewhere that direct SMTP is faster sending multiple mails but is this also true for applications that instanciate a PHPMailer class for every mail? I did find an application that translates undisclosed-recipients to local language. Thunderbird will do that if you specify Bcc without To or Cc on outgoing mail (using SMTP). However with translations you can never win. If I prepare an email on my localized Thunderbird and send it to a non native speaker then he will get my localized "undisclosed-recipients". (Which I guess votes for an empty or missing To.) |
Using
I don't know what happens if you call mail with a If I were to rewrite PHPMailer (which may or may not be happening!), I would probably drop support for You should not instantiate a new PHPMailer instance for sending multiple messages – see the mailing list example for how to do that. My own system can send up to around 200 messages per second using PHPMailer this way. |
With 200 messages per second my provider would block me just after one second for exceeding his max messages limit using SMTP. |
Rate and volume limits are not PHPMailer's concern! That's a higher-level problem to deal with in apps. Try it - most shared hosting that allows |
mail() with $to=null is allowed. This results in an empty To: header. So sending with mail() is not possible without To: header. My provider does not allow SMTP on localhost without specifying credentials. I do not need credentials using mail() so I do not need to expose them in my PHP code. Volume limits are indeed not PHPMailers concern but I have to deal with it when I choose between SMTP or mail() and that is where PHPMailer gets involved. |
That's interesting. It's unusual for providers to allow authentication without encryption. Can you show a transcript? Mail imposes some limits on throughput because it can't support keepalive. It also has a habit of corrupting messages by imposing unnecessarily short line lengths (< 76 chars, see old PHPMailer bugs). |
Unfortunately no transcript. There are limits to what I want to do on this live server. |
OK. That suggests that your local PHP config is not sending mail through localhost but some remote server. We are straying off topic here! Given mail's behaviour with no to address, I think |
The DKIM signature is invalid it an email is sent using mail() that does not have a To: address.
PHPMailer tests for this condition in presend() and creates a header field To: undisclosed recipients; However mail() does not know about this and creates an empty To:. Now the DKIM signature is invalidated.
Solution is to set an empty To: header in presend id mail() and no to addresses:
$this->mailHeader .= $this->headerLine('To', '');
The $autoSignHeaders array contains a "CC" value but the following test is case sensitive and the actual header is "Cc". Now the Cc: field is not signed where it was intended to be signed.
Solution is to change SautoSignHeaders value to "Cc".
The text was updated successfully, but these errors were encountered: