Rev 2575: (Vincent Ladeuil) Fix #115209 - Unable to handle http code 400: Bad Request When issuing too many ranges in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Jul 3 08:35:47 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2575
revision-id: pqm at pqm.ubuntu.com-20070703073546-9dv8bocak8u3ou6m
parent: pqm at pqm.ubuntu.com-20070703064804-tphz64uvb1bhdj78
parent: ian.clatworthy at internode.on.net-20070703070332-45j7qw8z03fnulav
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2007-07-03 08:35:46 +0100
message:
  (Vincent Ladeuil) Fix #115209 - Unable to handle http code 400: Bad Request When issuing too many ranges
modified:
  bzrlib/tests/HTTPTestUtil.py   HTTPTestUtil.py-20050914180604-247d3aafb7a43343
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
  bzrlib/transport/http/response.py _response.py-20060613154423-a2ci7hd4iw5c7fnt-1
    ------------------------------------------------------------
    revno: 2574.1.1
    merged: ian.clatworthy at internode.on.net-20070703070332-45j7qw8z03fnulav
    parent: pqm at pqm.ubuntu.com-20070703064804-tphz64uvb1bhdj78
    parent: v.ladeuil+lp at free.fr-20070621081747-xj6dft70oigy1s8l
    committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
    branch nick: ianc-integration
    timestamp: Tue 2007-07-03 17:03:32 +1000
    message:
      (Vincent Ladeuil) Fix #115209 - Unable to handle http code 400: Bad Request When issuing too many ranges
    ------------------------------------------------------------
    revno: 2520.2.3
    merged: v.ladeuil+lp at free.fr-20070621081747-xj6dft70oigy1s8l
    parent: v.ladeuil+lp at free.fr-20070620135621-x43c0hnmzu0iuo6m
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 115209
    timestamp: Thu 2007-06-21 10:17:47 +0200
    message:
      Take aaron's remark into account.
      
      * bzrlib/tests/HTTPTestUtil.py:
      (LimitedRangeRequestHandler.get_multiple_ranges): Oops, remove
      commented-out pdb usage.
    ------------------------------------------------------------
    revno: 2520.2.2
    merged: v.ladeuil+lp at free.fr-20070620135621-x43c0hnmzu0iuo6m
    parent: v.ladeuil+lp at free.fr-20070612144857-1swhz5c3qo8n76l8
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 115209
    timestamp: Wed 2007-06-20 15:56:21 +0200
    message:
      Fix #115209 by issuing a single range request on 400: Bad Request
      
      * bzrlib/transport/http/response.py:
      (handle_response): Consider 400 as an indication that too much
      ranges were specified.
      
      * bzrlib/transport/http/_urllib2_wrappers.py:
      (Request): Add an 'accpeted_errors' parameters describing what
      error codes the caller will handle.
      (HTTPErrorProcessor): Mention that Request specific accepted error
      codes takes precedence.
      (HTTPDefaultErrorHandler.http_error_default): Remove dead code.
      
      * bzrlib/transport/http/_urllib.py:
      (HttpTransport_urllib._get): Add 400 as an accepted error iff
      ranges are specified.
      (HttpTransport_urllib._head): Restrict accepted errors.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase._degrade_range_hint,
      HttpTransportBase._get_ranges_hinted): Replace _retry_get.
      (HttpTransportBase.readv): Simplified and avoid the spurious _get()
      issued when _get was successful.
      
      * bzrlib/tests/test_http.py:
      (TestLimitedRangeRequestServer,
      TestLimitedRangeRequestServer_urllib,
      TestLimitedRangeRequestServer_pycurl): Bug #115209 specific tests.
      
      * bzrlib/tests/HTTPTestUtil.py:
      (LimitedRangeRequestHandler, LimitedRangeHTTPServer): New test
      classes to emulate apache throwing 400: Bad Request when too much
      ranges are specified.
      (AuthRequestHandler.do_GET): Remove dead code. Yeah, I know,
      not related to the bug :-/
    ------------------------------------------------------------
    revno: 2520.2.1
    merged: v.ladeuil+lp at free.fr-20070612144857-1swhz5c3qo8n76l8
    parent: pqm at pqm.ubuntu.com-20070608134340-flu6dlpzyo7izrrs
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 115209
    timestamp: Tue 2007-06-12 16:48:57 +0200
    message:
      First step to fix #115209 use _coalesce_offsets like other transports.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase._bytes_to_read_before_seek): Setting it to 512
      to minimize additional bytes transferred but still reducing number
      of ranges.
      (HttpTransportBase.readv): _coalesce_offsets does a better job
      than offset_to_ranges/ Reduce code duplication too.
      (HttpTransportBase.offsets_to_ranges): Deprecated.
      (HttpTransportBase._attempted_range_header): Made private. Adapted
      to offsets.
      (HttpTransportBase._range_header): Made private. Adapted to
      offsets.
      
      * bzrlib/transport/http/_urllib.py:
      (HttpTransport_urllib._get): s/ranges/offsets/
      
      * bzrlib/transport/http/_pycurl.py:
      (PyCurlTransport._get, PyCurlTransport._get_ranged): s/ranges/offsets/
      
      * bzrlib/tests/__init__.py:
      (Transport._coalesce_offsets): Fix minor typo.
      
      * bzrlib/tests/test_http.py:
      (TestOffsets): Deleted. We rely on Transport._coalesce_offsets
      only now.
      (TestRangeHeader.check_header): Convert ranges to offets and use
      Transport._coalesce_offsets.
      (TestRangeHeader.test_range_header_single): Use tuples not lists !
      (TestRanges._file_contents): Convert ranges to offets and use
      Transport._coalesce_offsets. And fix indentation too.
      (TestRanges._file_tail): Fix indentation.
      (TestRanges.test_range_header): Use _file_contents so that we can
      still use ranges.
