FAQ
Evening Chaps,

  https://bugs.php.net/bug.php?id=64439

  What do we think of the following approach:

  https://github.com/php/php-src/pull/505

Cheers
Joe

Search Discussions

  • Yasuo Ohgaki at Oct 25, 2013 at 11:26 pm
    Hi Joe,

    Like laruence said, error logs are written to various places.
    Handling only by SAPI will not solve issue, but introduces more
    confusion. (i.e. double escape/conversion)

    I also updated manual page that logs are written to various places.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Joe Watkins at Oct 26, 2013 at 5:43 am

    On 10/26/2013 12:25 AM, Yasuo Ohgaki wrote:
    Hi Joe,

    Like laruence said, error logs are written to various places.
    Handling only by SAPI will not solve issue, but introduces more
    confusion. (i.e. double escape/conversion)

    I also updated manual page that logs are written to various places.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
    4713 case 1: /*send an email */
    4714 if (!php_mail(opt, "PHP error_log message", message,
    headers, NULL TSRMLS_CC)) {
    4715 return FAILURE;
    4716 }
    4717 break;
    4718
    4719 case 2: /*send to an address */
    4720 php_error_docref(NULL TSRMLS_CC, E_WARNING, "TCP/IP
    option not available!");
    4721 return FAILURE;
    4722 break;
    4723
    4724 case 3: /*save to a file */
    4725 stream = php_stream_open_wrapper(opt, "a",
    IGNORE_URL_WIN | REPORT_ERRORS, NULL);
    4726 if (!stream) {
    4727 return FAILURE;
    4728 }
    4729 php_stream_write(stream, message, message_len);
    4730 php_stream_close(stream);
    4731 break;
    4732
    4733 case 4: /* send to SAPI */
    4734 if (sapi_module.log_message) {
    4735 sapi_module.log_message(message TSRMLS_CC);
    4736 } else {
    4737 return FAILURE;
    4738 }
    4739 break;
    4740
    4741 default:
    4742 php_log_err(message TSRMLS_CC);
    4743 break;


    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 ??

    I don't see the need to ignore the problem either.

    Cheers
    Joe
  • Joe Watkins at Oct 27, 2013 at 6:14 am

    On 10/26/2013 11:54 PM, Yasuo Ohgaki wrote:
    Hi Joe,

    On Sun, Oct 27, 2013 at 7:48 AM, Yasuo Ohgaki 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 ??

    Cheers
    Joe
  • Yasuo Ohgaki at Oct 27, 2013 at 7:34 am

    On Sun, Oct 27, 2013 at 3:14 PM, Joe Watkins wrote:

    The patch implements binsafe log for cli and cgi, do we need to implement
    any more ??

    It's better to check & fix all SAPIs :)

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Joe Watkins at Oct 27, 2013 at 7:40 am

    On 10/27/2013 07:33 AM, Yasuo Ohgaki wrote:
    On Sun, Oct 27, 2013 at 3:14 PM, Joe Watkins wrote:

    The patch implements binsafe log for cli and cgi, do we need to implement
    any more ??

    It's better to check & fix all SAPIs :)

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
    Indeed ...

    But the original question I asked was for approval on the approach ...

    I guess I got that ??

    I don't mind implementing other SAPI's at all, I was just wondering if
    the approach is satisfactory ...

    Cheers
    Joe
  • Yasuo Ohgaki at Oct 27, 2013 at 9:25 am
    Hi Joe,
    On Sun, Oct 27, 2013 at 4:40 PM, Joe Watkins wrote:
    On 10/27/2013 07:33 AM, Yasuo Ohgaki wrote:

    On Sun, Oct 27, 2013 at 3:14 PM, Joe Watkins <pthreads@pthreads.org>
    wrote:

    The patch implements binsafe log for cli and cgi, do we need to implement
    any more ??

    It's better to check & fix all SAPIs :)

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net

    Indeed ...
    But the original question I asked was for approval on the approach ...

    I guess I got that ??

    I don't mind implementing other SAPI's at all, I was just wondering if the
    approach is satisfactory ...

    I think approach is ok.

    We should leave receiver how the special characters are treated. Even if
    receiver has problem with null chars, the result is merely a 'truncated
    message' for most cases.

    However, I should mention that some database systems (e.g. Oracle) just
    ignore null char and it enables SQL injection detection bypass. (i.e.
    application firewall bypass) Some databases would not accept null char as
    valid text and refuse to store data. I would say this is not our issue, but
    it's a kind of BC issue.

    There may be many developers against your patch. I would suggest to create
    RFC before start working on other SAPIs.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Joe Watkins at Oct 27, 2013 at 9:56 am

    On 10/27/2013 09:24 AM, Yasuo Ohgaki wrote:
    Hi Joe,
    On Sun, Oct 27, 2013 at 4:40 PM, Joe Watkins wrote:
    On 10/27/2013 07:33 AM, Yasuo Ohgaki wrote:

    On Sun, Oct 27, 2013 at 3:14 PM, Joe Watkins <pthreads@pthreads.org>
    wrote:

    The patch implements binsafe log for cli and cgi, do we need to implement
    any more ??

    It's better to check & fix all SAPIs :)

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net

    Indeed ...
    But the original question I asked was for approval on the approach ...

    I guess I got that ??

    I don't mind implementing other SAPI's at all, I was just wondering if the
    approach is satisfactory ...

    I think approach is ok.

    We should leave receiver how the special characters are treated. Even if
    receiver has problem with null chars, the result is merely a 'truncated
    message' for most cases.

    However, I should mention that some database systems (e.g. Oracle) just
    ignore null char and it enables SQL injection detection bypass. (i.e.
    application firewall bypass) Some databases would not accept null char as
    valid text and refuse to store data. I would say this is not our issue, but
    it's a kind of BC issue.

    There may be many developers against your patch. I would suggest to create
    RFC before start working on other SAPIs.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
    I don't see that it needs an RFC for the SAPI to have a means of logging
    binary safe data.

    The implementations of FPM and Apache2 are things that might require
    RFC, but I don't see that the basic CLI and CGI SAPI's do.

    So it might be best to merge as it is, with cli and cgi implemented for
    reference, and then if someone wants to do the Apache2 and FPM
    implementations they can, along with an RFC for them.

    Cheers
    Joe
  • Yasuo Ohgaki at Oct 27, 2013 at 9:04 pm
    Hi Joe,
    On Sun, Oct 27, 2013 at 6:53 PM, Joe Watkins wrote:

    I don't see that it needs an RFC for the SAPI to have a means of logging
    binary safe data.

    The implementations of FPM and Apache2 are things that might require RFC,
    but I don't see that the basic CLI and CGI SAPI's do.

    So it might be best to merge as it is, with cli and cgi implemented for
    reference, and then if someone wants to do the Apache2 and FPM
    implementations they can, along with an RFC for them.
    We need agreement patch that break compatibility/behavior, thus RFC is
    preferred.

    As long as we keep consistency, we could say it's not our business but it
    is users task handle it properly.

    I would vote some where between -1 to 0 to introduce needless
    inconsistency. Because users need conversion/escape anyway even if we make
    error logging binary safe.

    I'm not against making error logging binary safe, but I don't think we need
    inconsistency.

    Regards,

    --
    Yasuo Ohgaki
    yohgaki@ohgaki.net
  • Julien Pauli at Oct 28, 2013 at 10:51 am

    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<mailto:
    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/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610

    Julien Pauli
  • Tjerk Meesters at Oct 28, 2013 at 11:01 am

    On Mon, Oct 28, 2013 at 6:50 PM, 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
    <mailto:
    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/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
  • Joe Watkins at Oct 28, 2013 at 11:11 am

    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<mailto:
    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/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610

    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.

    Cheers
    Joe
  • Julien Pauli at Oct 28, 2013 at 11:43 am

    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 <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

    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<https://github.com/krakjoe/php-src/commit/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610>

    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
  • Joe Watkins at Oct 28, 2013 at 11:45 am

    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 <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

    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<https://github.com/krakjoe/php-src/commit/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610>

    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
  • Joe Watkins at Oct 28, 2013 at 12:03 pm

    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 <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 >>>>> 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<https://github.com/krakjoe/php-src/commit/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610>


    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
  • Julien Pauli at Oct 28, 2013 at 1:24 pm

    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 <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 >>>>>> 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/**<https://github.com/krakjoe/**php-src/commit/**>
    be5f38ddd449c20230c042aef9757e****fb2ee08188#diff-**
    1a9cfc6173e3a434387996e46086da****56R610<https://github.com/**
    krakjoe/php-src/commit/**be5f38ddd449c20230c042aef9757e**
    fb2ee08188#diff-**1a9cfc6173e3a434387996e46086da**56R610<https://github.com/krakjoe/php-src/commit/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610>

    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<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
  • Joe Watkins at Oct 28, 2013 at 1:34 pm

    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 <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 >>>>>>> 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/**<https://github.com/krakjoe/**php-src/commit/**>
    be5f38ddd449c20230c042aef9757e****fb2ee08188#diff-**
    1a9cfc6173e3a434387996e46086da****56R610<https://github.com/**
    krakjoe/php-src/commit/**be5f38ddd449c20230c042aef9757e**
    fb2ee08188#diff-**1a9cfc6173e3a434387996e46086da**56R610<https://github.com/krakjoe/php-src/commit/be5f38ddd449c20230c042aef9757efb2ee08188#diff-1a9cfc6173e3a434387996e46086da56R610>

    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<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 ...

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedOct 25, '13 at 6:40p
activeOct 28, '13 at 1:34p
posts17
users5
websitephp.net

People

Translate

site design / logo © 2022 Grokbase