[Python-checkins] cpython (merge 3.4 -> default): Issue #19524: Fixed resource leak in the HTTP connection when an invalid
serhiy.storchaka
python-checkins at python.org
Sat Sep 6 20:46:08 CEST 2014
http://hg.python.org/cpython/rev/43bf95480c3c
changeset: 92367:43bf95480c3c
parent: 92365:25032ec29315
parent: 92366:c1fb19907cc4
user: Serhiy Storchaka <storchaka at gmail.com>
date: Sat Sep 06 21:43:49 2014 +0300
summary:
Issue #19524: Fixed resource leak in the HTTP connection when an invalid
response is received. Patch by Martin Panter.
files:
Lib/test/test_urllib.py | 73 ++++++++++++++-------------
Lib/test/test_urllib2.py | 29 +++++++++++
Lib/urllib/request.py | 25 +++++----
Misc/ACKS | 1 +
Misc/NEWS | 3 +
5 files changed, 86 insertions(+), 45 deletions(-)
diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
--- a/Lib/test/test_urllib.py
+++ b/Lib/test/test_urllib.py
@@ -48,43 +48,48 @@
return opener.open(url, data)
+def fakehttp(fakedata):
+ class FakeSocket(io.BytesIO):
+ io_refs = 1
+
+ def sendall(self, data):
+ FakeHTTPConnection.buf = data
+
+ def makefile(self, *args, **kwds):
+ self.io_refs += 1
+ return self
+
+ def read(self, amt=None):
+ if self.closed:
+ return b""
+ return io.BytesIO.read(self, amt)
+
+ def readline(self, length=None):
+ if self.closed:
+ return b""
+ return io.BytesIO.readline(self, length)
+
+ def close(self):
+ self.io_refs -= 1
+ if self.io_refs == 0:
+ io.BytesIO.close(self)
+
+ class FakeHTTPConnection(http.client.HTTPConnection):
+
+ # buffer to store data for verification in urlopen tests.
+ buf = None
+ fakesock = FakeSocket(fakedata)
+
+ def connect(self):
+ self.sock = self.fakesock
+
+ return FakeHTTPConnection
+
+
class FakeHTTPMixin(object):
def fakehttp(self, fakedata):
- class FakeSocket(io.BytesIO):
- io_refs = 1
-
- def sendall(self, data):
- FakeHTTPConnection.buf = data
-
- def makefile(self, *args, **kwds):
- self.io_refs += 1
- return self
-
- def read(self, amt=None):
- if self.closed:
- return b""
- return io.BytesIO.read(self, amt)
-
- def readline(self, length=None):
- if self.closed:
- return b""
- return io.BytesIO.readline(self, length)
-
- def close(self):
- self.io_refs -= 1
- if self.io_refs == 0:
- io.BytesIO.close(self)
-
- class FakeHTTPConnection(http.client.HTTPConnection):
-
- # buffer to store data for verification in urlopen tests.
- buf = None
-
- def connect(self):
- self.sock = FakeSocket(fakedata)
-
self._connection_class = http.client.HTTPConnection
- http.client.HTTPConnection = FakeHTTPConnection
+ http.client.HTTPConnection = fakehttp(fakedata)
def unfakehttp(self):
http.client.HTTPConnection = self._connection_class
diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -1,5 +1,6 @@
import unittest
from test import support
+from test import test_urllib
import os
import io
@@ -13,6 +14,7 @@
from urllib.request import Request, OpenerDirector, _parse_proxy, _proxy_bypass_macosx_sysconf
from urllib.parse import urlparse
import urllib.error
+import http.client
# XXX
# Request
@@ -1393,6 +1395,33 @@
self.assertEqual(len(http_handler.requests), 1)
self.assertFalse(http_handler.requests[0].has_header(auth_header))
+ def test_http_closed(self):
+ """Test the connection is cleaned up when the response is closed"""
+ for (transfer, data) in (
+ ("Connection: close", b"data"),
+ ("Transfer-Encoding: chunked", b"4\r\ndata\r\n0\r\n\r\n"),
+ ("Content-Length: 4", b"data"),
+ ):
+ header = "HTTP/1.1 200 OK\r\n{}\r\n\r\n".format(transfer)
+ conn = test_urllib.fakehttp(header.encode() + data)
+ handler = urllib.request.AbstractHTTPHandler()
+ req = Request("http://dummy/")
+ req.timeout = None
+ with handler.do_open(conn, req) as resp:
+ resp.read()
+ self.assertTrue(conn.fakesock.closed,
+ "Connection not closed with {!r}".format(transfer))
+
+ def test_invalid_closed(self):
+ """Test the connection is cleaned up after an invalid response"""
+ conn = test_urllib.fakehttp(b"")
+ handler = urllib.request.AbstractHTTPHandler()
+ req = Request("http://dummy/")
+ req.timeout = None
+ with self.assertRaises(http.client.BadStatusLine):
+ handler.do_open(conn, req)
+ self.assertTrue(conn.fakesock.closed, "Connection not closed")
+
class MiscTests(unittest.TestCase):
diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -1170,18 +1170,21 @@
h.set_tunnel(req._tunnel_host, headers=tunnel_headers)
try:
- h.request(req.get_method(), req.selector, req.data, headers)
- except OSError as err: # timeout error
+ try:
+ h.request(req.get_method(), req.selector, req.data, headers)
+ except OSError as err: # timeout error
+ raise URLError(err)
+ r = h.getresponse()
+ except:
h.close()
- raise URLError(err)
- else:
- r = h.getresponse()
- # If the server does not send us a 'Connection: close' header,
- # HTTPConnection assumes the socket should be left open. Manually
- # mark the socket to be closed when this response object goes away.
- if h.sock:
- h.sock.close()
- h.sock = None
+ raise
+
+ # If the server does not send us a 'Connection: close' header,
+ # HTTPConnection assumes the socket should be left open. Manually
+ # mark the socket to be closed when this response object goes away.
+ if h.sock:
+ h.sock.close()
+ h.sock = None
r.url = req.get_full_url()
# This line replaces the .msg attribute of the HTTPResponse
diff --git a/Misc/ACKS b/Misc/ACKS
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1012,6 +1012,7 @@
Todd R. Palmer
Juan David Ibáñez Palomar
Jan Palus
+Martin Panter
Mathias Panzenböck
M. Papillon
Peter Parente
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -132,6 +132,9 @@
Library
-------
+- Issue #19524: Fixed resource leak in the HTTP connection when an invalid
+ response is received. Patch by Martin Panter.
+
- Issue #20421: Add a .version() method to SSL sockets exposing the actual
protocol version in use.
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list