You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by sf...@apache.org on 2011/08/28 21:45:21 UTC
svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Author: sf
Date: Sun Aug 28 19:45:21 2011
New Revision: 1162579
URL: http://svn.apache.org/viewvc?rev=1162579&view=rev
Log:
Every 32 ranges, pass the prepared ranges down the filter chain.
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=1162579&r1=1162578&r2=1162579&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
+++ httpd/httpd/trunk/modules/http/byterange_filter.c Sun Aug 28 19:45:21 2011
@@ -378,6 +378,12 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
}
APR_BRIGADE_CONCAT(bsend, tmpbb);
+ if (i && i % 32 == 0) {
+ /* Every now and then, pass what we have down the filter chain */
+ if ((rv = ap_pass_brigade(f->next, bsend)) != APR_SUCCESS)
+ return rv;
+ apr_brigade_cleanup(bsend);
+ }
}
if (found == 0) {
Re: svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 29 August 2011, Plüm, Rüdiger, VF-Group wrote:
> > -----Original Message-----
> > From: Stefan Fritsch [mailto:sf@sfritsch.de]
> > Sent: Montag, 29. August 2011 09:45
> > To: dev@httpd.apache.org
> > Subject: Re: svn commit: r1162579 -
> > /httpd/httpd/trunk/modules/http/byterange_filter.c
> >
> > On Sunday 28 August 2011, Stefan Fritsch wrote:
> > > This is broken. It causes the Content-Length header to contain
> > > the size of the original file instead of the response body. Is
> > > the correct fix to add apr_table_unset(r->headers_out,
> > > "Content-Length") ?
> >
> > Committed that to trunk and updated
> > http://people.apache.org/~sf/byterange-no-merge.2.2.diff to
> > include it
> > and a change to reset the status to 200 if the range header is
> > invalid. The latter issue was fixed in trunk by Eric's MaxRanges
> > change.
>
> Patch looks good, but a few comments:
>
> 1. r1162669 is missing (provided this was really a good idea from
> me :-)).
The compiler should be able to optimize that and the old code is way
more readable.
I have proposed the above patch for backport with an CHANGES entry
added but no other changes.
Re: svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 29, 2011, at 5:06 AM, Plüm, Rüdiger, VF-Group wrote:
>
>
>> -----Original Message-----
>> From: Stefan Fritsch [mailto:sf@sfritsch.de]
>> Sent: Montag, 29. August 2011 09:45
>> To: dev@httpd.apache.org
>> Subject: Re: svn commit: r1162579 -
>> /httpd/httpd/trunk/modules/http/byterange_filter.c
>>
>> On Sunday 28 August 2011, Stefan Fritsch wrote:
>>
>>> This is broken. It causes the Content-Length header to contain the
>>> size of the original file instead of the response body. Is the
>>> correct fix to add apr_table_unset(r->headers_out,
>>> "Content-Length") ?
>>
>> Committed that to trunk and updated
>> http://people.apache.org/~sf/byterange-no-merge.2.2.diff to
>> include it
>> and a change to reset the status to 200 if the range header is
>> invalid. The latter issue was fixed in trunk by Eric's MaxRanges
>> change.
>>
>
> Patch looks good, but a few comments:
>
> 1. r1162669 is missing (provided this was really a good idea from me :-)).
> 2. I adjusted trunk code to drop the copying of the original range
> header to "or" as well as this does not seem to be needed (r1162687).
>
+1 on the diff-file with those 2 changes…
Will commit in 1.5 hrs unless I hear vetoes and will then push
on for a T&R
RE: svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Stefan Fritsch [mailto:sf@sfritsch.de]
> Sent: Montag, 29. August 2011 09:45
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1162579 -
> /httpd/httpd/trunk/modules/http/byterange_filter.c
>
> On Sunday 28 August 2011, Stefan Fritsch wrote:
>
> > This is broken. It causes the Content-Length header to contain the
> > size of the original file instead of the response body. Is the
> > correct fix to add apr_table_unset(r->headers_out,
> > "Content-Length") ?
>
> Committed that to trunk and updated
> http://people.apache.org/~sf/byterange-no-merge.2.2.diff to
> include it
> and a change to reset the status to 200 if the range header is
> invalid. The latter issue was fixed in trunk by Eric's MaxRanges
> change.
>
Patch looks good, but a few comments:
1. r1162669 is missing (provided this was really a good idea from me :-)).
2. I adjusted trunk code to drop the copying of the original range
header to "or" as well as this does not seem to be needed (r1162687).
Regards
Rüdiger
Re: svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sunday 28 August 2011, Stefan Fritsch wrote:
> This is broken. It causes the Content-Length header to contain the
> size of the original file instead of the response body. Is the
> correct fix to add apr_table_unset(r->headers_out,
> "Content-Length") ?
Committed that to trunk and updated
http://people.apache.org/~sf/byterange-no-merge.2.2.diff to include it
and a change to reset the status to 200 if the range header is
invalid. The latter issue was fixed in trunk by Eric's MaxRanges
change.
RE: svn commit: r1162579 - /httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Stefan Fritsch [mailto:sf@sfritsch.de]
> Sent: Sonntag, 28. August 2011 23:36
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1162579 -
> /httpd/httpd/trunk/modules/http/byterange_filter.c
>
> On Sun, 28 Aug 2011, sf@apache.org wrote:
>
> > Author: sf
> > Date: Sun Aug 28 19:45:21 2011
> > New Revision: 1162579
> >
> > URL: http://svn.apache.org/viewvc?rev=1162579&view=rev
> > Log:
> > Every 32 ranges, pass the prepared ranges down the filter chain.
> >
> > 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/by
> terange_filter.c?rev=1162579&r1=1162578&r2=1162579&view=diff
> >
> ==============================================================
> ================
> > --- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
> > +++ httpd/httpd/trunk/modules/http/byterange_filter.c Sun
> Aug 28 19:45:21 2011
> > @@ -378,6 +378,12 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
> > }
> >
> > APR_BRIGADE_CONCAT(bsend, tmpbb);
> > + if (i && i % 32 == 0) {
> > + /* Every now and then, pass what we have down
> the filter chain */
> > + if ((rv = ap_pass_brigade(f->next, bsend)) !=
> APR_SUCCESS)
> > + return rv;
> > + apr_brigade_cleanup(bsend);
> > + }
> > }
>
> This is broken. It causes the Content-Length header to
> contain the size of
> the original file instead of the response body. Is the
> correct fix to add
> apr_table_unset(r->headers_out, "Content-Length") ?
This looks correct. Maybe to be extra save we should also set r->clength to
0 as well.
Regards
Rüdiger
Re: svn commit: r1162579 -
/httpd/httpd/trunk/modules/http/byterange_filter.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sun, 28 Aug 2011, sf@apache.org wrote:
> Author: sf
> Date: Sun Aug 28 19:45:21 2011
> New Revision: 1162579
>
> URL: http://svn.apache.org/viewvc?rev=1162579&view=rev
> Log:
> Every 32 ranges, pass the prepared ranges down the filter chain.
>
> 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=1162579&r1=1162578&r2=1162579&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
> +++ httpd/httpd/trunk/modules/http/byterange_filter.c Sun Aug 28 19:45:21 2011
> @@ -378,6 +378,12 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
> }
>
> APR_BRIGADE_CONCAT(bsend, tmpbb);
> + if (i && i % 32 == 0) {
> + /* Every now and then, pass what we have down the filter chain */
> + if ((rv = ap_pass_brigade(f->next, bsend)) != APR_SUCCESS)
> + return rv;
> + apr_brigade_cleanup(bsend);
> + }
> }
This is broken. It causes the Content-Length header to contain the size of
the original file instead of the response body. Is the correct fix to add
apr_table_unset(r->headers_out, "Content-Length") ?