Bug 41120 - Filters corrupt the data sent to the browser if they dont exist
Summary: Filters corrupt the data sent to the browser if they dont exist
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_ext_filter (show other bugs)
Version: 2.2.3
Hardware: Sun Solaris
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: FixedInTrunk
Depends on:
Blocks:
 
Reported: 2006-12-07 02:34 UTC by Sean Timmins
Modified: 2009-01-07 17:40 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sean Timmins 2006-12-07 02:34:21 UTC
If you define an external filter and the actual filter does not exist, or for
instance is not executable, then the data sent back to the browser becomes
corrupted.

Using a build of httpd 2.2.3 on solaris 9, all content was removed (headers
still present) resulting in a blank page. When processing content served by a
backend tomcat server (using mod_proxy_ajp) the content was (apprently
arbitrarily) missing a certain number of bytes from the begining.

The only error in the error_log file was as follows:

[Wed Dec 06 16:30:27 2006] [error] [client 10.0.3.19] (32)Broken pipe:
ef_unified_filter() failed

Droppping a valid filter onto the file system results in an immediate fix.

Sample configuration/filter:

ExtFilterDefine testfilter cmd="/var/tmp/filter.pl"
<Location />
  SetOutputFilter testfilter
</Location>

And simply make sure the file /var/tmp/filter.pl does not exist. URLs should
stop working. Then put the following in /var/tmp/filter.pl:

#!/usr/local/bin/perl

use strict;

while (<STDIN>)
{
  print;
}

Make it executable. Now URLs should be served correctly.
Comment 1 Jeff Trawick 2006-12-07 04:07:11 UTC
You could experiment with a code change like this (several lines after call to
init_filter_instance()):

static apr_status_t ef_output_filter(ap_filter_t *f, apr_bucket_brigade *bb)
{
    request_rec *r = f->r;
    ef_ctx_t *ctx = f->ctx;
    apr_status_t rv;

    if (!ctx) {
        if ((rv = init_filter_instance(f)) != APR_SUCCESS) {
            /* return rv; */
            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r,
                          "mod_ext_filter: ignoring error, sending unfiltered
data");
            f->etx->noop = 1;
        }
        ctx = f->ctx;
    }
    if (ctx->noop) {
        ap_remove_output_filter(f);
        return ap_pass_brigade(f->next, bb);
    }

This doesn't seem so useful in the grand scheme of things.  It seems to me
rather rare that somebody would

*) implement/configure a filter
*) not test it immediately after configuration to ensure that it actually worked
*) desire for data to be sent unfiltered if the filter can't be started

I wonder if this scenario was confusing because of how the error was handled?
Maybe the output filter was called again even after it returned an error the
first time, and that led to the garbled output instead of no output?

That seems unlikely, but I don't see how else you'd be missing just the first
part of the response.  If it can happen then this patch might help:

Index: mod_ext_filter.c
===================================================================
--- mod_ext_filter.c	(revision 482178)
+++ mod_ext_filter.c	(working copy)
@@ -67,6 +67,7 @@
     ef_dir_t *dc;
     ef_filter_t *filter;
     int noop;
+    apr_status_t perm_error;
 #if APR_FILES_AS_SOCKETS
     apr_pollset_t *pollset;
 #endif
@@ -855,10 +856,15 @@
 
     if (!ctx) {
         if ((rv = init_filter_instance(f)) != APR_SUCCESS) {
+            f->ctx->perm_error = rv;
             return rv;
         }
         ctx = f->ctx;
     }
+    else if (ctx->perm_error != 0) {
+        return ctx->perm_error;
+    }
+    
     if (ctx->noop) {
         ap_remove_output_filter(f);
         return ap_pass_brigade(f->next, bb);
Comment 2 Nick Kew 2009-01-04 12:53:54 UTC
Fixed in trunk in r731358.

This would actually not have happened with DebugLevel 2 in ExtFilterOptions.  But relying on debug is not acceptable, particularly where an external program might fail to start for other reasons such as system too busy.  So I've made the startup check unconditional, and added a configuration option to abort or continue without the filter.
Comment 3 Nick Kew 2009-01-07 17:40:11 UTC
Fix backported to 2.2 in r732586