[PATCH 2 of 2] httpclient: import revision fc731618702a of py-nonblocking-http

Augie Fackler durin42 at gmail.com
Tue May 17 13:36:51 CDT 2011


# HG changeset patch
# User Augie Fackler <durin42 at gmail.com>
# Date 1305646083 18000
# Node ID 886fed778423d4dfc0f95fd7684a0c144cd934a5
# Parent  8689945a9491d807e3a7154c544166a82423efa3
httpclient: import revision fc731618702a of py-nonblocking-http

diff --git a/mercurial/httpclient/__init__.py b/mercurial/httpclient/__init__.py
--- a/mercurial/httpclient/__init__.py
+++ b/mercurial/httpclient/__init__.py
@@ -112,6 +112,10 @@
 
     def complete(self):
         """Returns true if this response is completely loaded.
+
+        Note that if this is a connection where complete means the
+        socket is closed, this will nearly always return False, even
+        in cases where all the data has actually been loaded.
         """
         if self._chunked:
             return self._chunked_done
@@ -174,10 +178,7 @@
             return True
         logger.debug('response read %d data during _select', len(data))
         if not data:
-            if not self.headers:
-                self._load_response(self._end_headers)
-                self._content_len = 0
-            elif self._content_len == _LEN_CLOSE_IS_END:
+            if self.headers and self._content_len == _LEN_CLOSE_IS_END:
                 self._content_len = len(self._body)
             return False
         else:
@@ -561,17 +562,46 @@
                         continue
                     if not data:
                         logger.info('socket appears closed in read')
-                        outgoing_headers = body = None
-                        break
+                        self.sock = None
+                        self._current_response = None
+                        # This if/elif ladder is a bit subtle,
+                        # comments in each branch should help.
+                        if response is not None and (
+                            response.complete() or
+                            response._content_len == _LEN_CLOSE_IS_END):
+                            # Server responded completely and then
+                            # closed the socket. We should just shut
+                            # things down and let the caller get their
+                            # response.
+                            logger.info('Got an early response, '
+                                        'aborting remaining request.')
+                            break
+                        elif was_first and response is None:
+                            # Most likely a keepalive that got killed
+                            # on the server's end. Commonly happens
+                            # after getting a really large response
+                            # from the server.
+                            logger.info(
+                                'Connection appeared closed in read on first'
+                                ' request loop iteration, will retry.')
+                            reconnect('read')
+                            continue
+                        else:
+                            # We didn't just send the first data hunk,
+                            # and either have a partial response or no
+                            # response at all. There's really nothing
+                            # meaningful we can do here.
+                            raise HTTPStateError(
+                                'Connection appears closed after '
+                                'some request data was written, but the '
+                                'response was missing or incomplete!')
+                    logger.debug('read %d bytes in request()', len(data))
                     if response is None:
                         response = self.response_class(r[0], self.timeout)
                     response._load_response(data)
-                    if (response._content_len == _LEN_CLOSE_IS_END
-                        and len(data) == 0):
-                        response._content_len = len(response._body)
-                    if response.complete():
-                        w = []
-                        response.will_close = True
+                    # Jump to the next select() call so we load more
+                    # data if the server is still sending us content.
+                    continue
                 except socket.error, e:
                     if e[0] != errno.EPIPE and not was_first:
                         raise
@@ -662,4 +692,7 @@
 
 class HTTPProxyConnectFailedException(httplib.HTTPException):
     """Connecting to the HTTP proxy failed."""
+
+class HTTPStateError(httplib.HTTPException):
+    """Invalid internal state encountered."""
 # no-check-code
diff --git a/mercurial/httpclient/tests/simple_http_test.py b/mercurial/httpclient/tests/simple_http_test.py
--- a/mercurial/httpclient/tests/simple_http_test.py
+++ b/mercurial/httpclient/tests/simple_http_test.py
@@ -26,6 +26,7 @@
 # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+import socket
 import unittest
 
 import http
@@ -39,7 +40,7 @@
     def _run_simple_test(self, host, server_data, expected_req, expected_data):
         con = http.HTTPConnection(host)
         con._connect()
