You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by sterling <st...@covalent.net> on 2001/05/29 20:59:45 UTC
[PATCH] option 2: backout changes that cause 'no headers'
Since there needs to be some design work to figure out the best place to
insert the header filters, i suggest we backout the change that inserts
them in the 'insert filters' phase -
here is a patch for it, in case you don't want to just undo the original
commit:
Index: modules/http/http_core.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_core.c,v
retrieving revision 1.276
diff -u -r1.276 http_core.c
--- modules/http/http_core.c 2001/05/21 23:47:15 1.276
+++ modules/http/http_core.c 2001/05/29 18:28:47
@@ -295,13 +295,6 @@
return OK;
}
-static void ap_http_insert_filter(request_rec *r)
-{
- ap_add_output_filter("BYTERANGE", NULL, r, r->connection);
- ap_add_output_filter("CONTENT_LENGTH", NULL, r, r->connection);
- ap_add_output_filter("HTTP_HEADER", NULL, r, r->connection);
-}
-
static void register_hooks(apr_pool_t *p)
{
ap_hook_pre_connection(ap_pre_http_connection,NULL,NULL,
@@ -311,7 +304,6 @@
ap_hook_http_method(http_method,NULL,NULL,APR_HOOK_REALLY_LAST);
ap_hook_default_port(http_port,NULL,NULL,APR_HOOK_REALLY_LAST);
- ap_hook_insert_filter(ap_http_insert_filter, NULL, NULL, APR_HOOK_REALLY_LAST);
ap_register_input_filter("HTTP_IN", ap_http_filter, AP_FTYPE_CONNECTION);
ap_register_input_filter("DECHUNK", ap_dechunk_filter, AP_FTYPE_TRANSCODE);
ap_register_output_filter("HTTP_HEADER", ap_http_header_filter,
Index: modules/http/http_request.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_request.c,v
retrieving revision 1.99
diff -u -r1.99 http_request.c
--- modules/http/http_request.c 2001/05/21 23:47:16 1.99
+++ modules/http/http_request.c 2001/05/29 18:28:47
@@ -517,6 +517,10 @@
new->output_filters = r->connection->output_filters;
new->input_filters = r->connection->input_filters;
+ ap_add_output_filter("BYTERANGE", NULL, new, new->connection);
+ ap_add_output_filter("CONTENT_LENGTH", NULL, new, new->connection);
+ ap_add_output_filter("HTTP_HEADER", NULL, new, new->connection);
+
apr_table_setn(new->subprocess_env, "REDIRECT_STATUS",
apr_psprintf(r->pool, "%d", r->status));
Index: server/protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/protocol.c,v
retrieving revision 1.22
diff -u -r1.22 protocol.c
--- server/protocol.c 2001/05/22 01:31:11 1.22
+++ server/protocol.c 2001/05/29 18:29:21
@@ -590,6 +590,11 @@
? r->server->keep_alive_timeout * APR_USEC_PER_SEC
: r->server->timeout * APR_USEC_PER_SEC));
+ ap_add_output_filter("BYTERANGE", NULL, r, r->connection);
+ ap_add_output_filter("CONTENT_LENGTH", NULL, r, r->connection);
+ ap_add_output_filter("HTTP_HEADER", NULL, r, r->connection);
+
+
/* Get the request... */
if (!read_request_line(r)) {
if (r->status == HTTP_REQUEST_URI_TOO_LARGE) {
Re: [PATCH] option 2: backout changes that cause 'no headers'
Posted by Graham Leggett <mi...@sharp.fm>.
sterling wrote:
> Since there needs to be some design work to figure out the best place to
> insert the header filters, i suggest we backout the change that inserts
> them in the 'insert filters' phase -
>
> here is a patch for it, in case you don't want to just undo the original
> commit:
This means mod_headers is broken again.
I had problems trying to commit the original patch last night, which is
why it wasn't done. Does anyone have a problem with the patch as it
stands, or does anyone have a better solution?
A fix is always better than no fix.
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [PATCH] option 2: backout changes that cause 'no headers'
Posted by sterling <st...@covalent.net>.
That's fine with me -
yeah - probably memorial day silence... i'm still running with the ap_die
patch and it is working fine for me -
thanks -
sterling
On Thu, 31 May 2001, Graham Leggett wrote:
> sterling wrote:
>
> > Since there needs to be some design work to figure out the best place to
> > insert the header filters, i suggest we backout the change that inserts
> > them in the 'insert filters' phase -
>
> The HTTP_HEADER filter needs to be inserted last - any filter that ends
> up being run after the HTTP_HEADER filter will be completely
> ineffective.
>
> The last opportunity to add a filter before the handlers are run is in
> the insert_filter phase.
>
> As a result, I conclude that the correct fix for this is the original
> insert_filter patch.
>
> As to the missing-headers-on-error problem, the current behavior is to
> assume that the filters will be correctly set up when ap_die() is
> called. This of course is bogus - when there is an error condition you
> cannot assume anything.
>
> As a result, I conclude that the correct fix is to add the
> filter-is-there check in ap_die().
>
> So far there has been no opinions yet on this issue - possibly because
> of the Memorial Day weekend (I'm in the UK, so I'm not sure) - but if
> there are no objections, I would like to apply the insert_filter
> HTTP_HEADER fix + the ap_die() filter fix tomorrow morning.
>
> Regards,
> Graham
>
Re: [PATCH] option 2: backout changes that cause 'no headers'
Posted by Graham Leggett <mi...@sharp.fm>.
sterling wrote:
> Since there needs to be some design work to figure out the best place to
> insert the header filters, i suggest we backout the change that inserts
> them in the 'insert filters' phase -
The HTTP_HEADER filter needs to be inserted last - any filter that ends
up being run after the HTTP_HEADER filter will be completely
ineffective.
The last opportunity to add a filter before the handlers are run is in
the insert_filter phase.
As a result, I conclude that the correct fix for this is the original
insert_filter patch.
As to the missing-headers-on-error problem, the current behavior is to
assume that the filters will be correctly set up when ap_die() is
called. This of course is bogus - when there is an error condition you
cannot assume anything.
As a result, I conclude that the correct fix is to add the
filter-is-there check in ap_die().
So far there has been no opinions yet on this issue - possibly because
of the Memorial Day weekend (I'm in the UK, so I'm not sure) - but if
there are no objections, I would like to apply the insert_filter
HTTP_HEADER fix + the ap_die() filter fix tomorrow morning.
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [PATCH] option 2: backout changes that cause 'no headers'
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 29 May 2001, sterling wrote:
> Since there needs to be some design work to figure out the best place to
> insert the header filters, i suggest we backout the change that inserts
> them in the 'insert filters' phase -
i've committed this. will be happy to see the "right" fix when its
implemented/decided, but this has been a known issue for a full week now.