httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject resolving the "removing the first connection filter in chain" bug
Date Mon, 10 Jan 2005 23:09:54 GMT
1.5+ years ago I've submitted this bug report. Back then the answer was (I 
think by Justin): the fix is to rewrite the filter chains to be 
double-linked and that will be possible only in 2.1. As now it's about 
time to implement this, please suggest how it's the best to proceeed.

-------- Original Message --------
Subject: [BUG] in 2.0's ap_read_request with regards to removing the first 
connection filter in chain
Date: Fri, 23 May 2003 14:01:45 +1000
From: Stas Bekman <stas@stason.org>
Reply-To: dev@httpd.apache.org
Organization: Hope, Humanized
To: dev@httpd.apache.org

there is an edge case bug in filter removal code.

ap_read_request has the following code:

     r->proto_output_filters = conn->output_filters;
     r->output_filters  = r->proto_output_filters;
     r->proto_input_filters = conn->input_filters;
     r->input_filters   = r->proto_input_filters;

this makes it impossible to remove the very first filter in
conn->output_filters or conn->input_filters, because while it gets
conn->removed from the corresponding conn filter
conn->chain. r->output_filters and r->input_filters are still pointing
conn->to that filter, which was removed. Result: the filter gets
conn->invoked again and again...

I was toying with different possible solutions but they all have
failed to work perfectly. I believe the only long-term solution, which
doesn't slow things down, is to extend the filter struct to add a
'prev' entry, turning the filter chain into double-linked list, so
remove_any_filter can fix
r->input_filters directly.

I was thinking to plug a dummy pass-through filter as an alternative
solution, but one has to ensure that no user filter will be able to
insert a filter in front of it. So if we can ensure that
conn->output_filters always starts with a dummy filter, removing any
other filters will work correctly. This can be accomplished by
changing add_any_filter and other functions to keep the dummy edge
filter at the top of the connection filter chain.

I have a cheating solution though, the patch below, instead of
removing the edge filter, replaces its frec with an frec for a dummy
pass-through filter.  I've run multiple filter tests on it and it
seems to work fine. The only potential problem with this solution if
anything is trying to access frec of the filter that was removed (just
because it may still be running). So this solution is not clean,
because it reminds me of cutting the brach ones sit on.


Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.70.2.2
diff -u -r1.70.2.2 http_core.h
--- include/http_core.h	8 May 2003 20:49:32 -0000	1.70.2.2
+++ include/http_core.h	23 May 2003 03:56:18 -0000
@@ -609,7 +609,9 @@
  extern AP_DECLARE_DATA ap_filter_rec_t *ap_content_length_filter_handle;
  extern AP_DECLARE_DATA ap_filter_rec_t *ap_net_time_filter_handle;
  extern AP_DECLARE_DATA ap_filter_rec_t *ap_core_input_filter_handle;
-
+extern AP_DECLARE_DATA ap_filter_rec_t *ap_core_dummy_input_filter_handle;
+extern AP_DECLARE_DATA ap_filter_rec_t *ap_core_dummy_output_filter_handle;
+
  /**
   * This hook provdes a way for modules to provide metrics/statistics about
   * their operational status.
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.225.2.6
diff -u -r1.225.2.6 core.c
--- server/core.c	13 May 2003 16:01:04 -0000	1.225.2.6
+++ server/core.c	23 May 2003 03:56:18 -0000
@@ -125,6 +125,8 @@
  AP_DECLARE_DATA ap_filter_rec_t *ap_content_length_filter_handle;
  AP_DECLARE_DATA ap_filter_rec_t *ap_net_time_filter_handle;
  AP_DECLARE_DATA ap_filter_rec_t *ap_core_input_filter_handle;
+AP_DECLARE_DATA ap_filter_rec_t *ap_core_dummy_input_filter_handle;
+AP_DECLARE_DATA ap_filter_rec_t *ap_core_dummy_output_filter_handle;

  static void *create_core_dir_config(apr_pool_t *a, char *dir)
  {
@@ -3483,6 +3485,19 @@
      } while (!APR_BRIGADE_EMPTY(b) && (e != APR_BRIGADE_SENTINEL(b))); \
  } while (0)

+static int core_dummy_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
+                                   ap_input_mode_t mode, apr_read_type_e 
block,
+                                   apr_off_t readbytes)
+{
+    return ap_get_brigade(f->next, b, mode, block, readbytes);
+}
+
+static int core_dummy_output_filter(ap_filter_t *f,
+                                    apr_bucket_brigade *b)
+{
+    return ap_pass_brigade(f->next, b);
+}
+
  static int core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
                               ap_input_mode_t mode, apr_read_type_e block,
                               apr_off_t readbytes)
@@ -4288,6 +4303,12 @@
       */
      ap_hook_insert_filter(core_insert_filter, NULL, NULL, APR_HOOK_MIDDLE);

+    ap_core_dummy_input_filter_handle =
+        ap_register_input_filter("CORE_DUMMY_IN", core_dummy_input_filter,
+                                 NULL, AP_FTYPE_NETWORK);
+    ap_core_dummy_output_filter_handle =
+        ap_register_output_filter("CORE_DUMMY_OUT", core_dummy_output_filter,
+                                  NULL, AP_FTYPE_NETWORK);
      ap_core_input_filter_handle =
          ap_register_input_filter("CORE_IN", core_input_filter,
                                   NULL, AP_FTYPE_NETWORK);
Index: server/util_filter.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/util_filter.c,v
retrieving revision 1.93.2.2
diff -u -r1.93.2.2 util_filter.c
--- server/util_filter.c	22 Feb 2003 18:09:57 -0000	1.93.2.2
+++ server/util_filter.c	23 May 2003 03:56:18 -0000
@@ -61,6 +61,7 @@
  #include "httpd.h"
  #include "http_log.h"
  #include "util_filter.h"
+#include "http_core.h"

  /* NOTE: Apache's current design doesn't allow a pool to be passed thru,
     so we depend on a global to hold the correct pool
@@ -459,8 +460,11 @@
                                   &c->output_filters);
  }

-static void remove_any_filter(ap_filter_t *f, ap_filter_t **r_filt,
ap_filter_t **p_filt,
-                              ap_filter_t **c_filt)
+typedef enum { INPUT, OUTPUT } type;
+
+static void remove_any_filter(ap_filter_t *f, ap_filter_t **r_filt,
+                              ap_filter_t **p_filt,
+                              ap_filter_t **c_filt, type in_out)
  {
      ap_filter_t **curr = r_filt ? r_filt : c_filt;
      ap_filter_t *fscan = *curr;
@@ -469,7 +473,12 @@
          *p_filt = (*p_filt)->next;

      if (*curr == f) {
-        *curr = (*curr)->next;
+        if (in_out == INPUT) {
+            (*curr)->frec = ap_core_dummy_input_filter_handle;
+        }
+        else {
+            (*curr)->frec = ap_core_dummy_output_filter_handle;
+        }
          return;
      }

@@ -486,14 +495,14 @@
  {
      remove_any_filter(f, f->r ? &f->r->input_filters : NULL,
                        f->r ? &f->r->proto_input_filters : NULL,
-                      &f->c->input_filters);
+                      &f->c->input_filters, INPUT);
  }

  AP_DECLARE(void) ap_remove_output_filter(ap_filter_t *f)
  {
      remove_any_filter(f, f->r ? &f->r->output_filters : NULL,
                        f->r ? &f->r->proto_output_filters : NULL,
-                      &f->c->output_filters);
+                      &f->c->output_filters, OUTPUT);
  }

  /*


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Mime
View raw message