You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2011/08/26 09:03:55 UTC
Re: svn commit: r1161681 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On 08/25/2011 08:33 PM, jim@apache.org wrote:
> Author: jim
> Date: Thu Aug 25 18:33:26 2011
> New Revision: 1161681
>
> URL: http://svn.apache.org/viewvc?rev=1161681&view=rev
> Log:
> Optimize... and break if we get eg 200-100
>
> Modified:
> httpd/httpd/trunk/modules/http/byterange_filter.c
>
> Modified: httpd/httpd/trunk/modules/http/byterange_filter.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/byterange_filter.c?rev=1161681&r1=1161680&r2=1161681&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
> +++ httpd/httpd/trunk/modules/http/byterange_filter.c Thu Aug 25 18:33:26 2011
> @@ -566,6 +566,9 @@ static int ap_set_byterange(request_rec
> break;
> }
> end = number;
> + if (start > end) {
> + break;
> + }
Previously we just ignored wrong ordered ranges and continued with the remaining ones.
Wouldn't that mean that now we only process ranges up to a wrong ordered one?
> if (!in_merge) {
> ostart = start;
> oend = end;
> @@ -575,7 +578,7 @@ static int ap_set_byterange(request_rec
> ostart = start;
> in_merge = 1;
> }
> - if (start > ostart && start < oend) {
> + if (start < oend) {
> in_merge = 1;
> }
> if ((end-1) >= oend) {
>
>
>
>
Regards
Rüdiger
Re: svn commit: r1161681 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 8/26/2011 6:42 AM, Jim Jagielski wrote:
> IMO, the function of ap_set_byterange() is to create the
> official working copy of r->range that will be used, and in doing
> so, provide first level verification of what is valid
> (as well as determining if we want to accept the byte-range
> as provided by the client or simply bail out with a 416
> or 200).
Certainly, but we can do so through an array, and can much more quickly
reassemble the canonical r->range from that array. Only two string ops
(decode, and then format). Once we have an untainted r->range, this is
what we aught to pass on to any back end proxy client.
> If we are going to fix it, imo, we should do it here, but
> I'm +-0 on whether to ignore or break when we hit a wrong
> order…
Not break; the spec is blindingly clear that a 200 is always a right
answer, so if we inhibit rewinding, we should fall back on 200 if
any range could be satisfied. (If the content is shorter than any
of the given ranges, we can 416). No?
Re: svn commit: r1161681 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
IMO, the function of ap_set_byterange() is to create the
official working copy of r->range that will be used, and in doing
so, provide first level verification of what is valid
(as well as determining if we want to accept the byte-range
as provided by the client or simply bail out with a 416
or 200).
If we are going to fix it, imo, we should do it here, but
I'm +-0 on whether to ignore or break when we hit a wrong
order…
On Aug 26, 2011, at 3:03 AM, Ruediger Pluem wrote:
>
>
> On 08/25/2011 08:33 PM, jim@apache.org wrote:
>> Author: jim
>> Date: Thu Aug 25 18:33:26 2011
>> New Revision: 1161681
>>
>> URL: http://svn.apache.org/viewvc?rev=1161681&view=rev
>> Log:
>> Optimize... and break if we get eg 200-100
>>
>> Modified:
>> httpd/httpd/trunk/modules/http/byterange_filter.c
>>
>> Modified: httpd/httpd/trunk/modules/http/byterange_filter.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/byterange_filter.c?rev=1161681&r1=1161680&r2=1161681&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
>> +++ httpd/httpd/trunk/modules/http/byterange_filter.c Thu Aug 25 18:33:26 2011
>> @@ -566,6 +566,9 @@ static int ap_set_byterange(request_rec
>> break;
>> }
>> end = number;
>> + if (start > end) {
>> + break;
>> + }
>
> Previously we just ignored wrong ordered ranges and continued with the remaining ones.
> Wouldn't that mean that now we only process ranges up to a wrong ordered one?
>
>> if (!in_merge) {
>> ostart = start;
>> oend = end;
>> @@ -575,7 +578,7 @@ static int ap_set_byterange(request_rec
>> ostart = start;
>> in_merge = 1;
>> }
>> - if (start > ostart && start < oend) {
>> + if (start < oend) {
>> in_merge = 1;
>> }
>> if ((end-1) >= oend) {
>>
>>
>>
>>
>
> Regards
>
> Rüdiger
>