Rev 3131: Make all the test pass. Looks like we are HTTP/1.1 compliant. in file:///v/home/vila/src/bzr/bugs/175524/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Dec 21 21:58:11 GMT 2007


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

------------------------------------------------------------
revno: 3131
revision-id:v.ladeuil+lp at free.fr-20071221215806-c2kdsnqsi3tvqjr4
parent: v.ladeuil+lp at free.fr-20071221122033-42bc21re0zj4kqbg
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 175524
timestamp: Fri 2007-12-21 22:58:06 +0100
message:
  Make all the test pass. Looks like we are HTTP/1.1 compliant.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  Review the debug prints and layered them.
  (AbstractAuthHandler.auth_required): Clean up the http pipe before
  issuing the new request after a 401 or 407 auth required error.
  
  * bzrlib/tests/test_http.py:
  Fix the remaining imports.
  (BadStatusRequestHandler.parse_request): Simplified. Close
  connection.
  (TestInvalidStatusServer.test_http_has,
  TestInvalidStatusServer.test_http_get): Document pycurl
  limitations.
  
  * bzrlib/tests/http_utils.py:
  (RedirectRequestHandler.parse_request, AuthRequestHandler.do_GET):
  Add a Content-Length header.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPRequestHandler.get_multiple_ranges): Close the
  connection since we didnt specify a Content-Length header.
  
  * bzrlib/tests/http_utils.py:
  (RedirectRequestHandler.parse_request): Add a Content-Length
  header.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPRequestHandler.handle_one_request): Any socket error
  close the connection.
modified:
  bzrlib/tests/http_server.py    httpserver.py-20061012142527-m1yxdj1xazsf8d7s-1
  bzrlib/tests/http_utils.py     HTTPTestUtil.py-20050914180604-247d3aafb7a43343
  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-21 12:20:33 +0000
+++ b/bzrlib/tests/http_server.py	2007-12-21 21:58:06 +0000
@@ -77,11 +77,13 @@
         try:
             SimpleHTTPServer.SimpleHTTPRequestHandler.handle_one_request(self)
         except socket.error, e:
-            if (len(e.args) > 0
-                and e.args[0] in (errno.EPIPE, errno.ECONNRESET,
-                                  errno.ECONNABORTED,)):
-                self.close_connection = 1
-            else:
+            # Any socket error should close the connection, but some are due to
+            # the client closing early and we don't want to pollute test
+            # results, so we raise only the others.
+            self.close_connection = 1
+            if (len(e.args) == 0
+                or e.args[0] not in (errno.EPIPE, errno.ECONNRESET,
+                                     errno.ECONNABORTED, errno.EBADF)):
                 raise
 
     _range_regexp = re.compile(r'^(?P<start>\d+)-(?P<end>\d+)$')
