Fix handling of BadStatusLine.

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

4 files changed, 120 insertions, 3 deletions

diff --git a/python2/httplib2/__init__.py b/python2/httplib2/__init__.py
index 9780d4e..191ef1e 100644
--- a/python2/httplib2/__init__.py
+++ b/python2/httplib2/__init__.py
@@ -1246,7 +1246,10 @@ class Http(object):
         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 @@ class Http(object):
                     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()
diff --git a/python2/httplib2test.py b/python2/httplib2test.py
index 3802879..104eaf7 100755
--- a/python2/httplib2test.py
+++ b/python2/httplib2test.py
@@ -144,6 +144,36 @@ class _MyHTTPConnection(object):
     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 @@ class HttpTest(unittest.TestCase):
         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:
diff --git a/python3/httplib2/__init__.py b/python3/httplib2/__init__.py
index 65f90ac..67f56b3 100644
--- a/python3/httplib2/__init__.py
+++ b/python3/httplib2/__init__.py
@@ -957,7 +957,10 @@ class Http(object):
         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 @@ class Http(object):
                 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):
diff --git a/python3/httplib2test.py b/python3/httplib2test.py
index 480d28e..8f7a613 100755
--- a/python3/httplib2test.py
+++ b/python3/httplib2test.py
@@ -138,6 +138,37 @@ class _MyHTTPConnection(object):
         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 @@ class HttpTest(unittest.TestCase):
         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 @@ class HttpTest(unittest.TestCase):
 

           # 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__ )),