Fix handling of BadStatusLine.

Fixes issue #250.
Review in https://codereview.appspot.com/7529045/.

Author: jcgregorio

python2/httplib2/__init__.py

@@ -1246,7 +1246,10 @@ def clear_credentials(self):
         self.authorizations = []
 
     def _conn_request(self, conn, request_uri, method, body, headers):
-        for i in range(RETRIES):
+        i = 0
+        seen_bad_status_line = False
+        while i < RETRIES:
+            i += 1
             try:
                 if hasattr(conn, 'sock') and conn.sock is None:
                     conn.connect()
@@ -1284,6 +1287,19 @@ def _conn_request(self, conn, request_uri, method, body, headers):
                     continue
             try:
                 response = conn.getresponse()
+            except httplib.BadStatusLine:
+                # If we get a BadStatusLine on the first try then that means
+                # the connection just went stale, so retry regardless of the
+                # number of RETRIES set.
+                if not seen_bad_status_line and i == 1:
+                    i = 0
+                    seen_bad_status_line = True
+                    conn.close()
+                    conn.connect()
+                    continue
+                else:
+                    conn.close()
+                    raise
             except (socket.error, httplib.HTTPException):
                 if i < RETRIES-1:
                     conn.close()

python2/httplib2test.py

@@ -144,6 +144,36 @@ def request(self, method, request_uri, body, headers):
     def getresponse(self):
         return _MyResponse("the body", status="200")
 
+class _MyHTTPBadStatusConnection(object):
+    "Mock of httplib.HTTPConnection that raises BadStatusLine."
+
+    num_calls = 0
+
+    def __init__(self, host, port=None, key_file=None, cert_file=None,
+                 strict=None, timeout=None, proxy_info=None):
+        self.host = host
+        self.port = port
+        self.timeout = timeout
+        self.log = ""
+        self.sock = None
+        _MyHTTPBadStatusConnection.num_calls = 0
+
+    def set_debuglevel(self, level):
+        pass
+
+    def connect(self):
+        pass
+
+    def close(self):
+        pass
+
+    def request(self, method, request_uri, body, headers):
+        pass
+
+    def getresponse(self):
+        _MyHTTPBadStatusConnection.num_calls += 1
+        raise httplib.BadStatusLine("")
+
 
 class HttpTest(unittest.TestCase):
     def setUp(self):
@@ -187,6 +217,17 @@ def testConnectionType(self):
         self.assertEqual(response['content-location'], "http://bitworking.org")
         self.assertEqual(content, "the body")
 
+    def testBadStatusLineRetry(self):
+        old_retries = httplib2.RETRIES
+        httplib2.RETRIES = 1
+        self.http.force_exception_to_status_code = False
+        try:
+            response, content = self.http.request("http://bitworking.org",
+                connection_type=_MyHTTPBadStatusConnection)
+        except httplib.BadStatusLine:
+            self.assertEqual(2, _MyHTTPBadStatusConnection.num_calls)
+        httplib2.RETRIES = old_retries
+
     def testGetUnknownServer(self):
         self.http.force_exception_to_status_code = False
         try:

python3/httplib2/__init__.py

@@ -957,7 +957,10 @@ def clear_credentials(self):
         self.authorizations = []
 
     def _conn_request(self, conn, request_uri, method, body, headers):
-        for i in range(RETRIES):
+        i = 0
+        seen_bad_status_line = False
+        while i < RETRIES:
+            i += 1
             try:
                 if conn.sock is None:
                     conn.connect()
@@ -990,6 +993,19 @@ def _conn_request(self, conn, request_uri, method, body, headers):
                 pass
             try:
                 response = conn.getresponse()
+            except (http.client.BadStatusLine, http.client.ResponseNotReady):
+                # If we get a BadStatusLine on the first try then that means
+                # the connection just went stale, so retry regardless of the
+                # number of RETRIES set.
+                if not seen_bad_status_line and i == 1:
+                    i = 0
+                    seen_bad_status_line = True
+                    conn.close()
+                    conn.connect()
+                    continue
+                else:
+                    conn.close()
+                    raise
             except socket.timeout:
                 raise
             except (socket.error, http.client.HTTPException):

python3/httplib2test.py

@@ -138,6 +138,37 @@ def getresponse(self):
         return _MyResponse(b"the body", status="200")
 
 
+class _MyHTTPBadStatusConnection(object):
+    "Mock of httplib.HTTPConnection that raises BadStatusLine."
+
+    num_calls = 0
+
+    def __init__(self, host, port=None, key_file=None, cert_file=None,
+                 strict=None, timeout=None, proxy_info=None):
+        self.host = host
+        self.port = port
+        self.timeout = timeout
+        self.log = ""
+        self.sock = None
+        _MyHTTPBadStatusConnection.num_calls = 0
+
+    def set_debuglevel(self, level):
+        pass
+
+    def connect(self):
+        pass
+
+    def close(self):
+        pass
+
+    def request(self, method, request_uri, body, headers):
+        pass
+
+    def getresponse(self):
+        _MyHTTPBadStatusConnection.num_calls += 1
+        raise http.client.BadStatusLine("")
+
+
 class HttpTest(unittest.TestCase):
     def setUp(self):
         if os.path.exists(cacheDirName):
@@ -169,6 +200,19 @@ def testConnectionType(self):
         self.assertEqual(response['content-location'], "http://bitworking.org")
         self.assertEqual(content, b"the body")
 
+
+    def testBadStatusLineRetry(self):
+        old_retries = httplib2.RETRIES
+        httplib2.RETRIES = 1
+        self.http.force_exception_to_status_code = False
+        try:
+            response, content = self.http.request("http://bitworking.org",
+                connection_type=_MyHTTPBadStatusConnection)
+        except http.client.BadStatusLine:
+            self.assertEqual(2, _MyHTTPBadStatusConnection.num_calls)
+        httplib2.RETRIES = old_retries
+
+
     def testGetUnknownServer(self):
         self.http.force_exception_to_status_code = False
         try:
@@ -485,7 +529,7 @@ def testSslCertValidation(self):
 
           # Test that we get a SSLHandshakeError if we try to access
           # https://www.google.com, using a CA cert file that doesn't contain
-          # the CA Gogole uses (i.e., simulating a cert that's not signed by a
+          # the CA Google uses (i.e., simulating a cert that's not signed by a
           # trusted CA).
           other_ca_certs = os.path.join(
                   os.path.dirname(os.path.abspath(httplib2.__file__ )),