You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2009/05/15 21:46:45 UTC

svn commit: r775300 - in /httpd/httpd/trunk: include/http_log.h server/log.c

Author: wrowe
Date: Fri May 15 19:46:45 2009
New Revision: 775300

URL: http://svn.apache.org/viewvc?rev=775300&view=rev
Log:
Revert the default resource abuse introduced in r104019, which can mask
child logger process failures, by migrating back to 2.0.49 and prior when
rotated logs were reliable on Solaris, etc.

Provides two new piped syntaxes for ErrorLog; "||program" or "|$shellcmd",
where the existing 2.2 behavior may be preserved or the new 2.4 behavior
explicitly elected.  The default should discourage users from "|$" syntax
except as required, in the future.



Modified:
    httpd/httpd/trunk/include/http_log.h
    httpd/httpd/trunk/server/log.c

Modified: httpd/httpd/trunk/include/http_log.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_log.h?rev=775300&r1=775299&r2=775300&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_log.h (original)
+++ httpd/httpd/trunk/include/http_log.h Fri May 15 19:46:45 2009
@@ -272,10 +272,23 @@
  * @param p The pool to allocate out of
  * @param program The program to run in the logging process
  * @return The piped log structure
+ * @tip The log program is invoked as APR_SHELLCMD_ENV, 
+ *      @see ap_open_piped_log_ex to modify this behavior
  */
 AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program);
 
 /**
+ * Open the piped log process specifying the execution choice for program
+ * @param p The pool to allocate out of
+ * @param program The program to run in the logging process
+ * @param cmdtype How to invoke program, e.g. APR_PROGRAM, APR_SHELLCMD_ENV, etc
+ * @return The piped log structure
+ */
+AP_DECLARE(piped_log *) ap_open_piped_log_ex(apr_pool_t *p,
+                                             const char *program,
+                                             apr_cmdtype_e cmdtype);
+
+/**
  * Close the piped log and kill the logging process
  * @param pl The piped log structure
  */

Modified: httpd/httpd/trunk/server/log.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=775300&r1=775299&r2=775300&view=diff
==============================================================================
--- httpd/httpd/trunk/server/log.c (original)
+++ httpd/httpd/trunk/server/log.c Fri May 15 19:46:45 2009
@@ -167,6 +167,8 @@
     char *program;
     /** The pid of the logging process */
     apr_proc_t *pid;
+    /** How to reinvoke program when it must be replaced */
+    apr_cmdtype_e cmdtype;
 #endif
 };
 
@@ -294,7 +296,8 @@
  * stderr for the child will be the same as the stdout of the parent.
  * Otherwise the child will inherit the stderr from the parent. */
 static int log_child(apr_pool_t *p, const char *progname,
-                     apr_file_t **fpin, int dummy_stderr)
+                     apr_file_t **fpin, apr_cmdtype_e cmdtype,
+                     int dummy_stderr)
 {
     /* Child process code for 'ErrorLog "|..."';
      * may want a common framework for this, since I expect it will
@@ -306,8 +309,7 @@
     apr_file_t *errfile;
 
     if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
-        && ((rc = apr_procattr_cmdtype_set(procattr,
-                                           APR_SHELLCMD_ENV)) == APR_SUCCESS)
+        && ((rc = apr_procattr_cmdtype_set(procattr, cmdtype)) == APR_SUCCESS)
         && ((rc = apr_procattr_io_set(procattr,
                                       APR_FULL_BLOCK,
                                       APR_NO_PIPE,
@@ -351,12 +353,26 @@
 
     if (*s->error_fname == '|') {
         apr_file_t *dummy = NULL;
+        apr_cmdtype_e cmdtype = APR_PROGRAM_ENV;
+        fname = s->error_fname + 1;
 
+        /* In 2.4 favor PROGRAM_ENV, accept "||prog" syntax for compatibility
+         * and "|$cmd" to override the default.
+         * Any 2.2 backport would continue to favor SHELLCMD_ENV so there 
+         * accept "||prog" to override, and "|$cmd" to ease conversion.
+         */
+        if (*fname == '|')
+            ++fname;
+        if (*fname == '$') {
+            cmdtype = APR_SHELLCMD_ENV;
+            ++fname;
+        }
+	
         /* 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, s->error_fname + 1, &dummy, is_main);
+        rc = log_child(p, fname, &dummy, cmdtype, is_main);
         if (rc != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
                          "Couldn't start ErrorLog process '%s'.",
@@ -896,12 +912,12 @@
     apr_status_t status;
 
     if (((status = apr_procattr_create(&procattr, pl->p)) != APR_SUCCESS) ||
-        ((status = apr_procattr_cmdtype_set(procattr,
-                                            APR_SHELLCMD_ENV)) != APR_SUCCESS) ||
+        ((status = apr_procattr_cmdtype_set(procattr, pl->cmdtype))
+         != APR_SUCCESS) ||
         ((status = apr_procattr_child_in_set(procattr,
                                              pl->read_fd,
                                              pl->write_fd))
-        != APR_SUCCESS) ||
+         != APR_SUCCESS) ||
         ((status = apr_procattr_child_errfn_set(procattr, log_child_errfn))
          != APR_SUCCESS) ||
         ((status = apr_procattr_error_check_set(procattr, 1)) != APR_SUCCESS)) {
@@ -1019,7 +1035,9 @@
 }
 
 
-AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program)
+AP_DECLARE(piped_log *) ap_open_piped_log_ex(apr_pool_t *p,
+                                             const char *program,
+                                             apr_cmdtype_e cmdtype)
 {
     piped_log *pl;
 
@@ -1027,6 +1045,7 @@
     pl->p = p;
     pl->program = apr_pstrdup(p, program);
     pl->pid = NULL;
+    pl->cmdtype = cmdtype;
     if (apr_file_pipe_create_ex(&pl->read_fd,
                                 &pl->write_fd,
                                 APR_FULL_BLOCK, p) != APR_SUCCESS) {
@@ -1053,13 +1072,15 @@
     return APR_SUCCESS;
 }
 
-AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p, const char *program)
+AP_DECLARE(piped_log *) ap_open_piped_log_ex(apr_pool_t *p,
+                                             const char *program,
+                                             apr_cmdtype_e cmdtype)
 {
     piped_log *pl;
     apr_file_t *dummy = NULL;
     int rc;
 
-    rc = log_child(p, program, &dummy, 0);
+    rc = log_child(p, program, &dummy, cmdtype, 0);
     if (rc != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
                      "Couldn't start piped log process '%s'.",
@@ -1078,6 +1099,12 @@
 
 #endif
 
+AP_DECLARE(piped_log *) ap_open_piped_log(apr_pool_t *p,
+                                          const char *program)
+{
+   return ap_open_piped_log_ex(p, program, APR_PROGRAM_ENV);
+}
+
 AP_DECLARE(void) ap_close_piped_log(piped_log *pl)
 {
     apr_pool_cleanup_run(pl->p, pl, piped_log_cleanup);