Rev 4513: Fix bug #383920 by inserting the missing Content-Length header. in file:///home/vila/src/bzr/bugs/383920-pycurl-hangs/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jul 7 09:24:58 BST 2009


At file:///home/vila/src/bzr/bugs/383920-pycurl-hangs/

------------------------------------------------------------
revno: 4513
revision-id: v.ladeuil+lp at free.fr-20090707082457-otq7zht7lgkpnpbr
parent: pqm at pqm.ubuntu.com-20090706163157-6hqiw8ehs0u4uojv
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 383920-pycurl-hangs
timestamp: Tue 2009-07-07 10:24:57 +0200
message:
  Fix bug #383920 by inserting the missing Content-Length header.
  
  * bzrlib/tests/test_transport_implementations.py:
  (TransportTests.test_get_unknown_file,
  TransportTests.test_get_bytes_unknown_file): Localize bug #383920.
  
  * bzrlib/tests/test_read_bundle.py:
  (TestReadBundleFromURL.setUp,
  TestReadBundleFromURL.read_mergeable_from_url): Fix regression
  caused by pqm not running pycurl + python2.6, which means that the
  https test server is not tested there and couldn't catch it. Clean
  up the tests too.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPRequestHandler.send_error): Send the Content-Length
  header or pycurl can't handle the 404...
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-07-06 06:46:30 +0000
+++ b/NEWS	2009-07-07 08:24:57 +0000
@@ -75,6 +75,10 @@
   of a remote repository.
   (Jelmer Vernooij, #332194)
 
+* Fix bogus ``send_error`` in python's BaseHTTPServer by inserting the missing
+  'Content-Length header. https pycurl clients can hang otherwise.
+  (Vincent Ladeuil, #383920).
+
 * Force deletion of readonly files during merge, update and other tree
   transforms.
   (Craig Hewetson, Martin Pool, #218206)

=== modified file 'bzrlib/tests/http_server.py'
--- a/bzrlib/tests/http_server.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/http_server.py	2009-07-07 08:24:57 +0000
@@ -14,6 +14,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+import BaseHTTPServer
 import errno
 import httplib
 import os
@@ -144,6 +145,35 @@
 
         return SimpleHTTPServer.SimpleHTTPRequestHandler.send_head(self)
 
+    def send_error(self, code, message=None):
+        """Send and log an error reply.
+
+        Replace the bogus BaseHTTPServer.py with one that add the
+        Content-Length header to address strict http clients.
+        """
+
+        try:
+            short, long = self.responses[code]
+        except KeyError:
+            short, long = '???', '???'
+        if message is None:
+            message = short
+        explain = long
+        self.log_error("code %d, message %s", code, message)
+        # using _quote_html to prevent Cross Site Scripting attacks (see bug
+        # #1100201)
+        content = (self.error_message_format %
+                   {'code': code,
+                    'message': BaseHTTPServer._quote_html(message),
+                    'explain': explain})
+        self.send_response(code, message)
+        self.send_header("Content-Length", "%d" % len(content))
+        self.send_header("Content-Type", self.error_content_type)
+        self.send_header('Connection', 'close')
+        self.end_headers()
+        if self.command != 'HEAD' and code >= 200 and code not in (204, 304):
+            self.wfile.write(content)
+
     def send_range_content(self, file, start, length):
         file.seek(start)
         self.wfile.write(file.read(length))

=== modified file 'bzrlib/tests/test_read_bundle.py'
--- a/bzrlib/tests/test_read_bundle.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_read_bundle.py	2009-07-07 08:24:57 +0000
@@ -84,70 +84,54 @@
 class TestReadBundleFromURL(TestTransportImplementation):
     """Test that read_bundle works properly across multiple transports"""
 
+    def setUp(self):
+        super(TestReadBundleFromURL, self).setUp()
+        self.bundle_name = 'test_bundle'
+        # read_mergeable_from_url will invoke get_transport which may *not*
+        # respect self._transport (i.e. returns a transport that is different
+        # from the one we want to test, so we must inject a correct transport
+        # into possible_transports first.
+        self.possible_transports = [self.get_transport(self.bundle_name)]
+        self._captureVar('BZR_NO_SMART_VFS', None)
+        wt = self.create_test_bundle()
+
+    def read_mergeable_from_url(self, url):
+        return bzrlib.bundle.read_mergeable_from_url(
+            url, possible_transports=self.possible_transports)
+
     def get_url(self, relpath=''):
         return bzrlib.urlutils.join(self._server.get_url(), relpath)
 
     def create_test_bundle(self):
         out, wt = create_bundle_file(self)
         if self.get_transport().is_readonly():
-            f = open('test_bundle', 'wb')
-            try:
-                f.write(out.getvalue())
-            finally:
-                f.close()
+            self.build_tree_contents([(self.bundle_name, out.getvalue())])
         else:
-            self.get_transport().put_file('test_bundle', out)
-            self.log('Put to: %s', self.get_url('test_bundle'))
+            self.get_transport().put_file(self.bundle_name, out)
+            self.log('Put to: %s', self.get_url(self.bundle_name))
         return wt
 
     def test_read_mergeable_from_url(self):
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-        # read_mergeable_from_url will invoke get_transport which may *not*
-        # respect self._transport (i.e. returns a transport that is different
-        # from the one we want to test, so we must inject a correct transport
-        # into possible_transports first.
-        t = self.get_transport('test_bundle')
-        possible_transports = [t]
-        info = bzrlib.bundle.read_mergeable_from_url(
-                    unicode(self.get_url('test_bundle')),
-                    possible_transports=possible_transports)
+        info = self.read_mergeable_from_url(
+            unicode(self.get_url(self.bundle_name)))
         revision = info.real_revisions[-1]
         self.assertEqual('commit-1', revision.revision_id)
 
     def test_read_fail(self):
         # Trying to read from a directory, or non-bundle file
         # should fail with NotABundle
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-
-        self.assertRaises(errors.NotABundle,
-            bzrlib.bundle.read_mergeable_from_url,
-            self.get_url('tree'))
-        self.assertRaises(errors.NotABundle,
-            bzrlib.bundle.read_mergeable_from_url,
-            self.get_url('tree/a'))
+        self.assertRaises(errors.NotABundle,
+                          self.read_mergeable_from_url, self.get_url('tree'))
+        self.assertRaises(errors.NotABundle,
+                          self.read_mergeable_from_url, self.get_url('tree/a'))
 
     def test_read_mergeable_respects_possible_transports(self):
-        t = self.get_transport('test_bundle')
-        if not isinstance(t, bzrlib.transport.ConnectedTransport):
+        if not isinstance(self.get_transport(self.bundle_name),
+                          bzrlib.transport.ConnectedTransport):
             # There is no point testing transport reuse for not connected
             # transports (the test will fail even).
-            return
-        self._captureVar('BZR_NO_SMART_VFS', None)
-        wt = self.create_test_bundle()
-        if wt is None:
-            return
-        # read_mergeable_from_url will invoke get_transport which may *not*
-        # respect self._transport (i.e. returns a transport that is different
-        # from the one we want to test, so we must inject a correct transport
-        # into possible_transports first.
-        possible_transports = [t]
-        url = unicode(self.get_url('test_bundle'))
-        info = bzrlib.bundle.read_mergeable_from_url(url,
-            possible_transports=possible_transports)
-        self.assertEqual(1, len(possible_transports))
+            raise tests.TestSkipped(
+                'Need a ConnectedTransport to test transport reuse')
+        url = unicode(self.get_url(self.bundle_name))
+        info = self.read_mergeable_from_url(url)
+        self.assertEqual(1, len(self.possible_transports))

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2009-04-20 04:19:45 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2009-07-07 08:24:57 +0000
@@ -203,6 +203,13 @@
         for content, f in itertools.izip(contents, content_f):
             self.assertEqual(content, f.read())
 
+    def test_get_unknown_file(self):
+        t = self.get_transport()
+        files = ['a', 'b']
+        contents = ['contents of a\n',
+                    'contents of b\n',
+                    ]
+        self.build_tree(files, transport=t, line_endings='binary')
         self.assertRaises(NoSuchFile, t.get, 'c')
         self.assertListRaises(NoSuchFile, t.get_multi, ['a', 'b', 'c'])
         self.assertListRaises(NoSuchFile, t.get_multi, iter(['a', 'b', 'c']))
@@ -242,6 +249,9 @@
         for content, fname in zip(contents, files):
             self.assertEqual(content, t.get_bytes(fname))
 
+    def test_get_bytes_unknown_file(self):
+        t = self.get_transport()
+
         self.assertRaises(NoSuchFile, t.get_bytes, 'c')
 
     def test_get_with_open_write_stream_sees_all_content(self):



More information about the bazaar-commits mailing list