-        con.sock.data.extend(server_data)
+        con.sock.data = server_data
         con.request('GET', '/')
 
         self.assertStringEqual(expected_req, con.sock.sent)
@@ -353,4 +354,33 @@
 
         self.assertEqual(('1.2.3.4', 80), con.sock.sa)
         self.assertEqual(expected_req, con.sock.sent)
+
+    def test_conn_keep_alive_but_server_close_anyway(self):
+        sockets = []
+        def closingsocket(*args, **kwargs):
+            s = util.MockSocket(*args, **kwargs)
+            sockets.append(s)
+            s.data = ['HTTP/1.1 200 OK\r\n',
+                      'Server: BogusServer 1.0\r\n',
+                      'Connection: Keep-Alive\r\n',
+                      'Content-Length: 16',
+                      '\r\n\r\n',
+                      'You can do that.']
+            s.close_on_empty = True
+            return s
+
+        socket.socket = closingsocket
+        con = http.HTTPConnection('1.2.3.4:80')
+        con._connect()
+        con.request('GET', '/')
+        r1 = con.getresponse()
+        r1.read()
+        self.assertFalse(con.sock.closed)
+        self.assert_(con.sock.remote_closed)
+        con.request('GET', '/')
+        self.assertEqual(2, len(sockets))
+
+    def test_no_response_raises_response_not_ready(self):
+        con = http.HTTPConnection('foo')
+        self.assertRaises(http.httplib.ResponseNotReady, con.getresponse)
 # no-check-code
diff --git a/mercurial/httpclient/tests/test_chunked_transfer.py b/mercurial/httpclient/tests/test_chunked_transfer.py
--- a/mercurial/httpclient/tests/test_chunked_transfer.py
+++ b/mercurial/httpclient/tests/test_chunked_transfer.py
@@ -53,7 +53,7 @@
         con = http.HTTPConnection('1.2.3.4:80')
         con._connect()
         sock = con.sock