=== modified file 'bzrlib/tests/HTTPTestUtil.py'
--- a/bzrlib/tests/HTTPTestUtil.py	2007-06-24 15:16:40 +0000
+++ b/bzrlib/tests/HTTPTestUtil.py	2007-07-03 07:03:32 +0000
@@ -150,6 +150,36 @@
         self.wfile.write(out_buffer.getvalue())
 
 
+class LimitedRangeRequestHandler(TestingHTTPRequestHandler):
+    """Errors out when range specifiers exceed the limit"""
+
+    def get_multiple_ranges(self, file, file_size, ranges):
+        """Refuses the multiple ranges request"""
+        tcs = self.server.test_case_server
+        if tcs.range_limit is not None and len(ranges) > tcs.range_limit:
+            file.close()
+            # Emulate apache behavior
+            self.send_error(400, "Bad Request")
+            return
+        return TestingHTTPRequestHandler.get_multiple_ranges(self, file,
+                                                             file_size, ranges)
+
+    def do_GET(self):
+        tcs = self.server.test_case_server
+        tcs.GET_request_nb += 1
+        return TestingHTTPRequestHandler.do_GET(self)
+
+
+class LimitedRangeHTTPServer(HttpServer):
+    """An HttpServer erroring out on requests with too much range specifiers"""
+
+    def __init__(self, request_handler=LimitedRangeRequestHandler,
+                 range_limit=None):
+        HttpServer.__init__(self, request_handler)
+        self.range_limit = range_limit
+        self.GET_request_nb = 0
+
+
 class SingleRangeRequestHandler(TestingHTTPRequestHandler):
     """Always reply to range request as if they were single.
 
@@ -337,8 +367,6 @@
             self.end_headers()
             return
 
-        TestingHTTPRequestHandler.do_GET(self)
-
 
 class BasicAuthRequestHandler(AuthRequestHandler):
     """Implements the basic authentication of a request"""

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-06-24 15:16:40 +0000
+++ b/bzrlib/tests/test_http.py	2007-07-03 07:03:32 +0000
@@ -53,6 +53,7 @@
     HTTPDigestAuthServer,
     HTTPServerRedirecting,
     InvalidStatusRequestHandler,
+    LimitedRangeHTTPServer,
     NoRangeRequestHandler,
     ProxyBasicAuthServer,
     ProxyDigestAuthServer,
@@ -65,6 +66,7 @@
     WallRequestHandler,
     )
 from bzrlib.transport import (
+    _CoalescedOffset,
     do_catching_redirections,
     get_transport,
     Transport,
@@ -361,30 +363,6 @@
         self.assertIsInstance(t, HttpTransport_urllib)
 
 
-class TestOffsets(TestCase):
-    """Test offsets_to_ranges method"""
-
-    def test_offsets_to_ranges_simple(self):
-        to_range = HttpTransportBase.offsets_to_ranges
-        ranges = to_range([(10, 1)])
-        self.assertEqual([[10, 10]], ranges)
-
-        ranges = to_range([(0, 1), (1, 1)])
-        self.assertEqual([[0, 1]], ranges)
-
-        ranges = to_range([(1, 1), (0, 1)])
-        self.assertEqual([[0, 1]], ranges)
-
-    def test_offset_to_ranges_overlapped(self):
-        to_range = HttpTransportBase.offsets_to_ranges
-
-        ranges = to_range([(10, 1), (20, 2), (22, 5)])
-        self.assertEqual([[10, 10], [20, 26]], ranges)
-
-        ranges = to_range([(10, 1), (11, 2), (22, 5)])
-        self.assertEqual([[10, 12], [22, 26]], ranges)
-
-
 class TestPost(object):
 
     def _test_post_body_is_received(self, scheme):
@@ -427,12 +405,15 @@
     """Test range_header method"""
 
     def check_header(self, value, ranges=[], tail=0):
-        range_header = HttpTransportBase.range_header
-        self.assertEqual(value, range_header(ranges, tail))
+        offsets = [ (start, end - start + 1) for start, end in ranges]
+        coalesce = Transport._coalesce_offsets
+        coalesced = list(coalesce(offsets, limit=0, fudge_factor=0))
+        range_header = HttpTransportBase._range_header
+        self.assertEqual(value, range_header(coalesced, tail))
 
     def test_range_header_single(self):
-        self.check_header('0-9', ranges=[[0,9]])
-        self.check_header('100-109', ranges=[[100,109]])
+        self.check_header('0-9', ranges=[(0,9)])
+        self.check_header('100-109', ranges=[(100,109)])
 
     def test_range_header_tail(self):
         self.check_header('-10', tail=10)
@@ -734,6 +715,67 @@
     """Tests range requests refusing server for pycurl implementation"""
 
 
+class TestLimitedRangeRequestServer(object):
+    """Tests readv requests against server that errors out on too much ranges.
+
+    This MUST be used by daughter classes that also inherit from
+    TestCaseWithWebserver.
+
+    We can't inherit directly from TestCaseWithWebserver or the
+    test framework will try to create an instance which cannot
+    run, its implementation being incomplete.
+    """
+
+    range_limit = 3
+
+    def create_transport_readonly_server(self):
+        # Requests with more range specifiers will error out
+        return LimitedRangeHTTPServer(range_limit=self.range_limit)
+
+    def get_transport(self):
+        return self._transport(self.get_readonly_server().get_url())
+
+    def setUp(self):
+        TestCaseWithWebserver.setUp(self)
+        # We need to manipulate ranges that correspond to real chunks in the
+        # response, so we build a content appropriately.
+        filler = ''.join(['abcdefghij' for _ in range(102)])
+        content = ''.join(['%04d' % v + filler for v in range(16)])
+        self.build_tree_contents([('a', content)],)
+
+    def test_few_ranges(self):
+        t = self.get_transport()
+        l = list(t.readv('a', ((0, 4), (1024, 4), )))
+        self.assertEqual(l[0], (0, '0000'))
+        self.assertEqual(l[1], (1024, '0001'))
+        self.assertEqual(1, self.get_readonly_server().GET_request_nb)
+
+    def test_a_lot_of_ranges(self):
+        t = self.get_transport()
+        l = list(t.readv('a', ((0, 4), (1024, 4), (4096, 4), (8192, 4))))
+        self.assertEqual(l[0], (0, '0000'))
+        self.assertEqual(l[1], (1024, '0001'))
+        self.assertEqual(l[2], (4096, '0004'))
+        self.assertEqual(l[3], (8192, '0008'))
+        # The server will refuse to serve the first request (too much ranges),
+        # a second request will succeeds.
+        self.assertEqual(2, self.get_readonly_server().GET_request_nb)
+
+
+class TestLimitedRangeRequestServer_urllib(TestLimitedRangeRequestServer,
+                                          TestCaseWithWebserver):
+    """Tests limited range requests server for urllib implementation"""
+
+    _transport = HttpTransport_urllib
+
+
+class TestLimitedRangeRequestServer_pycurl(TestWithTransport_pycurl,
+                                          TestLimitedRangeRequestServer,
+                                          TestCaseWithWebserver):
+    """Tests limited range requests server for pycurl implementation"""
+
+
+
 class TestHttpProxyWhiteBox(TestCase):
     """Whitebox test proxy http authorization.
 
