# HG changeset patch # User Augie Fackler # Date 2011-05-17 15:28:03 # Node ID a75e0f4ba0ab91277b654416d93abd12aa50b940 # Parent 436e5379d7ba13cad3438ef4ca3207235bb09717 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 @@ class HTTPResponse(object): 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 @@ class HTTPResponse(object): 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 @@ class HTTPConnection(object): 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 BadRequestData(httplib.HTTPExcepti 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 @@ class SimpleHttpTest(util.HttpTestBase, 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 @@ dotencode 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 @@ class ChunkedTransferTest(util.HttpTestB 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 make_preloaded_socket(data): """ def s(*args, **kwargs): sock = util.MockSocket(*args, **kwargs) - sock.data = data[:] + sock.early_data = data[:] return sock return s @@ -97,24 +97,27 @@ class ProxyHttpTest(util.HttpTestBase, u '\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 @@ class HttpSslTest(util.HttpTestBase, uni 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 @@ class HttpSslTest(util.HttpTestBase, uni 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 @@ class HttpSslTest(util.HttpTestBase, uni 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 @@ class MockSocket(object): 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 @@ class MockSSLSocket(object): 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: