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 23:06:30 UTC

Re: svn commit: r774884 - /httpd/httpd/trunk/server/log.c

rjung@apache.org wrote:
> Author: rjung
> Date: Thu May 14 19:13:17 2009
> New Revision: 774884
> 
> URL: http://svn.apache.org/viewvc?rev=774884&view=rev
> Log:
> Revert r774755: ap_server_root_relative() for
> 
> piped loggers is nonsense. It doesn't work when
> the logger path contains whitespace.

Ok; not sure what you mean here.  All of the following work today
provided the cwd is the path of httpd root;

ErrorLog "|/opt/apache\ server/bin/rotatelogs logs/foo.log 1M"
ErrorLog "|"c:\program files\apache\bin\rotatelogs" logs/foo.log 1M"
ErrorLog "|bin/rotatelogs logs/foo.log 1M"

Now even if the good intentions of the patch helped to resolve the
program-path, it doesn't help to solve the other argv elts which
represent paths.  The solution is for the cwd to be the server root.

> We already allow relative paths because we open
> the log program via the shell.

:)

> Code was taken from mod_rewrite and mod_log_forensic,
> which need to be still fixed (removing
> ap_server_root_relative() should suffice).

Right.  So here's the deal...

All that forked code in rewrite, log_forensic, dumpio, ssl, etc etc
must die.  Even significant bits of custom_log.

Let's get Joe's patch in, and let me get on that refactoring, because
I think the correct refactorings by Joe and I resolve 95% of the bugs
surrounding the logging schema.

Re: svn commit: r774884 - /httpd/httpd/trunk/server/log.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 14.05.2009 23:06, William A. Rowe, Jr. wrote:
> rjung@apache.org wrote:
>> Author: rjung
>> Date: Thu May 14 19:13:17 2009
>> New Revision: 774884
>>
>> URL: http://svn.apache.org/viewvc?rev=774884&view=rev
>> Log:
>> Revert r774755: ap_server_root_relative() for
>>
>> piped loggers is nonsense. It doesn't work when
>> the logger path contains whitespace.
> 
> Ok; not sure what you mean here.  All of the following work today
> provided the cwd is the path of httpd root;
> 
> ErrorLog "|/opt/apache\ server/bin/rotatelogs logs/foo.log 1M"
> ErrorLog "|"c:\program files\apache\bin\rotatelogs" logs/foo.log 1M"
> ErrorLog "|bin/rotatelogs logs/foo.log 1M"

The line break was an accident, so it was meant as "adding
ap_server_root_relative() in r774755 was nonsense, because it breaks the
working cases when whitespace is involved".

Re: svn commit: r774884 - /httpd/httpd/trunk/server/log.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
William A. Rowe, Jr. wrote:
> rjung@apache.org wrote:
>> Author: rjung
>> Date: Thu May 14 19:13:17 2009
>> New Revision: 774884
>>
>> URL: http://svn.apache.org/viewvc?rev=774884&view=rev
>> Log:
>> Revert r774755: ap_server_root_relative() for
>>
>> piped loggers is nonsense. It doesn't work when
>> the logger path contains whitespace.
> 
> Ok; not sure what you mean here.  All of the following work today
> provided the cwd is the path of httpd root;
> 
> ErrorLog "|/opt/apache\ server/bin/rotatelogs logs/foo.log 1M"
> ErrorLog "|"c:\program files\apache\bin\rotatelogs" logs/foo.log 1M"
> ErrorLog "|bin/rotatelogs logs/foo.log 1M"
> 
> Now even if the good intentions of the patch helped to resolve the
> program-path, it doesn't help to solve the other argv elts which
> represent paths.  The solution is for the cwd to be the server root.
> 
>> We already allow relative paths because we open
>> the log program via the shell.
> 
> :)

Caviat; you might be looking for this API to ensure correct behavior;

http://apr.apache.org/docs/apr/1.3/group__apr__thread__proc.html#g97766f50b0ae083a2f8a28e698a59baa