@@ -926,18 +968,21 @@
         server = self.get_readonly_server()
         self.transport = self._transport(server.get_url())
 
-    def _file_contents(self, relpath, ranges, tail_amount=0):
-         code, data = self.transport._get(relpath, ranges)
-         self.assertTrue(code in (200, 206),'_get returns: %d' % code)
-         for start, end in ranges:
-             data.seek(start)
-             yield data.read(end - start + 1)
+    def _file_contents(self, relpath, ranges):
+        offsets = [ (start, end - start + 1) for start, end in ranges]
+        coalesce = self.transport._coalesce_offsets
+        coalesced = list(coalesce(offsets, limit=0, fudge_factor=0))
+        code, data = self.transport._get(relpath, coalesced)
+        self.assertTrue(code in (200, 206),'_get returns: %d' % code)
+        for start, end in ranges:
+            data.seek(start)
+            yield data.read(end - start + 1)
 
     def _file_tail(self, relpath, tail_amount):
-         code, data = self.transport._get(relpath, [], tail_amount)
-         self.assertTrue(code in (200, 206),'_get returns: %d' % code)
-         data.seek(-tail_amount + 1, 2)
-         return data.read(tail_amount)
+        code, data = self.transport._get(relpath, [], tail_amount)
+        self.assertTrue(code in (200, 206),'_get returns: %d' % code)
+        data.seek(-tail_amount + 1, 2)
+        return data.read(tail_amount)
 
     def test_range_header(self):
         # Valid ranges
