summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRomain LE DISEZ <romain.le-disez@corp.ovh.com>2017-06-15 22:09:45 +0200
committerRomain LE DISEZ <romain.le-disez@corp.ovh.com>2017-06-15 22:49:17 +0200
commita7c8becd4ed41dbf1c88d36b8601d965d6e7cb75 (patch)
tree343bf255ad2afd21a0581c656110776ad9cea62e
parent6181351a656b68e6163398a891c9efa4e308b459 (diff)
Fix a socket leak in copy middleware
When the "copy" middleware tries to copy a segmented object which is bigger than max_file_size, it immediatly returns "413 Request Entity Too Large". But at that point, connections have already been established by the proxy server to the object servers. These connections must be closed before returning. Closes-Bug: #1698207 Change-Id: I430c48c4a81e8392fa271160bcbc1817ef0a88f7
Notes
Notes (review): Code-Review+2: Tim Burke <tim@swiftstack.com> Code-Review+2: Alistair Coles <alistairncoles@gmail.com> Workflow+1: Alistair Coles <alistairncoles@gmail.com> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Fri, 16 Jun 2017 11:59:45 +0000 Reviewed-on: https://review.openstack.org/474767 Project: openstack/swift Branch: refs/heads/master
-rw-r--r--swift/common/middleware/copy.py3
-rw-r--r--test/unit/common/middleware/test_copy.py10
2 files changed, 10 insertions, 3 deletions
diff --git a/swift/common/middleware/copy.py b/swift/common/middleware/copy.py
index 0ee5315..f6953ab 100644
--- a/swift/common/middleware/copy.py
+++ b/swift/common/middleware/copy.py
@@ -413,9 +413,11 @@ class ServerSideCopyMiddleware(object):
413 # which currently only happens because there are more than 413 # which currently only happens because there are more than
414 # CONTAINER_LISTING_LIMIT segments in a segmented object. In 414 # CONTAINER_LISTING_LIMIT segments in a segmented object. In
415 # this case, we're going to refuse to do the server-side copy. 415 # this case, we're going to refuse to do the server-side copy.
416 close_if_possible(source_resp.app_iter)
416 return HTTPRequestEntityTooLarge(request=req) 417 return HTTPRequestEntityTooLarge(request=req)
417 418
418 if source_resp.content_length > MAX_FILE_SIZE: 419 if source_resp.content_length > MAX_FILE_SIZE:
420 close_if_possible(source_resp.app_iter)
419 return HTTPRequestEntityTooLarge(request=req) 421 return HTTPRequestEntityTooLarge(request=req)
420 422
421 return source_resp 423 return source_resp
@@ -459,7 +461,6 @@ class ServerSideCopyMiddleware(object):
459 ssc_ctx = ServerSideCopyWebContext(self.app, self.logger) 461 ssc_ctx = ServerSideCopyWebContext(self.app, self.logger)
460 source_resp = self._get_source_object(ssc_ctx, source_path, req) 462 source_resp = self._get_source_object(ssc_ctx, source_path, req)
461 if source_resp.status_int >= HTTP_MULTIPLE_CHOICES: 463 if source_resp.status_int >= HTTP_MULTIPLE_CHOICES:
462 close_if_possible(source_resp.app_iter)
463 return source_resp(source_resp.environ, start_response) 464 return source_resp(source_resp.environ, start_response)
464 465
465 # Create a new Request object based on the original request instance. 466 # Create a new Request object based on the original request instance.
diff --git a/test/unit/common/middleware/test_copy.py b/test/unit/common/middleware/test_copy.py
index 08d9acd..bbf74bb 100644
--- a/test/unit/common/middleware/test_copy.py
+++ b/test/unit/common/middleware/test_copy.py
@@ -27,6 +27,7 @@ from swift.common import swob
27from swift.common.middleware import copy 27from swift.common.middleware import copy
28from swift.common.storage_policy import POLICIES 28from swift.common.storage_policy import POLICIES
29from swift.common.swob import Request, HTTPException 29from swift.common.swob import Request, HTTPException
30from swift.common.utils import closing_if_possible
30from test.unit import patch_policies, debug_logger, FakeMemcache, FakeRing 31from test.unit import patch_policies, debug_logger, FakeMemcache, FakeRing
31from test.unit.common.middleware.helpers import FakeSwift 32from test.unit.common.middleware.helpers import FakeSwift
32from test.unit.proxy.controllers.test_obj import set_http_connect, \ 33from test.unit.proxy.controllers.test_obj import set_http_connect, \
@@ -97,6 +98,9 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
97 })(self.app) 98 })(self.app)
98 self.ssc.logger = self.app.logger 99 self.ssc.logger = self.app.logger
99 100
101 def tearDown(self):
102 self.assertEqual(self.app.unclosed_requests, {})
103
100 def call_app(self, req, app=None, expect_exception=False): 104 def call_app(self, req, app=None, expect_exception=False):
101 if app is None: 105 if app is None:
102 app = self.app 106 app = self.app
@@ -122,8 +126,10 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
122 body = '' 126 body = ''
123 caught_exc = None 127 caught_exc = None
124 try: 128 try:
125 for chunk in body_iter: 129 # appease the close-checker
126 body += chunk 130 with closing_if_possible(body_iter):
131 for chunk in body_iter:
132 body += chunk
127 except Exception as exc: 133 except Exception as exc:
128 if expect_exception: 134 if expect_exception:
129 caught_exc = exc 135 caught_exc = exc