Re: Regression with range fix

2011-08-31 Thread Joe Orton
On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote:
 The first regression report, though slightly too late for the vote:
 
 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825
 
 The byterange_filter.c in the Debian update is exactly the one from 
 2.2.20. I will keep you updated.

Hi; I'm just back from holiday and catching up.

The behaviour changes in the patch which could feasibly break 
non-compliant clients are:

a) using 200 in some cases where a 206 response would end up being 
larger

b) using a chunked response where previously C-L was always used, in 
cases where =32 ranges are being returned

Anything else to watch out for?

Looking at the patch in 2.2.x; there is a lot of effort expended 
deadling with apr_bucket_split() returning ENOTIMPL - that looks 
unnecessary; the filter will only handle brigades containing buckets 
with known length, and all such buckets must be _split-able.

Regards, Joe


RE: Regression with range fix

2011-08-31 Thread Plüm, Rüdiger, VF-Group
 

 -Original Message-
 From: Joe Orton [mailto:jor...@redhat.com] 
 Sent: Mittwoch, 31. August 2011 11:13
 To: dev@httpd.apache.org
 Subject: Re: Regression with range fix
 
 On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote:
  The first regression report, though slightly too late for the vote:
  
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825
  
  The byterange_filter.c in the Debian update is exactly the one from 
  2.2.20. I will keep you updated.
 
 Hi; I'm just back from holiday and catching up.
 
 The behaviour changes in the patch which could feasibly break 
 non-compliant clients are:
 
 a) using 200 in some cases where a 206 response would end up being 
 larger
 
 b) using a chunked response where previously C-L was always used, in 
 cases where =32 ranges are being returned
 
 Anything else to watch out for?
 
 Looking at the patch in 2.2.x; there is a lot of effort expended 
 deadling with apr_bucket_split() returning ENOTIMPL - that looks 
 unnecessary; the filter will only handle brigades containing buckets 
 with known length, and all such buckets must be _split-able.

So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks?

Regards

Rüdiger



Re: Regression with range fix

2011-08-31 Thread Jim Jagielski

On Aug 31, 2011, at 8:21 AM, Plüm, Rüdiger, VF-Group wrote:

 
 
 -Original Message-
 From: Joe Orton [mailto:jor...@redhat.com] 
 Sent: Mittwoch, 31. August 2011 11:13
 To: dev@httpd.apache.org
 Subject: Re: Regression with range fix
 
 On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote:
 The first regression report, though slightly too late for the vote:
 
 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825
 
 The byterange_filter.c in the Debian update is exactly the one from 
 2.2.20. I will keep you updated.
 
 Hi; I'm just back from holiday and catching up.
 
 The behaviour changes in the patch which could feasibly break 
 non-compliant clients are:
 
 a) using 200 in some cases where a 206 response would end up being 
 larger
 
 b) using a chunked response where previously C-L was always used, in 
 cases where =32 ranges are being returned
 
 Anything else to watch out for?
 
 Looking at the patch in 2.2.x; there is a lot of effort expended 
 deadling with apr_bucket_split() returning ENOTIMPL - that looks 
 unnecessary; the filter will only handle brigades containing buckets 
 with known length, and all such buckets must be _split-able.
 
 So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks?
 

Belt and suspenders?



Re: Regression with range fix

2011-08-31 Thread Stefan Fritsch
On Wednesday 31 August 2011, Joe Orton wrote:
 On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote:
  The first regression report, though slightly too late for the
  vote:
  
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825
  
  The byterange_filter.c in the Debian update is exactly the one
  from 2.2.20. I will keep you updated.
 
 Hi; I'm just back from holiday and catching up.
 
 The behaviour changes in the patch which could feasibly break
 non-compliant clients are:
 
 a) using 200 in some cases where a 206 response would end up being
 larger

For the first user who complained, I think this is the problem. His 
client does a Range: bytes=0- request. I suspect the 200 (instead of 
206) response to this request lets the client assume that the server 
does not support ranges at all. I will try to get this verified.

 
 b) using a chunked response where previously C-L was always used,
 in cases where =32 ranges are being returned

 Anything else to watch out for?

c) a request with a byterange beyond the end of the file used to 
return 416 but now returns 200. This is a violation of a RFC2616 
SHOULD. We didn't catch this when testing.
This is how mplayer seems to determine that it has reached the end of 
file. This seems a rather stupid thing to do unless mplayer assumes 
that the file may grow. But as it's a SHOULD, we should fix it.


Re: Regression with range fix

2011-08-31 Thread William A. Rowe Jr.
On 8/31/2011 4:00 PM, Stefan Fritsch wrote:
 On Wednesday 31 August 2011, Joe Orton wrote:

 Anything else to watch out for?
 
 c) a request with a byterange beyond the end of the file used to 
 return 416 but now returns 200. This is a violation of a RFC2616 
 SHOULD. We didn't catch this when testing.
 This is how mplayer seems to determine that it has reached the end of 
 file. This seems a rather stupid thing to do unless mplayer assumes 
 that the file may grow. But as it's a SHOULD, we should fix it.

Yup, that's a major flaw.  If one range may be satisfied, we may not
return 416, but must return those ranges which we are able.  If no
ranges are satisfiable, the answer needs to be restored to 416.