@@ -946,11 +991,11 @@
         # Tail
         self.assertEqual('789', self._file_tail('a', 3))
         # Syntactically invalid range
-        self.assertRaises(errors.InvalidRange,
-                          self.transport._get, 'a', [(4, 3)])
+        self.assertListRaises(errors.InvalidRange,
+                          self._file_contents, 'a', [(4, 3)])
         # Semantically invalid range
-        self.assertRaises(errors.InvalidRange,
-                          self.transport._get, 'a', [(42, 128)])
+        self.assertListRaises(errors.InvalidRange,
+                          self._file_contents, 'a', [(42, 128)])
 
 
 class TestRanges_urllib(TestRanges, TestCaseWithWebserver):

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-06-28 05:19:04 +0000
+++ b/bzrlib/transport/__init__.py	2007-07-03 07:03:32 +0000
@@ -574,7 +574,7 @@
         :param fudge_factor: All transports have some level of 'it is
                 better to read some more data and throw it away rather 
                 than seek', so collapse if we are 'close enough'
-        :return: yield _CoalescedOffset objects, which have members for wher
+        :return: yield _CoalescedOffset objects, which have members for where
                 to start, how much to read, and how to split those 
                 chunks back up
         """

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-06-27 04:33:04 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-07-03 07:03:32 +0000
@@ -28,8 +28,13 @@
 
 from bzrlib import errors, ui
 from bzrlib.smart import medium
+from bzrlib.symbol_versioning import (
+        deprecated_method,
+        zero_seventeen,
+        )
 from bzrlib.trace import mutter
 from bzrlib.transport import (
+    _CoalescedOffset,
     Transport,
     )
 
@@ -231,7 +236,7 @@
 
         :param relpath: Path relative to transport base URL
         :param ranges: None to get the whole file;
-            or [(start,end)+], a list of tuples to fetch parts of a file.
+            or  a list of _CoalescedOffset to fetch parts of a file.
         :param tail_amount: The amount to get from the end of the file.
 
         :returns: (http_code, result_file)
@@ -249,75 +254,102 @@
         """
         return self
 
-    def _retry_get(self, relpath, ranges, exc_info):
-        """A GET request have failed, let's retry with a simpler request."""
-
-        try_again = False
-        # The server does not gives us enough data or
-        # a bogus-looking result, let's try again with
-        # a simpler request if possible.
+    def _degrade_range_hint(self, relpath, ranges, exc_info):
         if self._range_hint == 'multi':
             self._range_hint = 'single'
-            mutter('Retry %s with single range request' % relpath)
-            try_again = True
+            mutter('Retry "%s" with single range request' % relpath)
         elif self._range_hint == 'single':
             self._range_hint = None
