tahoe-lafs/trac-2024-07-25

Web-API does not correctly handle partial Range header #537

New issue

Closed

opened 2008-11-18 13:59:37 +00:00 by francois · 8 comments

francois commented

2008-11-18 13:59:37 +00:00

Owner

An HTTP request containing a partial range header raise the following exception even though it must be accepted according to RFC2616 section "14.35.1 Byte Ranges".

2008-11-18 14:45:23+0100 [HTTPChannel,4,127.0.0.1] Unhandled Error
	Traceback (most recent call last):
	  File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 186, in addCallbacks
	    self._runCallbacks()
	  File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 328, in _runCallbacks
	    self.result = callback(self.result, *args, **kw)
	  File "/home/francois/dev/tahoe/support/lib/python2.5/site-packages/Nevow-0.9.31-py2.5.egg/nevow/appserver.py", line 188, in _cbFinishRender
	    return self.gotPageContext(pageContext)
	  File "/home/francois/dev/tahoe/support/lib/python2.5/site-packages/Nevow-0.9.31-py2.5.egg/nevow/appserver.py", line 163, in gotPageContext
	    pageContext.tag.renderHTTP, pageContext
	--- <exception caught here> ---
	  File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 106, in maybeDeferred
	    result = f(*args, **kw)
	  File "/home/francois/dev/tahoe/src/allmydata/web/filenode.py", line 381, in renderHTTP
	    (str(offset), str(offset+size-1), str(filesize)))
	exceptions.TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

This request was generated by mplayer accessing an avi file stored in tahoe.

GET /file/URI%3ACHK%3foo%3bar/@@named=/test.avi HTTP/1.0

Host: localhost:8123
User-Agent: MPlayer/2:1.0~rc2-0ubuntu17-francois0
Icy-MetaData: 1
Range: bytes=2048-
Connection: close

An HTTP request containing a partial range header raise the following exception even though it must be accepted according to RFC2616 section "14.35.1 Byte Ranges". ``` 2008-11-18 14:45:23+0100 [HTTPChannel,4,127.0.0.1] Unhandled Error Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 186, in addCallbacks self._runCallbacks() File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 328, in _runCallbacks self.result = callback(self.result, *args, **kw) File "/home/francois/dev/tahoe/support/lib/python2.5/site-packages/Nevow-0.9.31-py2.5.egg/nevow/appserver.py", line 188, in _cbFinishRender return self.gotPageContext(pageContext) File "/home/francois/dev/tahoe/support/lib/python2.5/site-packages/Nevow-0.9.31-py2.5.egg/nevow/appserver.py", line 163, in gotPageContext pageContext.tag.renderHTTP, pageContext --- <exception caught here> --- File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 106, in maybeDeferred result = f(*args, **kw) File "/home/francois/dev/tahoe/src/allmydata/web/filenode.py", line 381, in renderHTTP (str(offset), str(offset+size-1), str(filesize))) exceptions.TypeError: unsupported operand type(s) for +: 'int' and 'NoneType' ``` This request was generated by mplayer accessing an avi file stored in tahoe. ``` GET /file/URI%3ACHK%3foo%3bar/@@named=/test.avi HTTP/1.0 Host: localhost:8123 User-Agent: MPlayer/2:1.0~rc2-0ubuntu17-francois0 Icy-MetaData: 1 Range: bytes=2048- Connection: close ```

tahoe-lafs added the

labels 2008-11-18 13:59:37 +00:00

tahoe-lafs added this to the undecided milestone 2008-11-18 13:59:37 +00:00

francois commented

2008-11-18 14:00:05 +00:00

Author

Owner

Attachment fix-http-range.patch (90391 bytes) added

Fix this bug and add associated tests

**Attachment** fix-http-range.patch (90391 bytes) added Fix this bug and add associated tests

fix-http-range.patch

88 KiB

zooko commented

2008-11-18 17:52:11 +00:00

Author

Owner

Thank you, francois!

I will apply this patch and test it and commit it as soon as possible, but first I have to figure out why my machine (yukyuk hardy) fails the test_unicode_filename test with your previous contribution.

Thank you, francois! I will apply this patch and test it and commit it as soon as possible, but first I have to figure out why my machine (yukyuk hardy) fails the test_unicode_filename test with your previous contribution.

warner commented

2008-12-06 05:08:17 +00:00

Author

Owner

Applied, in changeset:db7ad6da128609d2. Excellent patch! Thanks!

tahoe-lafs added the

fixed

label 2008-12-06 05:08:17 +00:00