@@ -154,6 +156,9 @@
             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
 
     def do_GET(self):
         """Serve a GET request.

=== modified file 'bzrlib/tests/http_utils.py'
--- a/bzrlib/tests/http_utils.py	2007-12-21 11:41:15 +0000
+++ b/bzrlib/tests/http_utils.py	2007-12-21 21:58:06 +0000
@@ -134,6 +134,8 @@
                 # Redirect as instructed
                 self.send_response(code)
                 self.send_header('Location', target)
+                # We do not send a body
+                self.send_header('Content-Length', '0')
                 self.end_headers()
                 return False # The job is done
             else:
@@ -230,6 +232,8 @@
             tcs.auth_required_errors += 1
             self.send_response(tcs.auth_error_code)
             self.send_header_auth_reqed()
+            # We do not send a body
+            self.send_header('Content-Length', '0')
             self.end_headers()
             return
 

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-12-21 12:20:33 +0000
+++ b/bzrlib/tests/test_http.py	2007-12-21 21:58:06 +0000
@@ -27,6 +27,7 @@
 import httplib
 import os
 import select
+import SimpleHTTPServer
 import socket
 import sys
 import threading
@@ -45,9 +46,8 @@
     http_server,
     http_utils,
     )
-
+from bzrlib.transport import http
 from bzrlib.transport.http import (
-    extract_auth,
     _urllib,
     _urllib2_wrappers,
     )
@@ -259,10 +259,10 @@
 
     def test_url_parsing(self):
         f = FakeManager()
-        url = extract_auth('http://example.com', f)
+        url = http.extract_auth('http://example.com', f)
         self.assertEquals('http://example.com', url)
         self.assertEquals(0, len(f.credentials))
-        url = extract_auth('http://user:pass@www.bazaar-vcs.org/bzr/bzr.dev', f)
+        url = http.extract_auth('http://user:pass@www.bazaar-vcs.org/bzr/bzr.dev', f)
         self.assertEquals('http://www.bazaar-vcs.org/bzr/bzr.dev', url)
         self.assertEquals(1, len(f.credentials))
         self.assertEquals([None, 'www.bazaar-vcs.org', 'user', 'pass'],
@@ -442,7 +442,7 @@
         offsets = [ (start, end - start + 1) for start, end in ranges]
         coalesce = transport.Transport._coalesce_offsets
         coalesced = list(coalesce(offsets, limit=0, fudge_factor=0))
-        range_header = HttpTransportBase._range_header
+        range_header = http.HttpTransportBase._range_header
         self.assertEqual(value, range_header(coalesced, tail))
 
     def test_range_header_single(self):
@@ -477,6 +477,9 @@
         return http_server.HttpServer(self._req_handler_class,
                                       protocol_version=self._protocol_version)
 
+    def _testing_pycurl(self):
+        return pycurl_present and self._transport == PyCurlTransport
+
 
 class WallRequestHandler(http_server.TestingHTTPRequestHandler):
     """Whatever request comes in, close the connection"""
@@ -515,27 +518,8 @@
     def parse_request(self):
         """Fakes handling a single HTTP request, returns a bad status"""
         ignored = http_server.TestingHTTPRequestHandler.parse_request(self)
-        try:
-            self.send_response(0, "Bad status")
-            self.end_headers()
-        except socket.error, e:
-            # We don't want to pollute the test results with
-            # spurious server errors while test succeed. In our
-            # case, it may occur that the test has already read
-            # the 'Bad Status' and closed the socket while we are
-            # still trying to send some headers... So the test is
-            # ok, but if we raise the exception, the output is
-            # dirty. So we don't raise, but we close the
-            # connection, just to be safe :)
-            spurious = [errno.EPIPE,
-                        errno.ECONNRESET,
-                        errno.ECONNABORTED,
-                        ]
-            if (len(e.args) > 0) and (e.args[0] in spurious):
-                self.close_connection = 1
-                pass
-            else:
-                raise
+        self.send_response(0, "Bad status")
+        self.close_connection = 1
         return False
 
 
@@ -556,7 +540,7 @@
 
 
 class InvalidStatusRequestHandler(http_server.TestingHTTPRequestHandler):
-    """Whatever request comes in, returns am invalid status"""
+    """Whatever request comes in, returns an invalid status"""
 
     def parse_request(self):
         """Fakes handling a single HTTP request, returns a bad status"""
@@ -573,6 +557,16 @@
 
     _req_handler_class = InvalidStatusRequestHandler
 
+    def test_http_has(self):
+        if self._testing_pycurl() and self._protocol_version == 'HTTP/1.1':
+            raise tests.KnownFailure('pycurl hangs if the server send back garbage')
+        super(TestInvalidStatusServer, self).test_http_has()
+
+    def test_http_get(self):
+        if self._testing_pycurl() and self._protocol_version == 'HTTP/1.1':
+            raise tests.KnownFailure('pycurl hangs if the server send back garbage')
+        super(TestInvalidStatusServer, self).test_http_get()
+
 
 class BadProtocolRequestHandler(http_server.TestingHTTPRequestHandler):
     """Whatever request comes in, returns a bad protocol version"""
@@ -880,8 +874,8 @@
             osutils.set_or_unset_env(name, value)
 
     def _proxied_request(self):
-        handler = ProxyHandler()
-        request = Request('GET','http://baz/buzzle')
+        handler = _urllib2_wrappers.ProxyHandler()
+        request = _urllib2_wrappers.Request('GET','http://baz/buzzle')
         handler.set_proxy(request, 'http')
         return request
 
@@ -1170,13 +1164,13 @@
                                        self.old_server.port)
         new_prefix = 'http://%s:%s' % (self.new_server.host,
                                        self.new_server.port)
-        self.old_server.redirections = \
-            [('/1(.*)', r'%s/2\1' % (old_prefix), 302),
-             ('/2(.*)', r'%s/3\1' % (old_prefix), 303),
-             ('/3(.*)', r'%s/4\1' % (old_prefix), 307),
-             ('/4(.*)', r'%s/5\1' % (new_prefix), 301),
-             ('(/[^/]+)', r'%s/1\1' % (old_prefix), 301),
-             ]
+        self.old_server.redirections = [
+            ('/1(.*)', r'%s/2\1' % (old_prefix), 302),
+            ('/2(.*)', r'%s/3\1' % (old_prefix), 303),
+            ('/3(.*)', r'%s/4\1' % (old_prefix), 307),
+            ('/4(.*)', r'%s/5\1' % (new_prefix), 301),
+            ('(/[^/]+)', r'%s/1\1' % (old_prefix), 301),
+            ]
         self.assertEquals('redirected 5 times',t._perform(req).read())
 
 

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-20 16:36:44 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-21 21:58:06 +0000
@@ -88,13 +88,9 @@
         """
         httplib.HTTPResponse.begin(self)
         if self.status in self._body_ignored_responses:
