From a7c8becd4ed41dbf1c88d36b8601d965d6e7cb75 Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Thu, 15 Jun 2017 22:09:45 +0200 Subject: [PATCH] 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 --- swift/common/middleware/copy.py | 3 ++- test/unit/common/middleware/test_copy.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/swift/common/middleware/copy.py b/swift/common/middleware/copy.py index 0ee5315a60..f6953ab2a6 100644 --- a/swift/common/middleware/copy.py +++ b/swift/common/middleware/copy.py @@ -413,9 +413,11 @@ class ServerSideCopyMiddleware(object): # which currently only happens because there are more than # CONTAINER_LISTING_LIMIT segments in a segmented object. In # this case, we're going to refuse to do the server-side copy. + close_if_possible(source_resp.app_iter) return HTTPRequestEntityTooLarge(request=req) if source_resp.content_length > MAX_FILE_SIZE: + close_if_possible(source_resp.app_iter) return HTTPRequestEntityTooLarge(request=req) return source_resp @@ -459,7 +461,6 @@ class ServerSideCopyMiddleware(object): ssc_ctx = ServerSideCopyWebContext(self.app, self.logger) source_resp = self._get_source_object(ssc_ctx, source_path, req) if source_resp.status_int >= HTTP_MULTIPLE_CHOICES: - close_if_possible(source_resp.app_iter) return source_resp(source_resp.environ, start_response) # 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 08d9acdfb1..bbf74bbc1b 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 from swift.common.middleware import copy from swift.common.storage_policy import POLICIES from swift.common.swob import Request, HTTPException +from swift.common.utils import closing_if_possible from test.unit import patch_policies, debug_logger, FakeMemcache, FakeRing from test.unit.common.middleware.helpers import FakeSwift from test.unit.proxy.controllers.test_obj import set_http_connect, \ @@ -97,6 +98,9 @@ class TestServerSideCopyMiddleware(unittest.TestCase): })(self.app) self.ssc.logger = self.app.logger + def tearDown(self): + self.assertEqual(self.app.unclosed_requests, {}) + def call_app(self, req, app=None, expect_exception=False): if app is None: app = self.app @@ -122,8 +126,10 @@ class TestServerSideCopyMiddleware(unittest.TestCase): body = '' caught_exc = None try: - for chunk in body_iter: - body += chunk + # appease the close-checker + with closing_if_possible(body_iter): + for chunk in body_iter: + body += chunk except Exception as exc: if expect_exception: caught_exc = exc