From: Nick Kew <niq@apache.org>
Date: Sun, 4 Jan 2009 20:52:41 +0000 (+0000)
Subject: Fix mod_ext_filter to detect failure to start the external program,
X-Git-Tag: 2.3.2~226
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4d61be3c2f43f918ad9f2ce59784b56c71eb1301;p=apache

Fix mod_ext_filter to detect failure to start the external program,
and add configuration option to abort or continue.
PR 41120


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@731358 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index ed00c4c342..3e6e7c382b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,12 @@
 Changes with Apache 2.3.2
 [ When backported to 2.2.x, remove entry from this file ]
 
+ *) mod_ext_filter: We need to detect failure to startup the filter
+    program (a mangled response is not acceptable).  Fix to detect
+    failure, and offer configuration option either to abort or
+    to remove the filter and continue.
+    PR 41120 [Nick Kew]
+
  *) mod_session_crypto: Rewrite the session_crypto module against the
     apr_crypto API. [Graham Leggett]
 
diff --git a/docs/manual/mod/mod_ext_filter.xml b/docs/manual/mod/mod_ext_filter.xml
index 68fdfea922..250100212e 100644
--- a/docs/manual/mod/mod_ext_filter.xml
+++ b/docs/manual/mod/mod_ext_filter.xml
@@ -340,6 +340,12 @@ delivery to the client</description>
       messages written to standard error by the external filter
       program will be saved in the Apache error log.
       <code>NoLogStderr</code> disables this feature.</dd>
+
+      <dt><code>Onfail=[abort|remove]</code></dt>
+      <dd>Determines how to proceed if the external filter program
+      cannot be started.  With <code>abort</code> (the default value)
+      the request will be aborted.  With <code>remove</code>, the
+      filter is removed and the request continues without it.</dd>
     </dl>
 
     <example><title>Example</title>
diff --git a/modules/filters/mod_ext_filter.c b/modules/filters/mod_ext_filter.c
index cf4bd4033a..81cd70bf39 100644
--- a/modules/filters/mod_ext_filter.c
+++ b/modules/filters/mod_ext_filter.c
@@ -58,6 +58,7 @@ typedef struct ef_filter_t {
 typedef struct ef_dir_t {
     int debug;
     int log_stderr;
+    int onfail;
 } ef_dir_t;
 
 typedef struct ef_ctx_t {
@@ -81,7 +82,6 @@ static apr_status_t ef_input_filter(ap_filter_t *, apr_bucket_brigade *,
                                     apr_off_t);
 
 #define DBGLVL_SHOWOPTIONS         1
-#define DBGLVL_ERRORCHECK          2
 #define DBGLVL_GORY                9
 
 #define ERRFN_USERDATA_KEY         "EXTFILTCHILDERRFN"
@@ -92,6 +92,7 @@ static void *create_ef_dir_conf(apr_pool_t *p, char *dummy)
 
     dc->debug = -1;
     dc->log_stderr = -1;
+    dc->onfail = -1;
 
     return dc;
 }
@@ -125,6 +126,13 @@ static void *merge_ef_dir_conf(apr_pool_t *p, void *basev, void *overridesv)
         a->log_stderr = base->log_stderr;
     }
 
+    if (over->onfail != -1) {   /* if admin coded something... */
+        a->onfail = over->onfail;
+    }
+    else {
+        a->onfail = base->onfail;
+    }
+
     return a;
 }
 
@@ -142,6 +150,12 @@ static const char *add_options(cmd_parms *cmd, void *in_dc,
     else if (!strcasecmp(arg, "NoLogStderr")) {
         dc->log_stderr = 0;
     }
+    else if (!strcasecmp(arg, "Onfail=remove")) {
+        dc->onfail = 1;
+    }
+    else if (!strcasecmp(arg, "Onfail=abort")) {
+        dc->onfail = 0;
+    }
     else {
         return apr_pstrcat(cmd->temp_pool,
                            "Invalid ExtFilterOptions option: ",
@@ -449,9 +463,9 @@ static apr_status_t init_ext_filter_process(ap_filter_t *f)
     ap_assert(rc == APR_SUCCESS);
     apr_pool_userdata_set(f->r, ERRFN_USERDATA_KEY, apr_pool_cleanup_null, ctx->p);
 
-    if (dc->debug >= DBGLVL_ERRORCHECK) {
-        rc = apr_procattr_error_check_set(ctx->procattr, 1);
-        ap_assert(rc == APR_SUCCESS);
+    rc = apr_procattr_error_check_set(ctx->procattr, 1);
+    if (rc != APR_SUCCESS) {
+        return rc;
     }
 
     /* add standard CGI variables as well as DOCUMENT_URI, DOCUMENT_PATH_INFO,
@@ -855,7 +869,19 @@ static apr_status_t ef_output_filter(ap_filter_t *f, apr_bucket_brigade *bb)
 
     if (!ctx) {
         if ((rv = init_filter_instance(f)) != APR_SUCCESS) {
-            return rv;
+            ctx = f->ctx;
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
+                          "can't initialise output filter %s: %s",
+                          f->frec->name,
+                          ctx->dc->onfail ? "removing" : "aborting");
+            ap_remove_output_filter(f);
+            if (ctx->dc->onfail) {
+                return ap_pass_brigade(f->next, bb);
+            }
+            else {
+                f->r->status = HTTP_INTERNAL_SERVER_ERROR;
+                return HTTP_INTERNAL_SERVER_ERROR;
+            }
         }
         ctx = f->ctx;
     }
@@ -886,7 +912,19 @@ static int ef_input_filter(ap_filter_t *f, apr_bucket_brigade *bb,
 
     if (!ctx) {
         if ((rv = init_filter_instance(f)) != APR_SUCCESS) {
-            return rv;
+            ctx = f->ctx;
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r,
+                          "can't initialise input filter %s: %s",
+                          f->frec->name,
+                          ctx->dc->onfail ? "removing" : "aborting");
+            ap_remove_input_filter(f);
+            if (ctx->dc->onfail) {
+                return ap_get_brigade(f->next, bb, mode, block, readbytes);
+            }
+            else {
+                f->r->status = HTTP_INTERNAL_SERVER_ERROR;
+                return HTTP_INTERNAL_SERVER_ERROR;
+            }
         }
         ctx = f->ctx;
     }