Page MenuHomePhabricator

Remove PHPMailer code which generates bogus "Message-ID" email headers
ClosedPublic

Authored by epriestley on May 19 2020, 6:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 25, 1:38 AM
Unknown Object (File)
Wed, Apr 17, 2:51 PM
Unknown Object (File)
Sat, Apr 6, 2:49 PM
Unknown Object (File)
Fri, Apr 5, 9:56 PM
Unknown Object (File)
Mar 24 2024, 8:30 PM
Unknown Object (File)
Mar 24 2024, 8:29 PM
Unknown Object (File)
Mar 24 2024, 8:29 PM
Unknown Object (File)
Mar 24 2024, 8:29 PM
Subscribers
None

Details

Summary

See https://discourse.phabricator-community.org/t/how-to-override-localhost-localdomain-in-email-message-id/3876/.

Currently, Phabricator generates a "Message-ID" only in a subset of cases (roughly: when the message is first-in-thread and we expect the thread may have more than one message).

In cases where it does not generate a message ID, it expects the SMTP server to generate one for it. Servers will generally do this, and some ONLY do this (that is, they ignore IDs from Phabricator and replace them). Thus, several pieces of configuration control whether Phabricator attempts to generate a "Message-ID" at all.

The PHPMailer code has fallback behavior which generates a "<random>@localhost.localdomain" message ID. This is never desirable and ignores Phabricator-level configuration that Message IDs should not be generated.

For now, remove this code: it is never the desired behavior and sometimes explicitly contradicts the intent of configuration.

Possibly, a better change may be to make Phabricator always generate a message ID in cases where it isn't forbidden from doing so by configuration. However, that's a more complicated change and it's not clear if/when it would produce better behavior, so start here for now.

Test Plan

Confirmed by affected user (see linked thread).

Diff Detail

Repository
rP Phabricator
Branch
messageid1
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 24475
Build 33725: Run Core Tests
Build 33724: arc lint + arc unit

Event Timeline

This revision was not accepted when it landed; it landed in state Needs Review.May 19 2020, 6:39 PM
This revision was automatically updated to reflect the committed changes.