> -----Original Message----- > From: Jim Jagielski > Sent: Freitag, 26. August 2011 13:38 > To: [email protected] > Subject: Re: svn commit: r1161661 - > /httpd/httpd/trunk/modules/http/byterange_filter.c > > I still think that your version is wrong wrong wrong and am > tempted to veto it. > > It completely invalidates what ap_set_byterange() is designed to > do (set r->range) as well as removes the ability to count > overlaps, non-ascends, etc...
The question is: Does r->range need to reflect the range definition that is actually delivered by the byte range filter? The comment in httpd.h only says: /** The Range: header */ So you could put it vice versa and say r->range is not allowed to contain an adjusted / optimized version of the Range header, but must contain the original Range header. To be honest I would not follow that, because the original Range header is still in r->headers_out and it makes sense to store an optimized version of the Range header in r->range. Nevertheless it is my opinion that it doesn't make sense to parse the Range header twice. If we need a proper r->range and the other statistics, we should have a function that parses the Range header and outputs 1. The number of ranges in the original header 2. The number of ranges after the merges. 3. Whatever else statistics we need. 4. The merged ranges as an array (like the one Stefans patch creates) 5. A proper r->range created from 4. Although ap_set_byterange is in the ap_ namespace it is a static function in byterange_filter.c. So we are free to adjust it as we like without the need for any bumps and possibly creating code that cannot be backported. Regards Rüdiger
