Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:69914 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 25958 invoked from network); 28 Oct 2013 12:03:19 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 28 Oct 2013 12:03:19 -0000 X-Host-Fingerprint: 149.254.58.248 genkt-058-248.t-mobile.co.uk Received: from [149.254.58.248] ([149.254.58.248:18885] helo=localhost.localdomain) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id B7/05-22054-5825E625 for ; Mon, 28 Oct 2013 07:03:18 -0500 Message-ID: To: internals@lists.php.net Date: Mon, 28 Oct 2013 12:03:14 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 References: <3E.D7.40084.12BBA625@pb1.pair.com> <526B554F.1020606@pthreads.org> <526CAF56.70908@pthreads.org> <526E464B.5030208@php.net> <526E4E53.5010107@php.net> In-Reply-To: <526E4E53.5010107@php.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Posted-By: 149.254.58.248 Subject: Re: [PHP-DEV] error_log binary unsafe From: krakjoe@php.net (Joe Watkins) On 10/28/2013 11:45 AM, Joe Watkins wrote: > On 10/28/2013 11:42 AM, Julien Pauli wrote: >> On Mon, Oct 28, 2013 at 12:11 PM, Joe Watkins wrote: >> >>> On 10/28/2013 10:50 AM, Julien Pauli wrote: >>> >>>> On Sun, Oct 27, 2013 at 7:14 AM, Joe Watkins >>>> wrote: >>>> >>>> On 10/26/2013 11:54 PM, Yasuo Ohgaki wrote: >>>>> >>>>> Hi Joe, >>>>>> >>>>>> On Sun, Oct 27, 2013 at 7:48 AM, Yasuo Ohgaki >>>>> >>>>> yohgaki@ohgaki.net>> wrote: >>>>>> >>>>>> On Sat, Oct 26, 2013 at 2:38 PM, Joe Watkins >>>>>> >>>>>> ****> >>>>>> wrote: >>>>>> >>>>>> Mail is not yet handled, TCP/IP is not supported any more, >>>>>> streams are binary safe. >>>>>> The SAPI and default error logging mechanism are all that >>>>>> require attention. >>>>>> >>>>>> The patch is not final and doesn't include a fix for every >>>>>> implementation of SAPI. >>>>>> >>>>>> I don't see the need for confusion ?? >>>>>> >>>>>> >>>>>> Generally speaking, I'm not against making functions/features >>>>>> binary safe. >>>>>> >>>>>> There are many implementations of syslog/SAPI and not sure >>>>>> if it >>>>>> is good for all. >>>>>> It could cause BC issue also. For example, application like >>>>>> OSSEC >>>>>> HIDS detects >>>>>> possible intrusion by analyzing logs. Patching SAPI may break >>>>>> these applications. >>>>>> >>>>>> I'm not against applying your patch to master, but it's not for >>>>>> released versions. >>>>>> We needs UPGRADE note if the patch is applied. >>>>>> >>>>>> >>>>>> I think it's good for master. >>>>>> Do you have commit karma? >>>>>> If not, I'm willing to merge your patch unless there are not >>>>>> objections. >>>>>> >>>>>> Regards, >>>>>> >>>>>> -- >>>>>> Yasuo Ohgaki >>>>>> yohgaki@ohgaki.net >>>>>> >>>>>> >>>>> I don't have karma ... >>>>> >>>>> There are lots of SAPI's, but this patch wasn't meant to implement >>>>> them >>>>> all, only to provide a route whereby they can implement binary safe >>>>> log_message in the shape of log_message_ex. >>>>> >>>>> The patch implements binsafe log for cli and cgi, do we need to >>>>> implement >>>>> any more ?? >>>>> >>>>> >>>> Joe: Why do you use strlen() ? This leads to the same not binary safe >>>> string, am I wrong ?? >>>> https://github.com/krakjoe/**php-src/commit/** >>>> be5f38ddd449c20230c042aef9757e**fb2ee08188#diff-** >>>> 1a9cfc6173e3a434387996e46086da**56R610 >>>> >>>> >>>> Julien Pauli >>>> >>>> >>> (sorry if you got this twice, my interweb is playing up) >>> >>> Hi Julien, >>> >>> The binary safe logging interface for SAPI is log_message_ex >>> and >>> the binary safe logging function for php is php_log_err_ex >>> The old function must remain, and use string length just as >>> it did >>> before. >>> >> >> I see, that means that the actual patch does not turn logging to binary >> safe logs, but gives functions for that. >> Turning logs to binary safe would then mean tracking every unsafe log >> function (php_log_err()) and turn it to php_log_err_ex() with an explicit >> string length. >> >> >> Julien Pauli >> > > http://lxr.php.net/search?q=&defs=&refs=php_log_err&path=&hist=&project=PHP_5_5 > > > Not the enormous task you imagine it to be :) > > Cheers > Joe Actually, looking at the one occurrence of php_err_log in mysqlnd, it doesn't need updating because it's not being used, the call to mysqlnd_get_backtrace is wrong ... When it is in use, the backtrace provides a length and so we can switch to php_err_log_ex easily. So, there isn't anything else to change, or not neceesarily ... What there is to consider is Apache and FPM: Apaches logging functions don't accept a length because they are formatters. I'm reluctant to touch apache logging at all, it could be a massive BC break. APR has functions for logging data that do accept a length and would be binary safe, however, I've never used them, we want an apache geek really to look at that ... FPM has a custom logging interface (zlog) that doesn't accept a length either, I've not looked too deeply into FPM, I'm not sure, do we have an "fpm" expert to turn to, it never really comes up ?? Other SAPI's: Theese should be updated to use binary safe logging (log_message_ex), I've looked into the most popular, updated the cli and cgi versions for reference. Oh an the mail function, php_mail, also doesn't accept a length ... Not sure where to go from here ... Cheers Joe