Summary:
We/I broke a couple of things here recently (see D5911) and are doing some work here in general (see D5912, etc.).
Generally, this code is pretty oldschool and not especially well architected for modern application-oriented Phabricator. It hardcodes a lot of stuff which should be applications' responsibilites.
Take the first steps toward making it more solid to reduce the risk here. In particular:
- Factor out the "self mail" and "duplicate mail" checks and add unit tests.
- Make Message-ID hash handling automatic.
Test Plan: Ran unit tests.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5915
Summary: this can happen if you have Phabricator and email lists co-mingling such that Phabricator receives an email multiple times. we can prevent this from then spamming everyone or otherwise taking the action multiple times by storing a message id hash and dropping the message if we have more than one message that matches.
Test Plan: simulated sending the same email multiple times on the command line. noted only the first one made it through.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1726
Differential Revision: https://secure.phabricator.com/D4328
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: See D3252.
Test Plan: This one is nasty to test, I'm going to make some coffee first.
Reviewers: davidreuss, vrana, btrahan
Reviewed By: davidreuss
CC: aran
Maniphest Tasks: T452
Differential Revision: https://secure.phabricator.com/D3254
Summary:
If mails are not sent in UTF-8 we cannot just it verbatim, so we have to
encode it into UTF-8 if it is not the case. Mime headers use different
encodings like "quoted-printable", which we have to handle.
It looks like "Subject: =?iso-8859-1?Q?opr=E6t_s=E5_den_task?=", and can
be decoded by ##iconv_mime_decode##.
Furthermore the body of the email might be in various encodings as well,
which we attempt to pull from the content-type header of the plain text
part of the mail.
Test Plan:
Attempted receiving mails in a variety of flavors. These could be
converted to test-cases once i know if this is a sane solution. Got
expected results from mails sent with Windows-1252 and ISO-8859-1.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, davidreuss
Differential Revision: 1093
Summary: by default it was looking for php to be in /usr/bin/php on some systems
this is incorrect. It should be using /usr/bin/env php for calling php with the
correct path.
Test Plan: patch and try to run.
Reviewers: epriestley
Reviewed By: epriestley
CC: svemir, aran, epriestley
Differential Revision: 1136
Summary:
This makes debugging issues a lot easier, since any exceptions thrown
gets logged and can be seen at /mail/received/, for inspection.
Test Plan:
Created a task via the public-author functionality, and saw no errors
without this patch applied. Even worse, the exception was never shown at
command line either. Not sure what's up with that. Output buffering, or
whatever?
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, davidreuss
Differential Revision: 1042
Summary: There are currently two files, but all scripts require both of them,
which is clearly silly. In the longer term I want to rewrite all of this init
stuff to be more structured (e.g., merge webroot/index.php and __init_script__
better) but this reduces the surface area of the ad-hoc "include files" API we
have now, at least.
Test Plan:
- Grepped for __init_env__.php (no hits)
- Ran a unit test (to test unit changes)
- Ran a daemon (to test daemon changes)
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 976
Summary:
Mail clients can send messages where the body is represented as 'inline'
attachments. Don't treat any such text attachments as actual attachments.
Test Plan:
toulouse, can you verify this fixes the issue?
Reviewed By: toulouse
Reviewers: toulouse
CC: aran, toulouse, epriestley
Differential Revision: 441
Summary:
Allow files to be attached to a task by attaching them to an email reply to the
task.
Test Plan:
Applied this patch live since I haven't managed to get inbound email configured
locally, then attached files to a task via email.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, aran, jungejason
Differential Revision: 369
Summary:
Sendmail isn't actually OK with passing ENV stuff via 'aliases', accept it as an
argument instead.
Test Plan:
Sent real email to a real server, got differential updates!
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 233
Summary:
Provides support for per-user x per-object unique reply-to email addresses, plus
SMTP integration.
This does not actually make Phabricator use these in outbound email.
Test Plan:
Used test console to validate in-Phabricator routing and handling.
Piped emails into the "mail_handler.php" script to validate mail parsing.
Configured sendmail and sent mail to Phabricator.
Technically I haven't conducted all parts of this test on the same machine since
I lost the will to configure more SMTP servers after configuring phabricator.com
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 226