-        sock.read_wait_sentinel = 'end-of-body'
+        sock.read_wait_sentinel = '0\r\n\r\n'
         sock.data = ['HTTP/1.1 200 OK\r\n',
                      'Server: BogusServer 1.0\r\n',
                      'Content-Length: 6',
diff --git a/mercurial/httpclient/tests/test_proxy_support.py b/mercurial/httpclient/tests/test_proxy_support.py
--- a/mercurial/httpclient/tests/test_proxy_support.py
+++ b/mercurial/httpclient/tests/test_proxy_support.py
@@ -43,7 +43,7 @@
     """
     def s(*args, **kwargs):
         sock = util.MockSocket(*args, **kwargs)
-        sock.data = data[:]
+        sock.early_data = data[:]
         return sock
     return s
 
@@ -97,24 +97,27 @@
              '\r\n'
              '1234567890'])
         con._connect()
-        con.sock.data.extend(['HTTP/1.1 200 OK\r\n',
-                              'Server: BogusServer 1.0\r\n',
-                              'Content-Length: 10\r\n',
-                              '\r\n'
-                              '1234567890'
-                              ])
+        con.sock.data = ['HTTP/1.1 200 OK\r\n',
+                         'Server: BogusServer 1.0\r\n',
+                         'Content-Length: 10\r\n',
+                         '\r\n'
+                         '1234567890'
+                         ]
+        connect_sent = con.sock.sent
+        con.sock.sent = ''
         con.request('GET', '/')
 
-        expected_req = ('CONNECT 1.2.3.4:443 HTTP/1.0\r\n'
-                        'Host: 1.2.3.4\r\n'
-                        'accept-encoding: identity\r\n'
-                        '\r\n'
-                        'GET / HTTP/1.1\r\n'
-                        'Host: 1.2.3.4\r\n'
-                        'accept-encoding: identity\r\n\r\n')
+        expected_connect = ('CONNECT 1.2.3.4:443 HTTP/1.0\r\n'
+                            'Host: 1.2.3.4\r\n'
+                            'accept-encoding: identity\r\n'
+                            '\r\n')
+        expected_request = ('GET / HTTP/1.1\r\n'
+                            'Host: 1.2.3.4\r\n'
+                            'accept-encoding: identity\r\n\r\n')
 
         self.assertEqual(('127.0.0.42', 4242), con.sock.sa)
-        self.assertStringEqual(expected_req, con.sock.sent)
+        self.assertStringEqual(expected_connect, connect_sent)
+        self.assertStringEqual(expected_request, con.sock.sent)
         resp = con.getresponse()
         self.assertEqual(resp.status, 200)
         self.assertEqual('1234567890', resp.read())
diff --git a/mercurial/httpclient/tests/test_ssl.py b/mercurial/httpclient/tests/test_ssl.py
--- a/mercurial/httpclient/tests/test_ssl.py
+++ b/mercurial/httpclient/tests/test_ssl.py
@@ -41,15 +41,15 @@
         con._connect()
         # extend the list instead of assign because of how
         # MockSSLSocket works.
-        con.sock.data.extend(['HTTP/1.1 200 OK\r\n',
-                              'Server: BogusServer 1.0\r\n',
-                              'MultiHeader: Value\r\n'
-                              'MultiHeader: Other Value\r\n'
-                              'MultiHeader: One More!\r\n'
-                              'Content-Length: 10\r\n',
-                              '\r\n'
-                              '1234567890'
-                              ])
+        con.sock.data = ['HTTP/1.1 200 OK\r\n',
+                         'Server: BogusServer 1.0\r\n',
+                         'MultiHeader: Value\r\n'
+                         'MultiHeader: Other Value\r\n'
+                         'MultiHeader: One More!\r\n'
+                         'Content-Length: 10\r\n',
+                         '\r\n'
+                         '1234567890'
+                         ]
         con.request('GET', '/')
 
         expected_req = ('GET / HTTP/1.1\r\n'
@@ -68,17 +68,15 @@
     def testSslRereadInEarlyResponse(self):
         con = http.HTTPConnection('1.2.3.4:443')
         con._connect()
-        # extend the list instead of assign because of how
-        # MockSSLSocket works.
-        con.sock.early_data.extend(['HTTP/1.1 200 OK\r\n',
-                                    'Server: BogusServer 1.0\r\n',
-                                    'MultiHeader: Value\r\n'
-                                    'MultiHeader: Other Value\r\n'
-                                    'MultiHeader: One More!\r\n'
-                                    'Content-Length: 10\r\n',
-                                    '\r\n'
-                                    '1234567890'
-                                    ])
+        con.sock.early_data = ['HTTP/1.1 200 OK\r\n',
+                               'Server: BogusServer 1.0\r\n',
+                               'MultiHeader: Value\r\n'
+                               'MultiHeader: Other Value\r\n'
+                               'MultiHeader: One More!\r\n'
+                               'Content-Length: 10\r\n',
+                               '\r\n'
+                               '1234567890'
+                               ]
 
         expected_req = self.doPost(con, False)
         self.assertEqual(None, con.sock,
@@ -92,3 +90,4 @@
                          resp.headers.getheaders('multiheader'))
         self.assertEqual(['BogusServer 1.0'],
                          resp.headers.getheaders('server'))
+# no-check-code
diff --git a/mercurial/httpclient/tests/util.py b/mercurial/httpclient/tests/util.py
--- a/mercurial/httpclient/tests/util.py
+++ b/mercurial/httpclient/tests/util.py
@@ -88,7 +88,7 @@
     def ready_for_read(self):
         return ((self.early_data and http._END_HEADERS in self.sent)
                 or (self.read_wait_sentinel in self.sent and self.data)
-                or self.closed)
+                or self.closed or self.remote_closed)
 
     def send(self, data):
         # this is a horrible mock, but nothing needs us to raise the
@@ -117,6 +117,11 @@
     def __getattr__(self, key):
         return getattr(self._sock, key)
 
+    def __setattr__(self, key, value):
+        if key not in ('_sock', '_fail_recv'):
+            return setattr(self._sock, key, value)
+        return object.__setattr__(self, key, value)
+
     def recv(self, amt=-1):
         try:
             if self._fail_recv:


More information about the Mercurial-devel mailing list