From ae247a5ff6bd937f2049b2d403893d46840dfc21 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sun, 21 Oct 2012 14:09:05 +0200 Subject: [PATCH 1/2] Issue #16220: wsgiref now always calls close() on an iterable response. Patch by Brent Tubbs. --- Lib/test/test_wsgiref.py | 49 +++++++++++++++------------------------- Lib/wsgiref/handlers.py | 12 ++++++---- Misc/ACKS | 1 + Misc/NEWS | 3 +++ 4 files changed, 29 insertions(+), 36 deletions(-) diff --git a/Lib/test/test_wsgiref.py b/Lib/test/test_wsgiref.py index a08f66b7b47..c06f94a8882 100644 --- a/Lib/test/test_wsgiref.py +++ b/Lib/test/test_wsgiref.py @@ -656,40 +656,27 @@ class HandlerTests(TestCase): b"data", h.stdout.getvalue()) -# This epilogue is needed for compatibility with the Python 2.5 regrtest module + def testCloseOnError(self): + side_effects = {'close_called': False} + MSG = b"Some output has been sent" + def error_app(e,s): + s("200 OK",[])(MSG) + class CrashyIterable(object): + def __iter__(self): + while True: + yield b'blah' + raise AssertionError("This should be caught by handler") + def close(self): + side_effects['close_called'] = True + return CrashyIterable() + + h = ErrorHandler() + h.run(error_app) + self.assertEqual(side_effects['close_called'], True) + def test_main(): support.run_unittest(__name__) if __name__ == "__main__": test_main() - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -# the above lines intentionally left blank diff --git a/Lib/wsgiref/handlers.py b/Lib/wsgiref/handlers.py index 67064a68756..63d5993eca0 100644 --- a/Lib/wsgiref/handlers.py +++ b/Lib/wsgiref/handlers.py @@ -174,11 +174,13 @@ class BaseHandler: in the event loop to iterate over the data, and to call 'self.close()' once the response is finished. """ - if not self.result_is_file() or not self.sendfile(): - for data in self.result: - self.write(data) - self.finish_content() - self.close() + try: + if not self.result_is_file() or not self.sendfile(): + for data in self.result: + self.write(data) + self.finish_content() + finally: + self.close() def get_scheme(self): diff --git a/Misc/ACKS b/Misc/ACKS index ba87ecbb79b..8bdbe8f3c29 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1067,6 +1067,7 @@ Richard Townsend Laurence Tratt John Tromp Jason Trowbridge +Brent Tubbs Anthony Tuininga Erno Tukia David Turner diff --git a/Misc/NEWS b/Misc/NEWS index 107222f332c..b68970ae149 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -132,6 +132,9 @@ Core and Builtins Library ------- +- Issue #16220: wsgiref now always calls close() on an iterable response. + Patch by Brent Tubbs. + - Issue #16270: urllib may hang when used for retrieving files via FTP by using a context manager. Patch by Giampaolo Rodola'. From 66510fedb410804b224360c6bf75041920b77709 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sun, 21 Oct 2012 14:13:32 +0200 Subject: [PATCH 2/2] Fix whacky spacking in test_wsgiref --- Lib/test/test_wsgiref.py | 57 ---------------------------------------- 1 file changed, 57 deletions(-) diff --git a/Lib/test/test_wsgiref.py b/Lib/test/test_wsgiref.py index c06f94a8882..08f8d9a6043 100644 --- a/Lib/test/test_wsgiref.py +++ b/Lib/test/test_wsgiref.py @@ -39,9 +39,6 @@ class MockHandler(WSGIRequestHandler): pass - - - def hello_app(environ,start_response): start_response("200 OK", [ ('Content-Type','text/plain'), @@ -63,28 +60,6 @@ def run_amock(app=hello_app, data=b"GET / HTTP/1.0\n\n"): return out.getvalue(), err.getvalue() - - - - - - - - - - - - - - - - - - - - - - def compare_generic_iter(make_it,match): """Utility to compare a generic 2.1/2.2+ iterator with an iterable @@ -122,10 +97,6 @@ def compare_generic_iter(make_it,match): raise AssertionError("Too many items from .__next__()", it) - - - - class IntegrationTests(TestCase): def check_hello(self, out, has_length=True): @@ -195,8 +166,6 @@ class IntegrationTests(TestCase): out) - - class UtilityTests(TestCase): def checkShift(self,sn_in,pi_in,part,sn_out,pi_out): @@ -235,11 +204,6 @@ class UtilityTests(TestCase): util.setup_testing_defaults(kw) self.assertEqual(util.request_uri(kw,query),uri) - - - - - def checkFW(self,text,size,match): def make_it(text=text,size=size): @@ -258,7 +222,6 @@ class UtilityTests(TestCase): it.close() self.assertTrue(it.filelike.closed) - def testSimpleShifts(self): self.checkShift('','/', '', '/', '') self.checkShift('','/x', 'x', '/x', '') @@ -266,7 +229,6 @@ class UtilityTests(TestCase): self.checkShift('/a','/x/y', 'x', '/a/x', '/y') self.checkShift('/a','/x/', 'x', '/a/x', '/') - def testNormalizedShifts(self): self.checkShift('/a/b', '/../y', '..', '/a', '/y') self.checkShift('', '/../y', '..', '', '/y') @@ -280,7 +242,6 @@ class UtilityTests(TestCase): self.checkShift('/a/b', '/x//', 'x', '/a/b/x', '/') self.checkShift('/a/b', '/.', None, '/a/b', '') - def testDefaults(self): for key, value in [ ('SERVER_NAME','127.0.0.1'), @@ -300,7 +261,6 @@ class UtilityTests(TestCase): ]: self.checkDefault(key,value) - def testCrossDefaults(self): self.checkCrossDefault('HTTP_HOST',"foo.bar",SERVER_NAME="foo.bar") self.checkCrossDefault('wsgi.url_scheme',"https",HTTPS="on") @@ -310,7 +270,6 @@ class UtilityTests(TestCase): self.checkCrossDefault('SERVER_PORT',"80",HTTPS="foo") self.checkCrossDefault('SERVER_PORT',"443",HTTPS="on") - def testGuessScheme(self): self.assertEqual(util.guess_scheme({}), "http") self.assertEqual(util.guess_scheme({'HTTPS':"foo"}), "http") @@ -318,10 +277,6 @@ class UtilityTests(TestCase): self.assertEqual(util.guess_scheme({'HTTPS':"yes"}), "https") self.assertEqual(util.guess_scheme({'HTTPS':"1"}), "https") - - - - def testAppURIs(self): self.checkAppURI("http://127.0.0.1/") self.checkAppURI("http://127.0.0.1/spam", SCRIPT_NAME="/spam") @@ -446,15 +401,6 @@ class TestHandler(ErrorHandler): raise # for testing, we want to see what's happening - - - - - - - - - class HandlerTests(TestCase): def checkEnvironAttrs(self, handler): @@ -495,7 +441,6 @@ class HandlerTests(TestCase): h=TestHandler(); h.setup_environ() self.assertEqual(h.environ['wsgi.url_scheme'],'http') - def testAbstractMethods(self): h = BaseHandler() for name in [ @@ -504,7 +449,6 @@ class HandlerTests(TestCase): self.assertRaises(NotImplementedError, getattr(h,name)) self.assertRaises(NotImplementedError, h._write, "test") - def testContentLength(self): # Demo one reason iteration is better than write()... ;) @@ -596,7 +540,6 @@ class HandlerTests(TestCase): "\r\n".encode("iso-8859-1")+MSG)) self.assertIn("AssertionError", h.stderr.getvalue()) - def testHeaderFormats(self): def non_error_app(e,s):