This has worked as far as we can tell. Several people who never got emails before get them now.
However, there is an undesirable side-effect. In the Email Log for the article, the sender column shows as empty: “” <> . Presumably the sender information is being taken from a mail header whose function has changed.
@bdm, what specifically are your config.inc.php settings for:
force_default_envelope_sender
allow_envelope_sender
default_envelope_sender
?
Do you have a single journal setup within this site, or multiple journals? Do you have a “bounce” address setup in Journal Setup, Step 1.4? Are there values in both Name and Email in Journal Setup, Step 1.2?
force_default_envelope_sender and allow_envelope_sender are both On.
default_envelope_sender is set to a fake “dont reply” address in the domain of the server.
There is only one journal at this site. There is no bounce address (for no particular reason).
Emails go out with:
To: the intended recipient
From: the value of default_envelope_sender
Reply-To: the intended sender
Return-Path: the value of default_envelope_sender
I believe the problem is that at the point where the mail is recorded in the mail_log table of the database, the From header is null. Then the Email Log display in the History section of the Section Editor’s view takes the Sender column from the stored From string and thus displays “” <>.
As a workaround, I modified classes/mail/ArticleMailTemplate.inc.php (approx line 118) from $entry->setFrom($this->getFromString(false));
to $entry->setFrom($this->getReplyToString(false));
conditional on force_default_envelope_sender and allow_envelope_sender both On (though making it conditional on the email From header having nothing useful may be more robust).
This works in the cases we have tested, but it’s hard to think of all possible cases.
Your analysis seems sound, but I’m not seeing this issue locally.
The intent of the DMARC/SPF changes was to set the From and Reply-To consistently. There shouldn’t be a point where the Reply-To is set but the From is not.
I wonder if setting the From address is failing for some reason.
Do you have any local modifications to your code?
What is the format of the default_envelope_sender in config.inc.php? Is it: default_envelope_sender = "Noreply" <noreply@server.tld>
or default_envelope_sender = noreply@server.tld
?
We have default_envelope_sender = noreply@server.tld .
I have a suggestion (and please forgive my profound ignorance of the inner workings). In the new version of lib/pkp/classes/mail/Mail.inc.php at #1724 the relevant code is:
if (Config::getVar('email', 'force_default_envelope_sender') && Config::getVar('email', 'default_envelope_sender')) {
// if forcing the envelope sender for DMARC, move the from to a reply-to
$this->addHeader('Return-Path', Config::getVar('email', 'default_envelope_sender'));
if ($from) {
$originalFrom = $this->getFrom();
$this->addReplyTo($originalFrom['email'], $originalFrom['name']);
$from = null;
$this->setFrom(null);
}
$this->addHeader('From', Config::getVar('email', 'default_envelope_sender'));
}
It seems to me that after this code, the From field of the email will have two addresses: the first empty and the second default_envelope_sender. Moreover, the method getFromString() will produce a string version of the first address (namely "" <>), and that’s why "" <> gets stored in the from_address column of the email_log table in the db. I have verified using psql that "" <> is what gets stored, but I didn’t verify the intermediate steps that I’m conjecturing.
In a sense this is academic, because default_envelope_sender would be just as useless as "" <> when displaying the email log for an article. What we want to see in the “sender” column is the editor who sent the reviewing request, the reviewer who replied, etc… I think that by the time the mail is logged in the db, this information is only present in the ReplyTo header.
You may be on to the issue here, but is the line number of 1724 for this code correct? The PR has this added into the Mail::send() function, around line 511.
The idea was that since the we were sending the message, we could rewrite the “From” without concern for other processes, but since the Mail::send hook is called after this, that may be problematic. I don’t know that logging the email message actually depends on the Mail::send hook, but anything that did would not be able to find the sender in the From property.
I see another issue that could be related. When a reviewer responds to an invitation (to accept or decline), the user_id field of the new entry in the event_log table in the db is set to 0. It causes the “User” column of the article Event Log to show nothing except the little envelope image. Clicking on that image brings up an email form with a message “The email address <> is invalid.”
I’m not completely sure this is the same issue because I see a scattering of similar events recorded from before I made the DMARC changes. Also I just recovered from a disk crash where I had to restore a large number of files including the database.