Summary:
See PHI785. Ref T13164. In this case, an install wants to receive mail via Mailgun, but not configure it (DKIM + SPF) for outbound mail.
Allow individual mailers to be marked as not supporting inbound or outbound mail.
Test Plan:
- Added and ran unit tests.
- Went through some mail pathways locally, but I don't have every inbound/outbound configured so this isn't totally conclusive.
- Hit `bin/mail send-test` with a no-outbound mailer.
- I'll hold this until after the release cut so it can soak on `secure` for a bit.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13164
Differential Revision: https://secure.phabricator.com/D19546
Summary:
Ref T13151. See T11767. See PHI686. Although we limit outbound mail text bodies, the limit doesn't currently apply to attachments, HTML bodies, or headers. T11767 discusses improving this in the general case.
In the wild, an install hit an issue (see PHI686) where edits to Phriction pages generate very large HTML bodies. Check and respect the limit when building HTML bodies.
If we don't have enough room for the HTML body, we just drop it. We have the text body to fall back to, and HTML is difficult to truncate safely.
Test Plan: Added unit tests and made them pass.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13151
Differential Revision: https://secure.phabricator.com/D19489
Summary: Ref T13053. Uses the changes in D19026 to escape mail addresses. Those rules may not be right yet, but they're at least all in one place, have test coverage, and aren't obviously incorrect.
Test Plan: Will vet this more extensively when re-testing all mailers.
Maniphest Tasks: T13053
Differential Revision: https://secure.phabricator.com/D19027
Summary:
Ref T13053. Postmark support recommends testing requests against a whitelist of known remote addresses to determine request authenticity. Today, the list can be found here:
<https://postmarkapp.com/support/article/800-ips-for-firewalls>
This is potentially less robust than, e.g., HMAC verification, since they may need to add new datacenters or support IPv6 or something. Users might also have weird network topologies where everything is proxied, and this makes testing/simulating more difficult.
Allow users to configure the list so that they don't need to hack things apart if Postmark adds a new datacenter or remote addresses are unreliable for some other reason, but ship with safe defaults for today.
Test Plan:
Tried to make local requests, got kicked out. Added `0.0.0.0/0` to the list, stopped getting kicked out.
I don't have a convenient way to route real Postmark traffic to my development laptop with an authentic remote address so I haven't verified that the published remote address is legitimate, but I'll vet that in production when I go through all the other mailers.
Maniphest Tasks: T13053
Differential Revision: https://secure.phabricator.com/D19025
Summary: Depends on D19007. Ref T12677.
Test Plan: Used `bin/mail send-test ... --mailer postmark` to deliver some mail via Postmark.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12677
Differential Revision: https://secure.phabricator.com/D19009
Summary:
Depends on D19004. Ref T13053. Ref T12677. If the new `cluster.mailers` is configured, make use of it. Also use it in the Sengrid/Mailgun inbound stuff.
Also fix a bug where "Must Encrypt" mail to no recipients could fatal because no `$mail` was returned.
Test Plan: Processed some mail locally. The testing on this is still pretty flimsy, but I plan to solidify it in an upcoming change.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13053, T12677
Differential Revision: https://secure.phabricator.com/D19005
Summary:
Depends on D19002. Ref T13053. Ref T12677. Adds a new option to allow configuration of multiple mailers.
Nothing actually uses this yet.
Test Plan: Tried to set it to various bad values, got reasonable error messages. Read documentation.
Reviewers: amckinley
Maniphest Tasks: T13053, T12677
Differential Revision: https://secure.phabricator.com/D19003
Summary:
Depends on D19000. Ref T13053. Ref T12677. Currently, most mailers are configured with a bunch of `<mailer>.setting-name` global config options.
This means that you can't configure two different SMTP servers, which is a reasonable thing to want to do in the brave new world of mail failover.
It also means you can't configure two Mailgun accounts or two SES accounts. Although this might seem a little silly, we've had more service disruptions because of policy issues / administrative error (where a particular account was disabled) than actual downtime, so maybe it's not completely ridiculous.
Realign mailers so they can take configuration directly in an explicit way. A later change will add new configuration to take advantage of this and let us move away from having ~10 global options for this stuff eventually.
(This also makes writing third-party mailers easier.)
Test Plan:
Processed some mail, ran existing unit tests. But I wasn't especially thorough.
I expect later changes to provide some tools to make this more testable, so I'll vet each provider more thoroughly and add coverage for multiple mailers after that stuff is ready.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13053, T12677
Differential Revision: https://secure.phabricator.com/D19002
Summary:
Ref T13053. Because I previously misunderstood what "multiplex" means, I used it in various contradictory and inconsistent ways.
We can send mail in two ways: either one mail to everyone with a big "To" and a big "Cc" (not default; better for mailing lists) or one mail to each recipient with just them in "To" (default; better for almost everything else).
"Multiplexing" is combining multiple signals over a single channel, so it more accurately describes the big to/cc. However, it is sometimes used to descibe the other approach. Since it's ambiguous and I've tainted it through misuse, get rid of it and use more clear language.
(There's still some likely misuse in the SMS stuff, and a couple of legitimate uses in other contexts.)
Test Plan: Grepped for `multiplex`, saw less of it.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13053
Differential Revision: https://secure.phabricator.com/D18994
Summary:
We are submitting `epriestley (Evan Priestley) <noreply@meta.phacility.com>`, but should be submitting `"epriestley (Evan Priestley)" <noreply@meta.phacility.com>`.
Add the missing quotes.
Test Plan: Locally, this makes the API calls work against the Mailgun sandbox domain.
Reviewers: chad, amckinley
Reviewed By: chad, amckinley
Differential Revision: https://secure.phabricator.com/D17831
Summary: All classes should extend from some other class. See D13275 for some explanation.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13283
Summary: Fixes T6372. Apparently ye olde error logs get some crazy spam action as is... Looking around at call sites, we do not specify $config (which could specify the supportage of message id header) so it seems correct to default this to something. I went with "true" as the spot we use this seems like pretty easy stuff that will always work.
Test Plan: lots of thinking
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6372
Differential Revision: https://secure.phabricator.com/D10749
Summary:
Fixes T5956. We changed the default mail encoding to `quoted-printable` to fix delivery via SendGrid via SMTP, but this broke multiple other mailers.
- Change the default back to 8bit (which works everywhere except SendGrid).
- Add a configuration setting for selecting `quoted-printable`.
- Document this issue.
- Discourage use of SendGrid in documentation.
(IMPORTANT) @klimek @nickz This reverts the `quoted-printable` fix for SendGrid. You will need to adjust your configurations (set `phpmailer.smtp-encoding` to `quoted-printable`) and restart your daemons or mail will get double newlines again.
Test Plan:
- Sent mail via SendGrid with various `phpmailer.smtp-encoding` settings, saw mail arrive with specified encoding.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: klimek, nickz, epriestley
Maniphest Tasks: T5956
Differential Revision: https://secure.phabricator.com/D10397
Summary:
Added support for side-by-side HTML and plaintext email building.
We can control if the HTML stuff is sent by by a new config, metamta.html-emails
Test Plan:
Been running this in our deployment for a few months now.
====Well behaved clients====
- Gmail
- Mail.app
====Bad clients====
- [[ http://airmailapp.com/ | Airmail ]]. They confuse Gmail too, though.
====Need testing====
- Outlook (Windows + Mac)
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: webframp, taoqiping, chad, epriestley, Korvin
Maniphest Tasks: T992
Differential Revision: https://secure.phabricator.com/D9375
Summary: The adapter was mostly copy-paste, and I missed the supportsMessageIDHeader stuff.
Test Plan: Sent a message, checked headers.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9715
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary:
- Support file attachments in Mailgun, after D8831.
- Fix `bin/mail send-test --attach ...` flag.
- Make `bin/mail send-test` route mail through the daemons.
- Remove the `workerTaskID` on MetaMTAMail, which is only used (needlessly) by `bin/mail resend` and creates a huge mess elsewhere.
- Currently, when mail fails, the daemon exits with a very generic and useless message. Instead, make `sendNow()` throw when it fails, so the real reason is surfaced. This is OK now because mail is always sent via the daemons.
- Now that Mailgun supports attachments, document it.
- Update a bunch of mail docs.
Test Plan:
- Sent mail.
- Sent mail with attachments.
- Read documentation.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8832
Summary:
When sending the "Reply-To" header to Mailgun, Phabricator would
previously send two headers for every "Reply-To": "Reply-To[0][email]" and
"Reply-To[0][name]". Instead, explicitly build the header as specified by RFC
2822 and send it to Mailgun pre-baked.
Pretty sure this bug was a cargo-cult from the Sendgrid code, where (apparently)
this actually works.
Test Plan:
Triggered an email from Phabricator, saw that the header was sent
properly.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8645
Summary:
Ref T2222.
- Restore mail tags for ApplicationTransactions mail.
- Restore subject line verbs.
- Denormalize line count and repository PHID.
- Fix an issue with the mailgun adapter where headers weren't attached properly.
Test Plan: Sent some mail, verified it had correct subjects and tags.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8378
Summary:
As you've suggested, I took the SendGrid code and massaged it until it played nice with Mailgun.
btw - unless I'm missing something, it appears that the SendGrid receiver lets you spoof emails (it performs no validation on the data received).
Test Plan: Opened a task with Mailgun. Felt great.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4326
Differential Revision: https://secure.phabricator.com/D7989
Summary: Until we implement an "enum" type for config, make this a bit harder to get wrong. A user entered "TLS", but the correct value is "tls". The documentation is consistent about this, but the behavior is sitll surprsing.
Test Plan: eyeballed it
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7778
Summary: Currently, adapters can only fail mail temporarily. Allow them to indicate a permanent failure by throwing a special exception.
Test Plan: Added and ran unit tests.
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6847
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary:
See f5c2a2ab4b (commitcomment-2333247)
Copy of working implementation from PHPMailerLite.
Also expose the SSL/TLS options.
Test Plan: Switched to this mailer, configured Gmail SMTP, sent email. Verified email arrived intact.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, mbeck
Differential Revision: https://secure.phabricator.com/D4239
Summary:
Support SMTP as the mailer and user could turn on SMTP authentication if needed.
Import PHPMailer as PHPMailerLite doesn't support SMTP.
Make class PhabricatorMailImplementationPHPMailerAdapter final.
Test Plan: N/A
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2139
Differential Revision: https://secure.phabricator.com/D4063
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary: Last of the big final patches. Left a few debatable classes (12 out of about 400) that I'll deal with individually eventually.
Test Plan: Ran testEverythingImplemented.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1881
Summary:
- Add attachment support for SendGrid.
- Add attachment support to the MetaMTA test console.
Test Plan:
- Sent myself a file with Amazon SES via test console.
- Sent myself a file with SendGrid via test console.
Reviewers: mareksapota, jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 1089
function.
Summary: Fix PhabricatorMailImplementationPHPMailerLiteAdapter to actually use
given parameter.
Test Plan: Use setIsHTML with false as parameter, sent mail should be in plain
text.
Reviewers: jungejason
Reviewed By: jungejason
CC: aran, jungejason, epriestley
Differential Revision: 1001
