httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: svn commit: r562507 - /httpd/httpd/trunk/modules/filters/mod_deflate.c
Date Sat, 04 Aug 2007 12:08:26 GMT
On Sat, 04 Aug 2007 13:35:20 +0200
Ruediger Pluem <rpluem@apache.org> wrote:

> 
> 
> On 08/03/2007 05:42 PM, niq@apache.org wrote:
> > Author: niq
> > Date: Fri Aug  3 08:42:30 2007
> > New Revision: 562507
> > 
> > URL: http://svn.apache.org/viewvc?view=rev&rev=562507
> > Log:
> > Generalise the content encoding detection and protocol:
> > so it aslo works in inflate out filter, as suggested by rpluem.
> > 
> > NOTE: this fails with some generators (cgi, asis) due to a deeper
> > bug: content-encoding is set later than mod_deflate sees it.  This
> > has always been the case, and could use a separate fix if anyone
> > wants inflate_out other than in a proxy situation.  And it's not
> > mod_deflate's problem.
> 
> So your idea is to address PR 42993 separately?

The patch there doesn't address this issue either: both headers_out
and r->content_encoding are empty at the point where we check
(in my simple mod_asis test).

As regards PR42993 (which was not even filed when I committed the
first version of this update, for the input filter only), I'd have
thought a simpler fix would be to merge r->content_encoding into
the headers if it's non-null.  But I'd rather figure out a general
fix than hack around it in mod_deflate only to see it popping up
in other filters.


> > +	/* these are unlikely to be set anyway, but ... */
> > +        apr_table_unset(r->headers_out, "Content-Length");
> > +        apr_table_unset(r->headers_out, "Content-MD5");
> > +        apr_table_unset(r->headers_out, "Content-Range");
> 
> I agree with Joe's comment a while a ago that we simply should remove
> ourselves if we see a Content-Range header (in both cases input and
> output filter). I think we can only deliver a result in this case
> that is not expected by the client. It might
> make even sense that in the output filter case we return a 416 if the
> request headers contain a Range header.

Agreed, but I think that's really a separate issue.  Actually for the
output filters, we should be using the protocol flags
AP_FILTER_PROTO_CHANGE|AP_FILTER_PROTO_CHANGE_LENGTH|AP_FILTER_PROTO_NO_BYTERANGE

This is really why I started out fixing just the input filter - I didn't
plan to think through the extra complexities in the output:-)

If it'll get your vote, I'll change all three filters to remove
themselves and log a warning if called with a byterange.  But that
feels to me like a separate patch.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Mime
View raw message