FFFF Issue #19524: Fixed resource leak in the HTTP connection when an invalid · python/cpython@f54c350 · GitHub
[go: up one dir, main page]
Skip to content

Commit f54c350

Browse files
serhiy-storchakaserhiy-storchaka
committed
Issue #19524: Fixed resource leak in the HTTP connection when an invalid
response is received. Patch by Martin Panter.
1 parent 1d52096 commit f54c350

File tree

5 files changed

+79
-38
lines changed

5 files changed

+79
-38
lines changed

‎Lib/test/test_urllib.py‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -48,43 +48,48 @@ def urlopen(url, data=None, proxies=None):
4848
return opener.open(url, data)
4949

5050

51-
class FakeHTTPMixin(object):
52-
def fakehttp(self, fakedata):
53-
class FakeSocket(io.BytesIO):
54-
io_refs = 1
51+
def fakehttp(fakedata):
52+
class FakeSocket(io.BytesIO):
53+
io_refs = 1
5554

56-
def sendall(self, data):
57-
FakeHTTPConnection.buf = data
55+
def sendall(self, data):
56+
FakeHTTPConnection.buf = data
5857

59-
def makefile(self, *args, **kwds):
60-
self.io_refs += 1
61-
return self
58+
def makefile(self, *args, **kwds):
59+
self.io_refs += 1
60+
return self
6261

63-
def read(self, amt=None):
64-
if self.closed:
65-
return b""
66-
return io.BytesIO.read(self, amt)
62+
def read(self, amt=None):
63+
if self.closed:
64+
return b""
65+
return io.BytesIO.read(self, amt)
6766

68-
def readline(self, length=None):
69-
if self.closed:
70-
return b""
71-
return io.BytesIO.readline(self, length)
67+
def readline(self, length=None):
68+
if self.closed:
69+
return b""
70+
return io.BytesIO.readline(self, length)
7271

73-
def close(self):
74-
self.io_refs -= 1
75-
if self.io_refs == 0:
76-
io.BytesIO.close(self)
72+
def close(self):
73+
self.io_refs -= 1
74+
if self.io_refs == 0:
75+
io.BytesIO.close(self)
76+
77+
class FakeHTTPConnection(http.client.HTTPConnection):
7778

78-
class FakeHTTPConnection(http.client.HTTPConnection):
79+
# buffer to store data for verification in urlopen tests.
80+
buf = None
81+
fakesock = FakeSocket(fakedata)
7982

80-
# buffer to store data for verification in urlopen tests.
81-
buf = None
83+
def connect(self):
84+
self.sock = self.fakesock
8285

83-
def connect(self):
84-
self.sock = FakeSocket(fakedata)
86+
return FakeHTTPConnection
8587

88+
89+
class FakeHTTPMixin(object):
90+
def fakehttp(self, fakedata):
8691
self._connection_class = http.client.HTTPConnection
87-
http.client.HTTPConnection = FakeHTTPConnection
92+
http.client.HTTPConnection = fakehttp(fakedata)
8893

8994
def unfakehttp(self):
9095
http.client.HTTPConnection = self._connection_class

‎Lib/test/test_urllib2.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import unittest
22
from test import support
3+
from test import test_urllib
34

45
import os
56
import io
@@ -13,6 +14,7 @@
1314
from urllib.request import Request, OpenerDirector, _parse_proxy, _proxy_bypass_macosx_sysconf
1415
from urllib.parse import urlparse
1516
import urllib.error
17+
import http.client
1618

1719
# XXX
1820
# Request
@@ -1393,6 +1395,33 @@ def _test_basic_auth(self, opener, auth_handler, auth_header,
13931395
self.assertEqual(len(http_handler.requests), 1)
13941396
self.assertFalse(http_handler.requests[0].has_header(auth_header))
13951397

1398+
def test_http_closed(self):
1399+
"""Test the connection is cleaned up when the response is closed"""
1400+
for (transfer, data) in (
1401+
("Connection: close", b"data"),
1402+
("Transfer-Encoding: chunked", b"4\r\ndata\r\n0\r\n\r\n"),
1403+
("Content-Length: 4", b"data"),
1404+
):
1405+
header = "HTTP/1.1 200 OK\r\n{}\r\n\r\n".format(transfer)
1406+
conn = test_urllib.fakehttp(header.encode() + data)
1407+
handler = urllib.request.AbstractHTTPHandler()
1408+
req = Request("http://dummy/")
1409+
req.timeout = None
1410+
with handler.do_open(conn, req) as resp:
1411+
resp.read()
1412+
self.assertTrue(conn.fakesock.closed,
1413+
"Connection not closed with {!r}".format(transfer))
1414+
1415+
def test_invalid_closed(self):
1416+
"""Test the connection is cleaned up after an invalid response"""
1417+
conn = test_urllib.fakehttp(b"")
1418+
handler = urllib.request.AbstractHTTPHandler()
1419+
req = Request("http://dummy/")
1420+
req.timeout = None
1421+
with self.assertRaises(http.client.BadStatusLine):
1422+
handler.do_open(conn, req)
1423+
self.assertTrue(conn.fakesock.closed, "Connection not closed")
1424+
13961425

13971426
class MiscTests(unittest.TestCase):
13981427

‎Lib/urllib/request.py‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,18 +1170,21 @@ def do_open(self, http_class, req, **http_conn_args):
11701170
h.set_tunnel(req._tunnel_host, headers=tunnel_headers)
11711171

11721172
try:
1173-
h.request(req.get_method(), req.selector, req.data, headers)
1174-
except OSError as err: # timeout error
1175-
h.close()
1176-
raise URLError(err)
1177-
else:
1173+
try:
1174+
h.request(req.get_method(), req.selector, req.data, headers)
1175+
except OSError as err: # timeout error
1176+
raise URLError(err)
11781177
r = h.getresponse()
1179-
# If the server does not send us a 'Connection: close' header,
1180-
# HTTPConnection assumes the socket should be left open. Manually
1181-
# mark the socket to be closed when this response object goes away.
1182-
if h.sock:
1183-
h.sock.close()
1184-
h.sock = None
1178+
except:
1179+
h.close()
1180+
raise
1181+
1182+
# If the server does not send us a 'Connection: close' header,
1183+
# HTTPConnection assumes the socket should be left open. Manually
1184+
# mark the socket to be closed when this response object goes away.
1185+
if h.sock:
1186+
h.sock.close()
1187+
h.sock = None
11851188

11861189
r.url = req.get_full_url()
11871190
# This line replaces the .msg attribute of the HTTPResponse

‎Misc/ACKS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,7 @@ Mike Pall
10031003
Todd R. Palmer
10041004 4268
Juan David Ibáñez Palomar
10051005
Jan Palus
1006+
Martin Panter
10061007
Mathias Panzenböck
10071008
M. Papillon
10081009
Peter Parente

‎Misc/NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ Core and Builtins
3232
Library
3333
-------
3434

35+
- Issue #19524: Fixed resource leak in the HTTP connection when an invalid
36+
response is received. Patch by Martin Panter.
37+
3538
- Issue #22051: turtledemo no longer reloads examples to re-run them.
3639
Initialization of variables and gui setup should be done in main(),
3740
which is called each time a demo is run, but not on import.

0 commit comments

Comments
 (0)
0