Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97568
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
Date: Sun, 8 Jan 2017 06:57:16 +0900
Message-ID: <CAGa2bXbgPpO4CfHyHrDDhbXsysk7HDsc17nQoGtWNv4zdaq5kA@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b6246f82630b50545883a9b
Subject: Improving mail() 5th parameter handling
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--047d7b6246f82630b50545883a9b
Content-Type: text/plain; charset=UTF-8

Hi all,

All of us knew details of PHPMailer and Swift Mailer issues with mail()'s
5th (additional_parameters) parameter by now, I suppose. Current behavior
(applying php_escape_shell_cmd to addtional_parameters) is not nice and
similar issue may raise with addtional_parameters in the future.

The issue could be mitigated by allowing array addtional_parameter. It's
basically the same as 4th (addtional_header) parameter change which is
committed by me.

 - Allow array additional_parameter and soft deprecate (document
deprecation) string one.
 - Use key as "option name" and validate chars
 - Use value as "option value" and validate some control chars then apply
escapeshellarg()

Since we cannot assume which shell to be used with sendmail command/how
sendmail command is invoked, this is not complete solution. (This includes
php.ini option setting, i.e. sendmail_path and mail.force_extra_parameters)
This is a mitigation, but it seems we are better to have this to protect
PHP systems.

Any comment for this change?
Or better, is anyone working on this?

Removing 5th option may be good idea also. The most severe BC impact would
be SMTP authentication. If users need SMTP authentication (or any other
options) with sendmail command, mail.force_extra_parameters/sendmail_path
ini setting may be used.

We cannot remove parameter suddenly. We may document deprecation now, raise
warning with 7.2, remove it by 7.3 or 8.0.

Are there comments for removing 5th option?

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--047d7b6246f82630b50545883a9b--