tahoe-lafs modified the milestone from undecided to 1.3.0

2008-12-06 05:08:17 +00:00

warner closed this issue

2008-12-06 05:08:17 +00:00

warner commented

2008-12-07 22:12:28 +00:00

Author

Owner

oh, do you know if we must also handle something like "-1000", in which the start of the range is implicitly "0" or "1" or whatever the Range header uses?

Tahoe currently throws an exception if you do something like "curl -r-1000 FILEURL".

oh, do you know if we must also handle something like "-1000", in which the start of the range is implicitly "0" or "1" or whatever the Range header uses? Tahoe currently throws an exception if you do something like "curl -r-1000 FILEURL".

francois commented

2008-12-08 09:57:14 +00:00

Author

Owner

From my reading of RFC2616, the first-byte-pos is required. However, the recommended action in case of invalid Range header is to ignore the header altogether instead of raising an exception.

Excerpt from section "14.35.1 Byte Ranges":

If the last-byte-pos value is present, it MUST be greater than or
equal to the first-byte-pos in that byte-range-spec, or the byte-
range-spec is syntactically invalid. The recipient of a byte-range-
set that includes one or more syntactically invalid byte-range-spec
values MUST ignore the header field that includes that byte-range-
set.

From my reading of RFC2616, the first-byte-pos is required. However, the recommended action in case of invalid Range header is to ignore the header altogether instead of raising an exception. Excerpt from section "14.35.1 Byte Ranges": ``` If the last-byte-pos value is present, it MUST be greater than or equal to the first-byte-pos in that byte-range-spec, or the byte- range-spec is syntactically invalid. The recipient of a byte-range- set that includes one or more syntactically invalid byte-range-spec values MUST ignore the header field that includes that byte-range- set. ```

warner commented

2008-12-08 21:07:17 +00:00

Author

Owner

Oh, joy, actually it looks like "-1000" means the last 1000 bytes of the file. From the last section of RFC2616.14.35.1:

Examples of byte-ranges-specifier values (assuming an entity-body of length 10000):

      - The first 500 bytes (byte offsets 0-499, inclusive):  bytes=0-
        499

      - The second 500 bytes (byte offsets 500-999, inclusive):
        bytes=500-999

      - The final 500 bytes (byte offsets 9500-9999, inclusive):
        bytes=-500

      - Or bytes=9500-

      - The first and last bytes only (bytes 0 and 9999):  bytes=0-0,-1

      - Several legal but not canonical specifications of the second 500
        bytes (byte offsets 500-999, inclusive):
         bytes=500-600,601-999
         bytes=500-700,601-999

Sigh. So, I think I'll just leave this the way it is until someone complains.

Oh, joy, actually it looks like "-1000" means the *last* 1000 bytes of the file. From the last section of RFC2616.14.35.1: ``` Examples of byte-ranges-specifier values (assuming an entity-body of length 10000): - The first 500 bytes (byte offsets 0-499, inclusive): bytes=0- 499 - The second 500 bytes (byte offsets 500-999, inclusive): bytes=500-999 - The final 500 bytes (byte offsets 9500-9999, inclusive): bytes=-500 - Or bytes=9500- - The first and last bytes only (bytes 0 and 9999): bytes=0-0,-1 - Several legal but not canonical specifications of the second 500 bytes (byte offsets 500-999, inclusive): bytes=500-600,601-999 bytes=500-700,601-999 ``` Sigh. So, I think I'll just leave this the way it is until someone complains.

francois commented

2008-12-09 11:20:14 +00:00

Author

Owner

Replying to warner:

Oh, joy, actually it looks like "-1000" means the last 1000 bytes of the file. From the last section of RFC2616.14.35.1:

Oops, my bad, I read the RFC too quickly.

Sigh. So, I think I'll just leave this the way it is until someone complains.

Agreed, then will probably be the right time to add support for multiple range specifiers.

Replying to [warner](/tahoe-lafs/trac-2024-07-25/issues/537#issuecomment-110118): > Oh, joy, actually it looks like "-1000" means the *last* 1000 bytes of the file. From the last section of RFC2616.14.35.1: Oops, my bad, I read the RFC too quickly. > Sigh. So, I think I'll just leave this the way it is until someone complains. Agreed, then will probably be the right time to add support for multiple range specifiers.

jsgf commented

2010-04-23 23:50:23 +00:00

Author

Owner

BTW, I managed to overlook this ticket and filed #989 to address this same concern. I also attached a patch which handles suffix byte ranges ("-1000").