-            mutter('Retry %s without ranges' % relpath)
-            try_again = True
-        if try_again:
-            # Note that since the offsets and the ranges may not
-            # be in the same order, we don't try to calculate a
-            # restricted single range encompassing unprocessed
-            # offsets.
-            code, f = self._get(relpath, ranges)
-            return try_again, code, f
+            mutter('Retry "%s" without ranges' % relpath)
         else:
-            # We tried all the tricks, but nothing worked. We
-            # re-raise original exception; the 'mutter' calls
-            # above will indicate that further tries were
-            # unsuccessful
+            # We tried all the tricks, but nothing worked. We re-raise original
+            # exception; the 'mutter' calls above will indicate that further
+            # tries were unsuccessful
             raise exc_info[0], exc_info[1], exc_info[2]
 
-    def readv(self, relpath, offsets):
-        """Get parts of the file at the given relative path.
-
-        :param offsets: A list of (offset, size) tuples.
-        :param return: A list or generator of (offset, data) tuples
+    def _get_ranges_hinted(self, relpath, ranges):
+        """Issue a ranged GET request taking server capabilities into account.
+
+        Depending of the errors returned by the server, we try several GET
+        requests, trying to minimize the data transferred.
+
+        :param relpath: Path relative to transport base URL
+        :param ranges: None to get the whole file;
+            or  a list of _CoalescedOffset to fetch parts of a file.
+        :returns: A file handle containing at least the requested ranges.
         """
-        ranges = self.offsets_to_ranges(offsets)
-        mutter('http readv of %s collapsed %s offsets => %s',
-                relpath, len(offsets), ranges)
-
+        exc_info = None
         try_again = True
         while try_again:
             try_again = False
             try:
                 code, f = self._get(relpath, ranges)
-            except (errors.InvalidRange, errors.ShortReadvError), e:
-                try_again, code, f = self._retry_get(relpath, ranges,
-                                                     sys.exc_info())
-
+            except errors.InvalidRange, e:
+                if exc_info is None:
+                    exc_info = sys.exc_info()
+                self._degrade_range_hint(relpath, ranges, exc_info)
+                try_again = True
+        return f
+
+    # _coalesce_offsets is a helper for readv, it try to combine ranges without
+    # degrading readv performances. _bytes_to_read_before_seek is the value
+    # used for the limit parameter and has been tuned for other transports. For
+    # HTTP, the name is inappropriate but the parameter is still useful and
+    # helps reduce the number of chunks in the response. The overhead for a
+    # chunk (headers, length, footer around the data itself is variable but
+    # around 50 bytes. We use 128 to reduce the range specifiers that appear in
+    # the header, some servers (notably Apache) enforce a maximum length for a
+    # header and issue a '400: Bad request' error when too much ranges are
+    # specified.
+    _bytes_to_read_before_seek = 128
+    # No limit on the offset number that get combined into one, we are trying
+    # to avoid downloading the whole file.
+    _max_readv_combined = 0
+
+    def readv(self, relpath, offsets):
+        """Get parts of the file at the given relative path.
+
+        :param offsets: A list of (offset, size) tuples.
+        :param return: A list or generator of (offset, data) tuples
+        """
+        sorted_offsets = sorted(list(offsets))
+        fudge = self._bytes_to_read_before_seek
+        coalesced = self._coalesce_offsets(sorted_offsets,
+                                           limit=self._max_readv_combine,
+                                           fudge_factor=fudge)
+        coalesced = list(coalesced)
+        mutter('http readv of %s  offsets => %s collapsed %s',
+                relpath, len(offsets), len(coalesced))
+
+        f = self._get_ranges_hinted(relpath, coalesced)
         for start, size in offsets:
             try_again = True
             while try_again:
                 try_again = False
-                f.seek(start, (start < 0) and 2 or 0)
+                f.seek(start, ((start < 0) and 2) or 0)
                 start = f.tell()
                 try:
                     data = f.read(size)
                     if len(data) != size:
                         raise errors.ShortReadvError(relpath, start, size,
                                                      actual=len(data))
-                except (errors.InvalidRange, errors.ShortReadvError), e:
-                    # Note that we replace 'f' here and that it
-                    # may need cleaning one day before being
-                    # thrown that way.
-                    try_again, code, f = self._retry_get(relpath, ranges,
-                                                         sys.exc_info())
+                except errors.ShortReadvError, e:
+                    self._degrade_range_hint(relpath, coalesced, sys.exc_info())
+
+                    # Since the offsets and the ranges may not be in the same
+                    # order, we don't try to calculate a restricted single
+                    # range encompassing unprocessed offsets.
+
+                    # Note: we replace 'f' here, it may need cleaning one day
+                    # before being thrown that way.
+                    f = self._get_ranges_hinted(relpath, coalesced)
+                    try_again = True
+
             # After one or more tries, we get the data.
             yield start, data
 
     @staticmethod
+    @deprecated_method(zero_seventeen)
     def offsets_to_ranges(offsets):
         """Turn a list of offsets and sizes into a list of byte ranges.
 
