Merge lp:~mbp/bzr/198646-http-boundary-2.3 into lp:bzr/2.3

[Martin Pool]

cope with proxies that drop the connection during a multipart response (bug 198646)

bug 198646 is that some buggy squids can close a connection in the middle of a mime multipart request, before the boundary line; users sometimes hit this and can't easily change their corporate proxy. hat tip to Henrik for the clear description.

This builds on a patch attached to that bug to handle this more cleanly, and it adds a test that does trigger the behaviour.

The test is a bit copy-pasty, but it does cover the case and it's not awful.

[Robert Collins (lifeless)]

It might be nice to note that retries will happen in the log, in case folk are confused/concerned by bzr being unexpectedly slow.

review: Approve

[Martin Pool (mbp)]

> It might be nice to note that retries will happen in the log, in case folk are
> confused/concerned by bzr being unexpectedly slow.

+1, it's not obvious here but _degrade_range_hint should be doing that.

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2011-01-20 21:15:10 +0000
+++ bzrlib/errors.py	2011-07-27 12:03:30 +0000
@@ -1722,6 +1722,19 @@
         InvalidHttpResponse.__init__(self, path, msg)
 
 
+class HttpBoundaryMissing(InvalidHttpResponse):
+    """A multipart response ends with no boundary marker.
+
+    This is a special case caused by buggy proxies, described in
+    <https://bugs.launchpad.net/bzr/+bug/198646>.
+    """
+
+    _fmt = "HTTP MIME Boundary missing for %(path)s: %(msg)s"
+
+    def __init__(self, path, msg):
+        InvalidHttpResponse.__init__(self, path, msg)
+
+
 class InvalidHttpContentType(InvalidHttpResponse):
 
     _fmt = 'Invalid http Content-type "%(ctype)s" for %(path)s: %(msg)s'

=== modified file 'bzrlib/tests/test_http.py'
--- bzrlib/tests/test_http.py	2011-01-12 22:21:05 +0000
+++ bzrlib/tests/test_http.py	2011-07-27 12:03:30 +0000
@@ -999,6 +999,72 @@
         self.assertEqual('single', t._range_hint)
 
 
+class TruncatedBeforeBoundaryRequestHandler(
+    http_server.TestingHTTPRequestHandler):
+    """Truncation before a boundary, like in bug 198646"""
+
+    _truncated_ranges = 1
+
+    def get_multiple_ranges(self, file, file_size, ranges):
+        self.send_response(206)
+        self.send_header('Accept-Ranges', 'bytes')
+        boundary = 'tagada'
+        self.send_header('Content-Type',
+                         'multipart/byteranges; boundary=%s' % boundary)
+        boundary_line = '--%s\r\n' % boundary
+        # Calculate the Content-Length
+        content_length = 0
+        for (start, end) in ranges:
+            content_length += len(boundary_line)
+            content_length += self._header_line_length(
+                'Content-type', 'application/octet-stream')
+            content_length += self._header_line_length(
+                'Content-Range', 'bytes %d-%d/%d' % (start, end, file_size))
+            content_length += len('\r\n') # end headers
+            content_length += end - start # + 1
+        content_length += len(boundary_line)
+        self.send_header('Content-length', content_length)
+        self.end_headers()
+
+        # Send the multipart body
+        cur = 0
+        for (start, end) in ranges:
+            if cur + self._truncated_ranges >= len(ranges):
+                # Abruptly ends the response and close the connection
+                self.close_connection = 1
+                return
+            self.wfile.write(boundary_line)
+            self.send_header('Content-type', 'application/octet-stream')
+            self.send_header('Content-Range', 'bytes %d-%d/%d'
+                             % (start, end, file_size))
+            self.end_headers()
+            self.send_range_content(file, start, end - start + 1)
+            cur += 1
+        # Final boundary
+        self.wfile.write(boundary_line)
+
+
+class TestTruncatedBeforeBoundary(TestSpecificRequestHandler):
+    """Tests the case of bug 198646, disconnecting before a boundary."""
+
+    _req_handler_class = TruncatedBeforeBoundaryRequestHandler
+
+    def setUp(self):
+        super(TestTruncatedBeforeBoundary, self).setUp()
+        self.build_tree_contents([('a', '0123456789')],)
+
+    def test_readv_with_short_reads(self):
+        server = self.get_readonly_server()
+        t = self.get_readonly_transport()
+        # Force separate ranges for each offset
+        t._bytes_to_read_before_seek = 0
+        ireadv = iter(t.readv('a', ((0, 1), (2, 1), (4, 2), (9, 1))))
+        self.assertEqual((0, '0'), ireadv.next())
+        self.assertEqual((2, '2'), ireadv.next())
+        self.assertEqual((4, '45'), ireadv.next())
+        self.assertEqual((9, '9'), ireadv.next())
+
+
 class LimitedRangeRequestHandler(http_server.TestingHTTPRequestHandler):
     """Errors out when range specifiers exceed the limit"""
 

=== modified file 'bzrlib/transport/http/__init__.py'
--- bzrlib/transport/http/__init__.py	2011-01-26 19:34:58 +0000
+++ bzrlib/transport/http/__init__.py	2011-07-27 12:03:30 +0000
@@ -271,7 +271,7 @@
                         cur_offset_and_size = iter_offsets.next()
 
             except (errors.ShortReadvError, errors.InvalidRange,
-                    errors.InvalidHttpRange), e:
+                    errors.InvalidHttpRange, errors.HttpBoundaryMissing), e:
                 mutter('Exception %r: %s during http._readv',e, e)
                 if (not isinstance(e, errors.ShortReadvError)
                     or retried_offset == cur_offset_and_size):

=== modified file 'bzrlib/transport/http/response.py'
--- bzrlib/transport/http/response.py	2009-03-23 14:59:43 +0000
+++ bzrlib/transport/http/response.py	2011-07-27 12:03:30 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2007 Canonical Ltd
+# Copyright (C) 2006-2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -109,6 +109,13 @@
             # To be on the safe side we allow it before any boundary line
             boundary_line = self._file.readline()
 
+        if boundary_line == '':
+            # A timeout in the proxy server caused the response to end early.
+            # See launchpad bug 198646.
+            raise errors.HttpBoundaryMissing(
+                self._path,
+                self._boundary)
+
         if boundary_line != '--' + self._boundary + '\r\n':
             # rfc822.unquote() incorrectly unquotes strings enclosed in <>
             # IIS 6 and 7 incorrectly wrap boundary strings in <>