Merge "bufferedhttp: Tolerate socket being None"

swift/common/bufferedhttp.py

@@ -33,7 +33,7 @@ import socket
 
 import eventlet
 from eventlet.green.httplib import CONTINUE, HTTPConnection, HTTPMessage, \
-    HTTPResponse, HTTPSConnection, _UNKNOWN
+    HTTPResponse, HTTPSConnection, _UNKNOWN, ImproperConnectionState
 from six.moves.urllib.parse import quote, parse_qsl, urlencode
 import six
 
@@ -56,16 +56,24 @@ class BufferedHTTPResponse(HTTPResponse):
 
     def __init__(self, sock, debuglevel=0, strict=0,
                  method=None):          # pragma: no cover
+        # sock should be an eventlet.greenio.GreenSocket
         self.sock = sock
-        # sock is an eventlet.greenio.GreenSocket
-        if six.PY2:
+        if sock is None:
+            # ...but it could be None if we close the connection as we're
+            # getting it wrapped up in a Response
+            self._real_socket = None
+            # No socket means no file-like -- set it to None like in
+            # HTTPResponse.close()
+            self.fp = None
+        elif six.PY2:
             # sock.fd is a socket._socketobject
             # sock.fd._sock is a _socket.socket object, which is what we want.
             self._real_socket = sock.fd._sock
+            self.fp = sock.makefile('rb')
         else:
             # sock.fd is a socket.socket, which should have a _real_close
             self._real_socket = sock.fd
-        self.fp = sock.makefile('rb')
+            self.fp = sock.makefile('rb')
         self.debuglevel = debuglevel
         self.strict = strict
         self._method = method
@@ -106,6 +114,8 @@ class BufferedHTTPResponse(HTTPResponse):
         if self.fp:
             self.fp.close()
             self.fp = None
+        if not self.sock:
+            raise ImproperConnectionState('Socket already closed')
         self.fp = self.sock.makefile('rb', 0)
         version, status, reason = self._read_status()
         if status != CONTINUE:

test/unit/common/test_bufferedhttp.py

@@ -99,6 +99,50 @@ class TestBufferedHTTP(unittest.TestCase):
                 if err:
                     raise Exception(err)
 
+    def test_get_expect(self):
+        bindsock = listen_zero()
+        request = []
+
+        def accept():
+            with Timeout(3):
+                sock, addr = bindsock.accept()
+                fp = sock.makefile('rwb')
+                request.append(fp.readline())
+                fp.write(b'HTTP/1.1 100 Continue\r\n\r\n')
+                fp.flush()
+                fp.write(b'HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\n'
+                         b'RESPONSE')
+                fp.flush()
+
+        server = spawn(accept)
+        try:
+            address = '%s:%s' % ('127.0.0.1', bindsock.getsockname()[1])
+            conn = bufferedhttp.BufferedHTTPConnection(address)
+            conn.putrequest('GET', '/path')
+            conn.endheaders()
+            resp = conn.getexpect()
+            self.assertIsInstance(resp, bufferedhttp.BufferedHTTPResponse)
+            self.assertEqual(resp.status, 100)
+            self.assertEqual(resp.version, 11)
+            self.assertEqual(resp.reason, 'Continue')
+            # I don't think you're supposed to "read" a continue response
+            self.assertRaises(AssertionError, resp.read)
+
+            resp = conn.getresponse()
+            self.assertIsInstance(resp, bufferedhttp.BufferedHTTPResponse)
+            self.assertEqual(resp.read(), b'RESPONSE')
+
+        finally:
+            server.wait()
+        self.assertEqual(request[0], b'GET /path HTTP/1.1\r\n')
+
+    def test_closed_response(self):
+        resp = bufferedhttp.BufferedHTTPResponse(None)
+        self.assertEqual(resp.status, 'UNKNOWN')
+        self.assertEqual(resp.version, 'UNKNOWN')
+        self.assertEqual(resp.reason, 'UNKNOWN')
+        self.assertEqual(resp.read(), b'')
+
     def test_nonstr_header_values(self):
         with mock.patch('swift.common.bufferedhttp.HTTPSConnection',
                         MockHTTPSConnection):