You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Brian J. France" <li...@firehawksystems.com> on 2006/04/18 20:37:24 UTC
mod_deflate patch
This was talked about a few weeks back but I don't think anything
ever came of it. The patch below would allow mod_deflate to compress
internal redirects while still skipping sub requests.
I have been running this on my personal server for a few weeks now
with no issues.
Let the voting begin.
Brian
Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c (revision 394996)
+++ modules/filters/mod_deflate.c (working copy)
@@ -240,7 +240,7 @@
const char *encoding;
/* only work on main request/no subrequests */
- if (!ap_is_initial_req(r)) {
+ if (r->main != NULL) {
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, bb);
}
Re: mod_deflate patch
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Brian J. France wrote:
>
> The patched block of code is only called when f->ctx is NULL and hasn't
> been setup yet by mod_deflate. I would assume when a sub request would
> get added the ctx for its ap_filter_t struct would be NULL and
> f->r->main would be the top request so the deflate filter would be
> removed.
Ding ding ding :) Ok thank you for pointing that out... once we have a filter
context we won't muck this up.
>> It seems this should be a conditional add-filter, never a conditional remove
>> filter event. add-filter on the top level request, noop on nested requests.
>
> Not sure I have the expertise to comment on that.
No need, your patch is fine :) Thanks for clarifications.
Re: mod_deflate patch
Posted by "Brian J. France" <li...@firehawksystems.com>.
On Apr 18, 2006, at 2:52 PM, William A. Rowe, Jr. wrote:
>> @@ -240,7 +240,7 @@
>> const char *encoding;
>> /* only work on main request/no subrequests */
>> - if (!ap_is_initial_req(r)) {
>> + if (r->main != NULL) {
>> ap_remove_output_filter(f);
>
> Actually, explain to me how this code successfully leaves the http
> protocol
> layer output_filter in the filter chain for subrequest components?
Using ap_is_initial_req:
AP_DECLARE(int) ap_is_initial_req(request_rec *r)
{
return (r->main == NULL) /* otherwise, this is a sub-
request */
&& (r->prev == NULL); /* otherwise, this is an internal
redirect */
}
it will remove the filter for both sub-request and internal
redirects. The patch just removes the filter if it is a sub request.
> I'd think
> this code (original, and even the patched flavor) could break the
> filter stack
> by yanking the deflate filter out from the middle of servicing a
> request, e.g.
> when a subrequest is included midstream.
The patched block of code is only called when f->ctx is NULL and
hasn't been setup yet by mod_deflate. I would assume when a sub
request would get added the ctx for its ap_filter_t struct would be
NULL and f->r->main would be the top request so the deflate filter
would be removed.
> It seems this should be a conditional add-filter, never a
> conditional remove
> filter event. add-filter on the top level request, noop on nested
> requests.
Not sure I have the expertise to comment on that.
Brian
Re: mod_deflate patch
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> @@ -240,7 +240,7 @@
> const char *encoding;
> /* only work on main request/no subrequests */
> - if (!ap_is_initial_req(r)) {
> + if (r->main != NULL) {
> ap_remove_output_filter(f);
Actually, explain to me how this code successfully leaves the http protocol
layer output_filter in the filter chain for subrequest components? I'd think
this code (original, and even the patched flavor) could break the filter stack
by yanking the deflate filter out from the middle of servicing a request, e.g.
when a subrequest is included midstream.
It seems this should be a conditional add-filter, never a conditional remove
filter event. add-filter on the top level request, noop on nested requests.
Re: mod_deflate patch
Posted by Ian Holsman <li...@holsman.net>.
+1 from me.
On 19/04/2006, at 4:37 AM, Brian J. France wrote:
> This was talked about a few weeks back but I don't think anything
> ever came of it. The patch below would allow mod_deflate to
> compress internal redirects while still skipping sub requests.
>
> I have been running this on my personal server for a few weeks now
> with no issues.
>
> Let the voting begin.
>
> Brian
>
> Index: modules/filters/mod_deflate.c
> ===================================================================
> --- modules/filters/mod_deflate.c (revision 394996)
> +++ modules/filters/mod_deflate.c (working copy)
> @@ -240,7 +240,7 @@
> const char *encoding;
> /* only work on main request/no subrequests */
> - if (!ap_is_initial_req(r)) {
> + if (r->main != NULL) {
> ap_remove_output_filter(f);
> return ap_pass_brigade(f->next, bb);
> }
>
--
Ian Holsman
Zilbo.com / (425) 296-6771 USA/ ++61 (03) 9818-0132 Australia
You miss 100% of the shots you don't take -- Wayne Gretzky
Re: mod_deflate patch
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
This patch is correct. +1 to any/all branches as applicable.
Brian J. France wrote:
> This was talked about a few weeks back but I don't think anything ever
> came of it. The patch below would allow mod_deflate to compress
> internal redirects while still skipping sub requests.
>
> I have been running this on my personal server for a few weeks now with
> no issues.
>
> Let the voting begin.
>
> Brian
>
> Index: modules/filters/mod_deflate.c
> ===================================================================
> --- modules/filters/mod_deflate.c (revision 394996)
> +++ modules/filters/mod_deflate.c (working copy)
> @@ -240,7 +240,7 @@
> const char *encoding;
> /* only work on main request/no subrequests */
> - if (!ap_is_initial_req(r)) {
> + if (r->main != NULL) {
> ap_remove_output_filter(f);
> return ap_pass_brigade(f->next, bb);
> }