There's another problem with log providers and vhosts and I think I have no idea how to fix it without doing dirty hacks...

The problem is with ap_open_logs function, which does following:

1. Main server log is opened (open_error_log()). If this log uses error log provider, s_main->error_log is set to NULL.

2. When there is no s_main->error_log, stderr is redirected to /dev/null.

3. Error logs for vhosts are opened (another open_error_log()). If there is some problem when opening these logs, any ap_log_error() call is sent to /dev/null.

The same happens for any further error message logged without server_rec *.

I think that what we need is to log to stderr_log only when it points to valid location (real stderr or some real file), but when it's set to /dev/null, we should use main server's error log provider.

I have created patch (attached) which does that, but I'm not satisfied with the way how it works... Maybe someone has better idea how to fix this issue?

Thanks,
Jan Kaluza

On 10/15/2013 05:27 PM, Jeff Trawick wrote:
Does this patch/commit look okay?  It works for me with a simple
provider in different scenarios (vhost that inherits provider setup from
s_main, vhost that has its own setup).

I suppose mod_syslog needs to disallow any attempts to configure it in a
vhost with different settings since openlog() is process-wide.  Jan, can
you look at that by chance?

---------- Forwarded message ----------
From: ** <traw...@apache.org <mailto:traw...@apache.org>>
Date: Tue, Oct 15, 2013 at 10:09 AM
Subject: svn commit: r1532344 - /httpd/httpd/trunk/server/log.c
To: c...@httpd.apache.org <mailto:c...@httpd.apache.org>


Author: trawick
Date: Tue Oct 15 14:09:29 2013
New Revision: 1532344

URL: http://svn.apache.org/r1532344
Log:
Follow-up to r1525597:

Initialize error log providers in vhosts, solving crashes
when logging from those vhosts as well as allowing a different
provider (or provider configuration) for vhosts.

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=1532344&r1=1532343&r2=1532344&view=diff
==============================================================================
--- httpd/httpd/trunk/server/log.c (original)
+++ httpd/httpd/trunk/server/log.c Tue Oct 15 14:09:29 2013
@@ -458,6 +458,18 @@ int ap_open_logs(apr_pool_t *pconf, apr_
                  virt->error_log = q->error_log;
              }
          }
+        else if (virt->errorlog_provider) {
+            /* separately-configured vhost-specific provider */
+            if (open_error_log(virt, 0, p) != OK) {
+                return DONE;
+            }
+        }
+        else if (s_main->errorlog_provider) {
+            /* inherit provider from s_main */
+            virt->errorlog_provider = s_main->errorlog_provider;
+            virt->errorlog_provider_handle =
s_main->errorlog_provider_handle;
+            virt->error_log = NULL;
+        }
          else {
              virt->error_log = s_main->error_log;
          }





--
Born in Roswell... married an alien...
http://emptyhammock.com/

Index: server/log.c
===================================================================
--- server/log.c	(revision 1532978)
+++ server/log.c	(working copy)
@@ -435,9 +435,12 @@
 #define NULL_DEVICE "/dev/null"
 #endif
 
-    if (replace_stderr && freopen(NULL_DEVICE, "w", stderr) == NULL) {
-        ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
-                     "unable to replace stderr with %s", NULL_DEVICE);
+    if (replace_stderr) {
+        if (freopen(NULL_DEVICE, "w", stderr) == NULL) {
+            ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
+                        "unable to replace stderr with %s", NULL_DEVICE);
+        }
+        stderr_log = NULL;
     }
 
     for (virt = s_main->next; virt; virt = virt->next) {
@@ -1032,6 +1035,8 @@
     const request_rec *rmain = NULL;
     core_server_config *sconf = NULL;
     ap_errorlog_info info;
+    ap_errorlog_provider *errorlog_provider = NULL;
+    void *errorlog_provider_handle = NULL;
 
     /* do we need to log once-per-req or once-per-conn info? */
     int log_conn_info = 0, log_req_info = 0;
@@ -1058,6 +1063,10 @@
 #endif
 
         logf = stderr_log;
+        if (!logf && ap_server_conf && ap_server_conf->errorlog_provider) {
+            errorlog_provider = ap_server_conf->errorlog_provider;
+            errorlog_provider_handle = ap_server_conf->errorlog_provider_handle;
+        }
     }
     else {
         int configured_level = r ? ap_get_request_module_loglevel(r, module_index)        :
@@ -1076,6 +1085,9 @@
             logf = s->error_log;
         }
 
+        errorlog_provider = s->errorlog_provider;
+        errorlog_provider_handle = s->errorlog_provider_handle;
+
         /* the faked server_rec from mod_cgid does not have s->module_config */
         if (s->module_config) {
             sconf = ap_get_core_module_config(s->module_config);
@@ -1189,7 +1201,7 @@
             continue;
         }
 
-        if (logf || (s->errorlog_provider->flags &
+        if (logf || (errorlog_provider->flags &
             AP_ERRORLOG_PROVIDER_ADD_EOL_STR)) {
             /* Truncate for the terminator (as apr_snprintf does) */
             if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
@@ -1203,8 +1215,8 @@
             write_logline(errstr, len, logf, level_and_mask);
         }
         else {
-            s->errorlog_provider->writer(&info, s->errorlog_provider_handle,
-                                         errstr, len);
+            errorlog_provider->writer(&info, errorlog_provider_handle,
+                                      errstr, len);
         }
 
         if (done) {

Reply via email to