-            if self.debuglevel > 0:
+            if self.debuglevel >= 2:
                 print "For status: [%s]," % self.status,
-                print "will ready body, length: ",
-                if  self.length is not None:
-                    print "[%d]" % self.length
-                else:
-                    print "None"
+                print "will ready body, length: %s" % self.length
             if not (self.length is None or self.will_close):
                 # In some cases, we just can't read the body not
                 # even try or we may encounter a 104, 'Connection
@@ -102,8 +98,8 @@
                 # and the server closed the connection just after
                 # having issued the response headers (even if the
                 # headers indicate a Content-Type...)
-                body = self.fp.read(self.length)
-                if self.debuglevel > 3:
+                body = self.read(self.length)
+                if self.debuglevel >= 9:
                     # This one can be huge and is generally not interesting
                     print "Consumed body: [%s]" % body
             self.close()
@@ -149,6 +145,9 @@
                 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",
                     pending)
@@ -442,7 +441,7 @@
             raise exc_type, exc_val, exc_tb
         else:
             if first_try:
-                if self._debuglevel > 0:
+                if self._debuglevel >= 2:
                     print 'Received exception: [%r]' % exc_val
                     print '  On connection: [%r]' % request.connection
                     method = request.get_method()
@@ -451,7 +450,7 @@
                 request.connection.close()
                 response = self.do_open(http_class, request, False)
             else:
-                if self._debuglevel > 0:
+                if self._debuglevel >= 2:
                     print 'Received second exception: [%r]' % exc_val
                     print '  On connection: [%r]' % request.connection
                 if exc_type in (httplib.BadStatusLine, httplib.UnknownProtocol):
@@ -478,7 +477,7 @@
                                                        request.get_selector()),
                         orig_error=exc_val)
 
-                if self._debuglevel > 0:
+                if self._debuglevel >= 2:
                     print 'On connection: [%r]' % request.connection
                     method = request.get_method()
                     url = request.get_full_url()
