Rev 3139: Fix the multi-ranges http server and add tests. in file:///v/home/vila/src/bzr/bugs/175524/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Dec 28 15:28:50 GMT 2007


At file:///v/home/vila/src/bzr/bugs/175524/

------------------------------------------------------------
revno: 3139
revision-id:v.ladeuil+lp at free.fr-20071228152844-r64au9m40wem2tuf
parent: v.ladeuil+lp at free.fr-20071224154228-gmtwk6qixarl9hyb
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 175524
timestamp: Fri 2007-12-28 16:28:44 +0100
message:
  Fix the multi-ranges http server and add tests.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (Response.finish): Fix the mutter message.
  
  * bzrlib/tests/test_http.py:
  (TestRangeRequestServer.test_complete_readv_leave_pipe_clean,
  TestRangeRequestServer.test_incomplete_readv_leave_pipe_clean):
  Add tests checking http pipe after readv.
  (TestMultipleRangeWithoutContentLengthServer): Keep the buggy
  get_multiple_ranges version as a test.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPRequestHandler.get_multiple_ranges): Calculate the
  body length and add a Content-Length header.
  (HttpServer.tearDown): Stop joining the server thread since it is
  garbage collected.
modified:
  bzrlib/tests/http_server.py    httpserver.py-20061012142527-m1yxdj1xazsf8d7s-1
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
-------------- next part --------------
=== modified file 'bzrlib/tests/http_server.py'
--- a/bzrlib/tests/http_server.py	2007-12-24 13:29:30 +0000
+++ b/bzrlib/tests/http_server.py	2007-12-28 15:28:44 +0000
@@ -126,6 +126,10 @@
                     return 0, []
         return tail, ranges
 
+    def _header_line_length(self, keyword, value):
+        header_line = '%s: %s\r\n' % (keyword, value)
+        return len(header_line)
+
     def send_head(self):
         """Overrides base implementation to work around a bug in python2.5."""
         path = self.translate_path(self.path)
@@ -162,23 +166,34 @@
     def get_multiple_ranges(self, file, file_size, ranges):
         self.send_response(206)
         self.send_header('Accept-Ranges', 'bytes')
-        boundary = "%d" % random.randint(0,0x7FFFFFFF)
-        self.send_header("Content-Type",
-                         "multipart/byteranges; boundary=%s" % boundary)
+        boundary = '%d' % random.randint(0,0x7FFFFFFF)
+        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
         for (start, end) in ranges:
-            self.wfile.write("--%s\r\n" % boundary)
-            self.send_header("Content-type", 'application/octet-stream')
-            self.send_header("Content-Range", "bytes %d-%d/%d" % (start,
-                                                                  end,
-                                                                  file_size))
+            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)
         # Final boundary
-        self.wfile.write("--%s\r\n" % boundary)
-        # Close the connection since we didn't specify the Content-Length
-        # FIXME: This is not 1.1 friendly
-        self.close_connection = 1
+        self.wfile.write(boundary_line)
 
     def do_GET(self):
         """Serve a GET request.
@@ -502,7 +517,9 @@
         """See bzrlib.transport.Server.tearDown."""
         self._httpd.tearDown()
         self._http_running = False
-        self._http_thread.join()
+        # We don't need to 'self._http_thread.join()' here since the thread is
+        # a daemonic one and will be garbage collected anyway. Joining just
+        # slows us down for no added benefit.
 
     def get_url(self):
         """See bzrlib.transport.Server.get_url."""

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-12-24 15:42:28 +0000
+++ b/bzrlib/tests/test_http.py	2007-12-28 15:28:44 +0000
@@ -728,7 +728,7 @@
         t = self._transport(server.get_url())
         # force transport to issue multiple requests by limiting the number of
         # bytes by request. Note that this apply to coalesced offsets only, a
-        # single range ill keep its size even if bigger than the limit.
+        # single range will keep its size even if bigger than the limit.
         t._get_max_size = 2
         l = list(t.readv('a', ((0, 1), (1, 1), (2, 4), (6, 4))))
         self.assertEqual(l[0], (0, '0'))
@@ -738,6 +738,33 @@
         # The server should have issued 3 requests
         self.assertEqual(3, server.GET_request_nb)
 
+    def test_complete_readv_leave_pipe_clean(self):
+        server = self.get_readonly_server()
+        t = self._transport(server.get_url())
+        # force transport to issue multiple requests
+        t._get_max_size = 2
+        l = list(t.readv('a', ((0, 1), (1, 1), (2, 4), (6, 4))))
+        # The server should have issued 3 requests
+        self.assertEqual(3, server.GET_request_nb)
+        self.assertEqual('0123456789', t.get_bytes('a'))
+        self.assertEqual(4, server.GET_request_nb)
+
+    def test_incomplete_readv_leave_pipe_clean(self):
+        server = self.get_readonly_server()
+        t = self._transport(server.get_url())
+        # force transport to issue multiple requests
+        t._get_max_size = 2
+        # Don't collapse readv results into a list so that we leave unread
+        # bytes on the socket
+        ireadv = iter(t.readv('a', ((0, 1), (1, 1), (2, 4), (6, 4))))
+        self.assertEqual((0, '0'), ireadv.next())
+        # The server should have issued one request so far 
+        self.assertEqual(1, server.GET_request_nb)
+        self.assertEqual('0123456789', t.get_bytes('a'))
+        # get_bytes issued an additional request, the readv pending ones are
+        # lost
+        self.assertEqual(2, server.GET_request_nb)
+
 
 class SingleRangeRequestHandler(http_server.TestingHTTPRequestHandler):
     """Always reply to range request as if they were single.
@@ -792,6 +819,33 @@
     _req_handler_class = NoRangeRequestHandler
 
 
+class MultipleRangeWithoutContentLengthRequestHandler(
+    http_server.TestingHTTPRequestHandler):
+    """Reply to multiple range requests without content length header."""
+
+    def get_multiple_ranges(self, file, file_size, ranges):
+        self.send_response(206)
+        self.send_header('Accept-Ranges', 'bytes')
+        boundary = "%d" % random.randint(0,0x7FFFFFFF)
+        self.send_header("Content-Type",
+                         "multipart/byteranges; boundary=%s" % boundary)
+        self.end_headers()
+        for (start, end) in ranges:
+            self.wfile.write("--%s\r\n" % boundary)
+            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)
+        # Final boundary
+        self.wfile.write("--%s\r\n" % boundary)
+
+
+class TestMultipleRangeWithoutContentLengthServer(TestRangeRequestServer):
+
+    _req_handler_class = MultipleRangeWithoutContentLengthRequestHandler
+
 class LimitedRangeRequestHandler(http_server.TestingHTTPRequestHandler):
     """Errors out when range specifiers exceed the limit"""
 

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-24 13:29:30 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-28 15:28:44 +0000
@@ -145,11 +145,7 @@
                 pending += len(data)
             if pending:
                 trace.mutter(
-                    # FIXME: this message is bogus if the server didn't give
-                    # the length, there is no way we can know how many bytes
-                    # are left !
-                    "bogus http server didn't give body length,"
-                    "%s bytes left on the socket",
+                    "%s bytes left on the HTTP socket",
                     pending)
             self.close()
         return pending



More information about the bazaar-commits mailing list