You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rb...@apache.org on 2002/03/04 06:54:45 UTC

cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

rbb         02/03/03 21:54:45

  Modified:    modules/http http_core.c http_protocol.c
  Log:
  Adding the same filters over and over again used to be okay, because
  we would lose the extra filters.  Now, if a filter is added, it is run.
  Unfortunately, this can cause an infinite loop, or it can cause request
  headers to appear twice.  This commit removes two instances in the core
  where we were inserting filters for a second and third time.  The bug
  was that error responses were causing infinite loops.
  
  This also removes the reset_filters function, which did the exact
  same thing as add_required_filters.  The two functions were both called
  in error conditions, which was part of what caused this bug.
  
  Revision  Changes    Path
  1.295     +1 -1      httpd-2.0/modules/http/http_core.c
  
  Index: http_core.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
  retrieving revision 1.294
  retrieving revision 1.295
  diff -u -r1.294 -r1.295
  --- http_core.c	3 Mar 2002 22:34:55 -0000	1.294
  +++ http_core.c	4 Mar 2002 05:54:44 -0000	1.295
  @@ -304,7 +304,7 @@
   
   static void ap_http_insert_filter(request_rec *r)
   {
  -    if (!r->main) {
  +    if (!r->main && !r->prev) {
           ap_add_output_filter_handle(ap_byterange_filter_handle,
                                       NULL, r, r->connection);
           ap_add_output_filter_handle(ap_content_length_filter_handle,
  
  
  
  1.393     +0 -11     httpd-2.0/modules/http/http_protocol.c
  
  Index: http_protocol.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
  retrieving revision 1.392
  retrieving revision 1.393
  diff -u -r1.392 -r1.393
  --- http_protocol.c	27 Feb 2002 03:55:31 -0000	1.392
  +++ http_protocol.c	4 Mar 2002 05:54:44 -0000	1.393
  @@ -1843,16 +1843,6 @@
       }
   }
   
  -static void reset_filters(request_rec *r)
  -{
  -    /* only reset request level filters,
  -     * connection level filters need to remain in tact
  -     */
  -    r->output_filters = r->connection->output_filters;
  -    ap_add_output_filter("CONTENT_LENGTH", NULL, r, r->connection);
  -    ap_add_output_filter("HTTP_HEADER", NULL, r, r->connection);
  -}
  -
   /* We should have named this send_canned_response, since it is used for any
    * response that can be generated by the server from the request record.
    * This includes all 204 (no content), 3xx (redirect), 4xx (client error),
  @@ -1870,7 +1860,6 @@
        * this value.
        */
       r->eos_sent = 0;
  -    reset_filters(r);
   
       /*
        * It's possible that the Location field might be in r->err_headers_out
  
  
  

RE: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Ryan Bloom <rb...@covalent.net>.
> On Sun, Mar 03, 2002 at 09:55:25PM -0800, Ryan Bloom wrote:
> > Justin,
> >
> > This fixes the second of your two bugs.  Thanks for finding those,
I'm
> > sorry I let them slip through.  BTW, the number of bugs that this
code
> > exposed has convinced me that this is the correct direction to go.
The
> > reason I say that is that all of the bugs that have been exposed are
> > happening because we tried to hack around problems with the filters.
> 
> Yeah, I think we hit a hot spot of problems.  Better to find and
> get it working now.  Obviously, this wasn't as easy as we all
> thought it should be.
> 
> I'm getting a SEGV when requesting / now (not an infinite loop)
> at util_filters.c:347.

I'm not seeing that SEGV.  I would track it down if I could, but it
doesn't exist on my machine.  Sorry.  Make sure you clean your tree
before you try the request.

Ryan




RE: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Ryan Bloom <rb...@covalent.net>.
> On Sun, Mar 03, 2002 at 09:55:25PM -0800, Ryan Bloom wrote:
> > Justin,
> >
> > This fixes the second of your two bugs.  Thanks for finding those,
I'm
> > sorry I let them slip through.  BTW, the number of bugs that this
code
> > exposed has convinced me that this is the correct direction to go.
The
> > reason I say that is that all of the bugs that have been exposed are
> > happening because we tried to hack around problems with the filters.
> 
> Yeah, I think we hit a hot spot of problems.  Better to find and
> get it working now.  Obviously, this wasn't as easy as we all
> thought it should be.
> 
> I'm getting a SEGV when requesting / now (not an infinite loop)
> at util_filters.c:347.

I'm not seeing that SEGV.  I would track it down if I could, but it
doesn't exist on my machine.  Sorry.  Make sure you clean your tree
before you try the request.

Ryan




Re: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sun, Mar 03, 2002 at 09:55:25PM -0800, Ryan Bloom wrote:
> Justin,
> 
> This fixes the second of your two bugs.  Thanks for finding those, I'm
> sorry I let them slip through.  BTW, the number of bugs that this code
> exposed has convinced me that this is the correct direction to go.  The
> reason I say that is that all of the bugs that have been exposed are
> happening because we tried to hack around problems with the filters.

Yeah, I think we hit a hot spot of problems.  Better to find and
get it working now.  Obviously, this wasn't as easy as we all
thought it should be.

I'm getting a SEGV when requesting / now (not an infinite loop)
at util_filters.c:347.

(gdb) print *r_filters->next->next->next      
$12 = {frec = 0x8101800, ctx = 0x0, next = 0x81a6400, prev = 0x8164b40, 
  r = 0x81a3e40, c = 0x81648d8}
(gdb) print *r_filters->next->next->next->frec
$14 = {name = 0x8101670 "net_time", filter_func = {
    out_func = 0x80c7d70 <net_time_filter>, 
    in_func = 0x80c7d70 <net_time_filter>}, ftype =
AP_FTYPE_HTTP_HEADER, 
  next = 0x0}
(gdb) print *r_filters->next->next->next->next
$13 = {frec = 0x65746e6f, ctx = 0x6c2d746e, next = 0x75676e61, 
  prev = 0x3a656761, r = 0xa6c7020, c = 0x746e6f43}

Somehow something is corrupted.  I have a few more shows to watch
on my TiVo before I take a look at it tonight.  =)  If you beat
me to analyzing it, great.  If not, I'll try to tackle it.  -- justin


Re: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sun, Mar 03, 2002 at 09:55:25PM -0800, Ryan Bloom wrote:
> Justin,
> 
> This fixes the second of your two bugs.  Thanks for finding those, I'm
> sorry I let them slip through.  BTW, the number of bugs that this code
> exposed has convinced me that this is the correct direction to go.  The
> reason I say that is that all of the bugs that have been exposed are
> happening because we tried to hack around problems with the filters.

Yeah, I think we hit a hot spot of problems.  Better to find and
get it working now.  Obviously, this wasn't as easy as we all
thought it should be.

I'm getting a SEGV when requesting / now (not an infinite loop)
at util_filters.c:347.

(gdb) print *r_filters->next->next->next      
$12 = {frec = 0x8101800, ctx = 0x0, next = 0x81a6400, prev = 0x8164b40, 
  r = 0x81a3e40, c = 0x81648d8}
(gdb) print *r_filters->next->next->next->frec
$14 = {name = 0x8101670 "net_time", filter_func = {
    out_func = 0x80c7d70 <net_time_filter>, 
    in_func = 0x80c7d70 <net_time_filter>}, ftype =
AP_FTYPE_HTTP_HEADER, 
  next = 0x0}
(gdb) print *r_filters->next->next->next->next
$13 = {frec = 0x65746e6f, ctx = 0x6c2d746e, next = 0x75676e61, 
  prev = 0x3a656761, r = 0xa6c7020, c = 0x746e6f43}

Somehow something is corrupted.  I have a few more shows to watch
on my TiVo before I take a look at it tonight.  =)  If you beat
me to analyzing it, great.  If not, I'll try to tackle it.  -- justin


RE: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Ryan Bloom <rb...@covalent.net>.
>   Modified:    modules/http http_core.c http_protocol.c
>   Log:
>   Adding the same filters over and over again used to be okay, because
>   we would lose the extra filters.  Now, if a filter is added, it is
run.
>   Unfortunately, this can cause an infinite loop, or it can cause
request
>   headers to appear twice.  This commit removes two instances in the
core
>   where we were inserting filters for a second and third time.  The
bug
>   was that error responses were causing infinite loops.
> 
>   This also removes the reset_filters function, which did the exact
>   same thing as add_required_filters.  The two functions were both
called
>   in error conditions, which was part of what caused this bug.

Justin,

This fixes the second of your two bugs.  Thanks for finding those, I'm
sorry I let them slip through.  BTW, the number of bugs that this code
exposed has convinced me that this is the correct direction to go.  The
reason I say that is that all of the bugs that have been exposed are
happening because we tried to hack around problems with the filters.

Ryan

 


RE: cvs commit: httpd-2.0/modules/http http_core.c http_protocol.c

Posted by Ryan Bloom <rb...@covalent.net>.
>   Modified:    modules/http http_core.c http_protocol.c
>   Log:
>   Adding the same filters over and over again used to be okay, because
>   we would lose the extra filters.  Now, if a filter is added, it is
run.
>   Unfortunately, this can cause an infinite loop, or it can cause
request
>   headers to appear twice.  This commit removes two instances in the
core
>   where we were inserting filters for a second and third time.  The
bug
>   was that error responses were causing infinite loops.
> 
>   This also removes the reset_filters function, which did the exact
>   same thing as add_required_filters.  The two functions were both
called
>   in error conditions, which was part of what caused this bug.

Justin,

This fixes the second of your two bugs.  Thanks for finding those, I'm
sorry I let them slip through.  BTW, the number of bugs that this code
exposed has convinced me that this is the correct direction to go.  The
reason I say that is that all of the bugs that have been exposed are
happening because we tried to hack around problems with the filters.

Ryan