@@ -513,8 +512,9 @@
                 trace.mutter('> %s %s' % (method, url))
                 hdrs = ['%s: %s' % (k, v) for k,v in headers.items()]
                 trace.mutter('> ' + '\n> '.join(hdrs) + '\n')
-            if self._debuglevel > 0:
-                print 'Request sent: [%r]' % request
+            if self._debuglevel >= 1:
+                print 'Request sent: [%r] from (%s)' \
+                    % (request, request.connection.sock.getsockname())
             response = connection.getresponse()
             convert_to_addinfourl = True
         except (socket.gaierror, httplib.BadStatusLine, httplib.UnknownProtocol,
@@ -534,7 +534,7 @@
 #            connection.send(body)
 #            response = connection.getresponse()
 
-        if self._debuglevel > 0:
+        if self._debuglevel >= 2:
             print 'Receives response: %r' % response
             print '  For: %r(%r)' % (request.get_method(),
                                      request.get_full_url())
@@ -549,7 +549,7 @@
             resp.code = r.status
             resp.msg = r.reason
             resp.version = r.version
-            if self._debuglevel > 0:
+            if self._debuglevel >= 2:
                 print 'Create addinfourl: %r' % resp
                 print '  For: %r(%r)' % (request.get_method(),
                                          request.get_full_url())
@@ -696,7 +696,7 @@
             newurl = headers.getheaders('uri')[0]
         else:
             return
-        if self._debuglevel > 0:
+        if self._debuglevel >= 1:
             print 'Redirected to: %s (followed: %r)' % (newurl,
                                                         req.follow_redirections)
         if req.follow_redirections is False:
@@ -759,7 +759,7 @@
         urllib2.ProxyHandler.__init__(self, proxies)
         # First, let's get rid of urllib2 implementation
         for type, proxy in self.proxies.items():
-            if self._debuglevel > 0:
+            if self._debuglevel >= 3:
                 print 'Will unbind %s_open for %r' % (type, proxy)
             delattr(self, '%s_open' % type)
 
@@ -768,13 +768,13 @@
         https_proxy = self.get_proxy_env_var('https')
 
         if http_proxy is not None:
-            if self._debuglevel > 0:
+            if self._debuglevel >= 3:
                 print 'Will bind http_request for %r' % http_proxy
             setattr(self, 'http_request',
                     lambda request: self.set_proxy(request, 'http'))
 
         if https_proxy is not None:
-            if self._debuglevel > 0:
+            if self._debuglevel >= 3:
                 print 'Will bind http_request for %r' % https_proxy
             setattr(self, 'https_request',
                     lambda request: self.set_proxy(request, 'https'))
@@ -825,7 +825,7 @@
             return request
 
         proxy = self.get_proxy_env_var(type)
-        if self._debuglevel > 0:
+        if self._debuglevel >= 3:
             print 'set_proxy %s_request for %r' % (type, proxy)
         # FIXME: python 2.5 urlparse provides a better _parse_proxy which can
         # grok user:password at host:port as well as
@@ -849,7 +849,7 @@
         else:
             phost = host + ':%d' % port
         request.set_proxy(phost, type)
-        if self._debuglevel > 0:
+        if self._debuglevel >= 3:
             print 'set_proxy: proxy set to %s://%s' % (type, phost)
         return request
 
@@ -950,6 +950,8 @@
                 # We already tried that, give up
                 return None
 
+            # Housekeeping
+            request.connection.cleanup_pipe()
             response = self.parent.open(request)
             if response:
                 self.auth_successful(request, response)
@@ -1352,7 +1354,7 @@
             )
 
         self.open = self._opener.open
-        if DEBUG >= 3:
+        if DEBUG >= 9:
             # When dealing with handler order, it's easy to mess
             # things up, the following will help understand which
             # handler is used, when and for what.



More information about the bazaar-commits mailing list