@@ -452,37 +484,40 @@
         else:
             return self.__class__(self.abspath(offset), self)
 
-    def attempted_range_header(self, ranges, tail_amount):
+    def _attempted_range_header(self, offsets, tail_amount):
         """Prepare a HTTP Range header at a level the server should accept"""
 
         if self._range_hint == 'multi':
             # Nothing to do here
-            return self.range_header(ranges, tail_amount)
+            return self._range_header(offsets, tail_amount)
         elif self._range_hint == 'single':
             # Combine all the requested ranges into a single
             # encompassing one
-            if len(ranges) > 0:
-                start, ignored = ranges[0]
-                ignored, end = ranges[-1]
+            if len(offsets) > 0:
                 if tail_amount not in (0, None):
-                    # Nothing we can do here to combine ranges
-                    # with tail_amount, just returns None. The
-                    # whole file should be downloaded.
+                    # Nothing we can do here to combine ranges with tail_amount
+                    # in a single range, just returns None. The whole file
+                    # should be downloaded.
                     return None
                 else:
-                    return self.range_header([(start, end)], 0)
+                    start = offsets[0].start
+                    last = offsets[-1]
+                    end = last.start + last.length - 1
+                    whole = self._coalesce_offsets([(start, end - start + 1)],
+                                                   limit=0, fudge_factor=0)
+                    return self._range_header(list(whole), 0)
             else:
                 # Only tail_amount, requested, leave range_header
                 # do its work
-                return self.range_header(ranges, tail_amount)
+                return self._range_header(offsets, tail_amount)
         else:
             return None
 
     @staticmethod
-    def range_header(ranges, tail_amount):
+    def _range_header(ranges, tail_amount):
         """Turn a list of bytes ranges into a HTTP Range header value.
 
-        :param ranges: A list of byte ranges, (start, end).
+        :param ranges: A list of _CoalescedOffset
         :param tail_amount: The amount to get from the end of the file.
 
         :return: HTTP range header string.
@@ -491,8 +526,9 @@
         provided.
         """
         strings = []
-        for start, end in ranges:
-            strings.append('%d-%d' % (start, end))
+        for offset in ranges:
+            strings.append('%d-%d' % (offset.start,
+                                      offset.start + offset.length - 1))
 
         if tail_amount:
             strings.append('-%d' % tail_amount)

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2007-05-06 06:59:57 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2007-06-12 14:48:57 +0000
@@ -136,10 +136,10 @@
         else:
             self._raise_curl_http_error(curl)
 
-    def _get(self, relpath, ranges, tail_amount=0):
+    def _get(self, relpath, offsets, tail_amount=0):
         # This just switches based on the type of request
-        if ranges is not None or tail_amount not in (0, None):
-            return self._get_ranged(relpath, ranges, tail_amount=tail_amount)
+        if offsets is not None or tail_amount not in (0, None):
+            return self._get_ranged(relpath, offsets, tail_amount=tail_amount)
         else:
             return self._get_full(relpath)
 
@@ -189,12 +189,12 @@
 
         return code, data
 
-    def _get_ranged(self, relpath, ranges, tail_amount):
+    def _get_ranged(self, relpath, offsets, tail_amount):
         """Make a request for just part of the file."""
         curl = self._curl
         abspath, data, header = self._setup_get_request(curl, relpath)
 
-        range_header = self.attempted_range_header(ranges, tail_amount)
+        range_header = self._attempted_range_header(offsets, tail_amount)
         if range_header is None:
             # Forget ranges, the server can't handle them
             return self._get_full(relpath)

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2007-04-22 16:32:04 +0000
+++ b/bzrlib/transport/http/_urllib.py	2007-06-20 13:56:21 +0000
@@ -113,18 +113,22 @@
 
         return response
 
