Re: error_log binary unsafe

On Mon, Oct 28, 2013 at 6:50 PM, Julien Pauli <jpauli@php.net> wrote: > On Sun, Oct 27, 2013 at 7:14 AM, Joe Watkins <pthreads@pthreads.org> > 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 > <mailto: > >> yohgaki@ohgaki.net>> wrote: > >> > >> On Sat, Oct 26, 2013 at 2:38 PM, Joe Watkins > >> > >> <pthreads@pthreads.org > >> <mailto:pthreads@pthreads.org>**> 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 <mailto: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/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610 If I'm reading the patch correctly, that should be resolved by updating all php_log_err() references in the rest of the project to use php_log_err_ex() instead. I'm not sure if that was deliberately left out, though. > > > Julien Pauli > -- -- Tjerk