On 08/04/2007 02:08 PM, Nick Kew wrote: > On Sat, 04 Aug 2007 13:35:20 +0200 > Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> > 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. This is a good plan. Agreed. > > >>> + /* 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. Ok. This is fine with me, but before I put my vote on this it is needed that we no longer fail the test that Joe mentioned (the one related to Content-Length). I do not want to introduce a regression here. Once this is done, the bytereange issue can be addressed separately as you proposed as it seems to be broken currently anyway. Regards Ru"diger
