You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2009/05/14 18:03:13 UTC
Re: svn commit: r774755 - in /httpd/httpd/trunk: CHANGES server/log.c
rjung@apache.org wrote:
> Author: rjung
> Date: Thu May 14 12:59:25 2009
> New Revision: 774755
>
> URL: http://svn.apache.org/viewvc?rev=774755&view=rev
> Log:
> Allow relative pathnames for piped loggers in ErrorLog.
> --- httpd/httpd/trunk/server/log.c (original)
> +++ httpd/httpd/trunk/server/log.c Thu May 14 12:59:25 2009
> @@ -320,16 +320,24 @@
>
> if (*s->error_fname == '|') {
> apr_file_t *dummy = NULL;
> + fname = ap_server_root_relative(p, s->error_fname + 1);
> +
> + if (!fname) {
> + ap_log_error(APLOG_MARK, APLOG_STARTUP, APR_EBADPATH, NULL,
> + "%s: Invalid error log path %s.",
> + ap_server_argv0, s->error_fname);
> + return DONE;
> + }
-1 for bug. This patch presumes "proggy foo bar" is a fspec, it's not,
it is a command line.
It results in "|c:/program files/apache/bin/proggy foo bar" if you are
lucky, but since "bin/proggy foo bar" doesn't really exist, this is all
free to fail, never mind that c:/... isn't properly quoted.
Please revert.
Re: svn commit: r774755 - in /httpd/httpd/trunk: CHANGES server/log.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Rainer Jung wrote:
>
> I checked it and you are totally right. Using ap_server_root_relative()
> breaks whitespace handling. I'll do a separate post about some
> inconsistencies in the path handling between Windows and Unix.
Since paths on unix and windows are inconsistent in the first place,
I'm looking forward to your post ;-)
This had nothing to do with path handling, but confusing an API which
takes a filepath and applying it to a command string :)
Re: svn commit: r774755 - in /httpd/httpd/trunk: CHANGES server/log.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 14.05.2009 18:50, William A. Rowe, Jr. wrote:
> Rainer Jung wrote:
>> The code is identical to what we do in mod_rewrite and mod_log_forensic.
>> I'll check the details, but I didn't commit this in January without
>> testing especially with rotatelogs and params.
>
> Thanks for pointing me at those other bugs! There's little need to dwell
> on the fact that mod_rewrite was entirely unix-centric, and that the
> mod_log_forensic was probably a cut-n-paste from the same. I'll review
> and fix.
I checked it and you are totally right. Using ap_server_root_relative()
breaks whitespace handling. I'll do a separate post about some
inconsistencies in the path handling between Windows and Unix.
Thanks for the heads up!
Re: svn commit: r774755 - in /httpd/httpd/trunk: CHANGES server/log.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Rainer Jung wrote:
>
> The code is identical to what we do in mod_rewrite and mod_log_forensic.
> I'll check the details, but I didn't commit this in January without
> testing especially with rotatelogs and params.
Thanks for pointing me at those other bugs! There's little need to dwell
on the fact that mod_rewrite was entirely unix-centric, and that the
mod_log_forensic was probably a cut-n-paste from the same. I'll review
and fix.
Re: svn commit: r774755 - in /httpd/httpd/trunk: CHANGES server/log.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 14.05.2009 18:03, William A. Rowe, Jr. wrote:
> rjung@apache.org wrote:
>> Author: rjung
>> Date: Thu May 14 12:59:25 2009
>> New Revision: 774755
>>
>> URL: http://svn.apache.org/viewvc?rev=774755&view=rev
>> Log:
>> Allow relative pathnames for piped loggers in ErrorLog.
>
>> --- httpd/httpd/trunk/server/log.c (original)
>> +++ httpd/httpd/trunk/server/log.c Thu May 14 12:59:25 2009
>> @@ -320,16 +320,24 @@
>>
>> if (*s->error_fname == '|') {
>> apr_file_t *dummy = NULL;
>> + fname = ap_server_root_relative(p, s->error_fname + 1);
>> +
>> + if (!fname) {
>> + ap_log_error(APLOG_MARK, APLOG_STARTUP, APR_EBADPATH, NULL,
>> + "%s: Invalid error log path %s.",
>> + ap_server_argv0, s->error_fname);
>> + return DONE;
>> + }
>
> -1 for bug. This patch presumes "proggy foo bar" is a fspec, it's not,
> it is a command line.
>
> It results in "|c:/program files/apache/bin/proggy foo bar" if you are
> lucky, but since "bin/proggy foo bar" doesn't really exist, this is all
> free to fail, never mind that c:/... isn't properly quoted.
>
> Please revert.
The code is identical to what we do in mod_rewrite and mod_log_forensic.
I'll check the details, but I didn't commit this in January without
testing especially with rotatelogs and params.
I'll report my findings.
Rainer