-    def _get(self, relpath, ranges, tail_amount=0):
+    def _get(self, relpath, offsets, tail_amount=0):
         """See HttpTransport._get"""
 
         abspath = self._real_abspath(relpath)
         headers = {}
-        if ranges or tail_amount:
-            range_header = self.attempted_range_header(ranges, tail_amount)
+        accepted_errors = [200, 404]
+        if offsets or tail_amount:
+            range_header = self._attempted_range_header(offsets, tail_amount)
             if range_header is not None:
+                accepted_errors.append(206)
+                accepted_errors.append(400)
                 bytes = 'bytes=' + range_header
                 headers = {'Range': bytes}
 
-        request = Request('GET', abspath, None, headers)
+        request = Request('GET', abspath, None, headers,
+                          accepted_errors=accepted_errors)
         response = self._perform(request)
 
         code = response.code
@@ -157,7 +161,8 @@
         Performs the request and leaves callers handle the results.
         """
         abspath = self._real_abspath(relpath)
-        request = Request('HEAD', abspath)
+        request = Request('HEAD', abspath,
+                          accepted_errors=[200, 404])
         response = self._perform(request)
 
         self._connection.fake_close()
@@ -172,7 +177,6 @@
         if code == 200: # "ok",
             return True
         else:
-            assert(code == 404, 'Only 200 or 404 are correct')
             return False
 
 

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2007-06-24 15:16:40 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2007-07-03 07:03:32 +0000
@@ -153,11 +153,13 @@
 
     def __init__(self, method, url, data=None, headers={},
                  origin_req_host=None, unverifiable=False,
-                 connection=None, parent=None,):
+                 connection=None, parent=None,
+                 accepted_errors=None):
         urllib2.Request.__init__(self, url, data, headers,
                                  origin_req_host, unverifiable)
         self.method = method
         self.connection = connection
+        self.accepted_errors = accepted_errors
         # To handle redirections
         self.parent = parent
         self.redirected_to = None
@@ -1138,13 +1140,24 @@
     instead, we leave our Transport handle them.
     """
 
+    accepted_errors = [200, # Ok
+                       206, # Partial content
+                       404, # Not found
+                       ]
+    """The error codes the caller will handle.
+
+    This can be specialized in the request on a case-by case basis, but the
+    common cases are covered here.
+    """
+
     def http_response(self, request, response):
         code, msg, hdrs = response.code, response.msg, response.info()
 
-        if code not in (200, # Ok
-                        206, # Partial content
-                        404, # Not found
-                        ):
+        accepted_errors = request.accepted_errors
+        if accepted_errors is None:
+            accepted_errors = self.accepted_errors
+
+        if code not in accepted_errors:
             response = self.parent.error('http', request, response,
                                          code, msg, hdrs)
         return response
@@ -1156,12 +1169,7 @@
     """Translate common errors into bzr Exceptions"""
 
     def http_error_default(self, req, fp, code, msg, hdrs):
-        if code == 404:
-            raise errors.NoSuchFile(req.get_selector(),
-                                    extra=HTTPError(req.get_full_url(),
-                                                    code, msg,
-                                                    hdrs, fp))
-        elif code == 403:
+        if code == 403:
             raise errors.TransportError('Server refuses to fullfil the request')
         elif code == 416:
             # We don't know which, but one of the ranges we
@@ -1173,6 +1181,7 @@
                                              'Unable to handle http code %d: %s'
                                              % (code, msg))
 
+
 class Opener(object):
     """A wrapper around urllib2.build_opener
 
@@ -1196,6 +1205,7 @@
             HTTPSHandler,
             HTTPDefaultErrorHandler,
             )
+
         self.open = self._opener.open
         if DEBUG >= 2:
             # When dealing with handler order, it's easy to mess

=== modified file 'bzrlib/transport/http/response.py'
--- a/bzrlib/transport/http/response.py	2006-12-11 08:10:33 +0000
+++ b/bzrlib/transport/http/response.py	2007-06-20 13:56:21 +0000
@@ -279,7 +279,9 @@
         return data
     elif code == 404:
         raise errors.NoSuchFile(url)
-    elif code == 416:
+    # Some servers will retun "400: Bad Request" when too much ranges are
+    # specified
+    elif code in (400, 416):
         # We don't know which, but one of the ranges we specified
         # was wrong. So we raise with 0 for a lack of a better
         # magic value.




More information about the bazaar-commits mailing list