You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rj...@apache.org on 2009/05/14 21:13:18 UTC
svn commit: r774884 - /httpd/httpd/trunk/server/log.c
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.
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).
Modified:
httpd/httpd/trunk/server/log.c
Modified: httpd/httpd/trunk/server/log.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=774884&r1=774883&r2=774884&view=diff
==============================================================================
--- httpd/httpd/trunk/server/log.c (original)
+++ httpd/httpd/trunk/server/log.c Thu May 14 19:13:17 2009
@@ -320,24 +320,16 @@
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;
- }
/* Spawn a new child logger. If this is the main server_rec,
* the new child must use a dummy stderr since the current
* stderr might be a pipe to the old logger. Otherwise, the
* child inherits the parents stderr. */
- rc = log_child(p, fname, &dummy, is_main);
+ rc = log_child(p, s->error_fname + 1, &dummy, is_main);
if (rc != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
"Couldn't start ErrorLog process '%s'.",
- fname);
+ s->error_fname + 1);
return DONE;
}
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
Re: svn commit: r774884 - /httpd/httpd/trunk/server/log.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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.