Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:69916 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 34493 invoked from network); 28 Oct 2013 13:34:14 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 28 Oct 2013 13:34:14 -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:3151] helo=localhost.localdomain) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id D5/36-22054-4D76E625 for ; Mon, 28 Oct 2013 08:34:12 -0500 To: internals@lists.php.net,Julien Pauli Message-ID: <526E67D1.1060901@php.net> Date: Mon, 28 Oct 2013 13:34:09 +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: 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 01:23 PM, Julien Pauli wrote: > On Mon, Oct 28, 2013 at 1:03 PM, Joe Watkins wrote: > >> 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>>>>> 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 ... >> > > Knowing little bit of APR, this is "just" a library abstracting the OS. > If there are apr function for binary safe logging, I guess they won't > require a mass effort to implement, there shouldn't need an Apache guru IMO. > The only thing to check, is the APR version they appeared, and if we are > compatible with 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 ... > > > Not sure as well, if we can't turn the logging system to a full binary safe > system, with no BC , then we shouldn't bother patching the 5.x branch and > think about a deeper solution against master, which would then require an > RFC. > > Julien.Pauli > I think I will look into APR and FPM myself and then reply back to the list, don't know when I'll have time to do that ... I'm not worried about the portability of the data loggers, they have existed for a very long time, there should be no problem with using them (they'll be available). But when you look at the parameters, they differ a little, the data loggers take labels, my only concern is for maintaining the output of the logging format exactly. I will look at them and update I guess ... Cheers ...