[patch-1] Range filter: support multiple ranges.

[Maxim Dounin]

Hello!

On Fri, Nov 10, 2017 at 04:41:57AM +0800, 胡聪 (hucc) wrote:

> On Thursday, Nov 9, 2017 10:48 PM +0300 Maxim Dounin wrote:

[...]

> >> When multiple ranges are requested, nginx will coalesce any of the ranges
> >> that overlap, or that are separated by a gap that is smaller than the
> >> NGX_HTTP_RANGE_MULTIPART_GAP macro.
> >
> >(Note that the patch also does reordering of ranges.  For some
> >reason this is not mentioned in the commit log.  There are also
> >other changes not mentioned in the commit log - for example, I see
> >ngx_http_range_t was moved to ngx_http_request.h.  These are
> >probably do not belong to the patch at all.)
> 
> I actually wait for you to give better advice. I tried my best to
> make the changes easier and more readable and I will split it into
> multiple patches based on your suggestions if these changes will be
> accepted.

General rule is: keep distinct changes in separate patches.  I 
don't think I have anything better to suggest here.

> >Reordering and/or coalescing ranges is not something that clients
> >usually expect to happen.  This was widely discussed at the time
> >of CVE-2011-3192 vulnerability in Apache.  As a result, RFC 7233
> >introduced the "MAY coalesce" clause.  But this doesn't make
> >clients, especially old ones, magically prepared for this.
> 
> I did not know the CVE-2011-3192. If multiple ranges list in
> ascending order and there are no overlapping ranges, the code will
> be much simpler. This is what I think.

While your intention is understood, this is certainly not 
something we should do.  As far as it is possible, we should 
preserve exact order and range sizes instead, to avoid 
compatibility problems and to preserve various use cases which 
require non-sequential order.

> >Moreover, this will certainly break some use cases like "request
> >some metadata first, and then rest of the file".  So this is
> >certainly not a good idea to always reorder / coalesce ranges
> >unless this is really needed for some reason.  (Or even at all,
> >as just returning 200 might be much more compatible with various
> >clients, as outlined above.)
> >
> >It is also not clear what you are trying to achieve with this
> >patch.  You may want to elaborate more on what problem you are
> >trying to solve, may be there are better solutions.
> 
> I am trying to support multiple ranges when proxy_buffering is off
> and sometimes slice is enabled. The data is always cached in the
> backend which is not nginx. As far as I know, similar architecture
> is widely used in CDN. So the implementation of multiple ranges in
> the architecture I mentioned above is required and inevitable.
> Besides, P2P clients desire for this feature to gather data-pieces.
> Hope I already made it clear.

So you are trying to support multi-range requests to resources 
which use slice module, correct?

Do you have any specific clients in mind?  I've seen very few 
legitimate clients which use multi-range requests.

-- 
Maxim Dounin
http://mdounin.ru/