Page MenuHomePhabricator

Refine inbound mail error behaviors, distinguishing between actual mail sender and acting-as sender
Closed, ResolvedPublic


See T12491. After D19018, inbound mail processing looks like this (paraphrased):

$sender = null;
try {
  $receiver = $this->loadReceiver();
  $sender = $receiver->loadSender($this);
  $receiver->validateSender($this, $sender);
} catch (Exception $ex) {
  if ($sender) {
    $this->sendErrorMail($ex, $sender);

This is an improvement on the old behavior, but there are some possible refinements or remaining issues:

We identify the receiver before the sender, so we can no longer ever send "Nothing can process this mail" errors: loadReceiver() will raise an exception before we get a $sender.

That's probably fine for object mail (mail to Txxx or Dxxx, for instance), but may be confusing if someone sends mail to grubs@ when they meant to mail bugs@. Before, we would say "nothing can process this". Now we'll say nothing, even if we recognize their address.

We currently must do this because we need a receiver to call $receiver->loadSender(...), and that logic needs to live on the receiver because application mail can be configured to use a "Default Author" for inbound mail when the address is unrecognized.

This logic should probably look more like this:

$sender = $this->loadExplicitSender();
$receiver = $this->loadReceiver();
if (!$sender) {
  $sender = $receiver->loadImplicitSender();
if (!$sender) {
  throw new Exception(...);

Where loadExplicitSender() looks at the mail itself, and loadImplicitSender() handles the additional identity rules.

This is also tricky because we don't really confirm a sender for object mail (to Txxx+...) in more-secure configurations until we verify that the "To" hash matches the user's mail key, but we currently need to coordinate with the receiver to do this.

If you send mail to bugs@ from an unrecognized address, but the inbound rules are configured to use a real default author (say, @bugreport) and include a bad command (like !porjact instead of !project), we'll send the error mail to @bugreport.

Normally this isn't a problem since they probably have a junk address anyway, but we'd be better off not sending this mail.

So I think there are really three separate ideas that are sort of being conflated here:

  • The tentative sender, as identified by "From" and other headers.
  • The "actor", who we'll process mail on behalf of. This differs when there's no "From" sender, the mail is going to bugs@, and bugs@ is configured to use a default @bugreport user.
  • The confirmed sender, as identified by knowing an object's mail hash. This may be different from the "From" sender, and I think it wins in the case of a disagreement.

It would be nice to refine the handling of these different ideas.

We now send error mail directly to users by PHID, so the "To" on the error mail will be their primary address. This might conceivably be confusing if they sent "From" a secondary address and expected the response to return to that address, but I'd guess this almost never happens and no one will notice or care.

Finally, if a user without an account sends invalid mail to bugs@ they will no longer get a response helping them write the mail correctly. I think there's nothing we can do about this while upholding the "no mail to unverified addresses" rule.

Event Timeline

epriestley triaged this task as Wishlist priority.Feb 7 2018, 1:35 PM
epriestley created this task.
epriestley mentioned this in Unknown Object (Phriction Wiki Document).Feb 10 2018, 1:02 AM

This logic should probably look more like this [differentiating between explicit senders and receive-as-users].

We also need this for T7477.

epriestley renamed this task from Refine inbound mail error behaviors to Refine inbound mail error behaviors, distinguishing between actual mail sender and acting-as sender.Jan 2 2019, 3:15 PM
epriestley moved this task from Far Future to Future on the Mail board.