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

[胡聪 (hucc)]

Hi,

On Tuesday, Nov 14, 2017 10:58 PM +0300, Maxim Dounin wrote:
>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?

Thank you very much for giving a detailed reply.

Yes, I am trying to support multi-range requests to resources
which use slice module.

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

In the beginning, we have just one such customer, and the client of
the customer gets 200 instead of 206. Now, we have some P2P clients
need to gather pieces that can not get from nearby. It is a huge
waste of resources if the clients get 200. In fact, this patch is
mainly to solve P2P's problem, and the requested ranges are indeed
in sequential order. This is the original request/idea. But since
the current implementation is too simple and too restrictive,
I want to make it better so I sent the patch.