From 5b246e875fe9ac3d764ea581ad52b04238f5bcc8 Mon Sep 17 00:00:00 2001 From: Kota Tsuyuzaki Date: Wed, 22 Jul 2015 10:39:22 -0700 Subject: [PATCH] Fix EC range GET/COPY handling When range GET (or COPY) for an EC object requested, if the requested range starts from more than last segments alignment (i.e. ceil(object_size/segment_size) * segment_size), proxy server will return the original content length w/o body, though Swift should return an error massage as a body and the length of message as the content length. The current behavior will cause stuck on some client. (e.g. curl) This patch fixes that proxy enables to return correct response, even if such an over range requested. Co-Authored-By: Clay Gerrard Change-Id: I21f81c842f563ac4dddc69011ed759b744bb20bd Closes-Bug: #1475499 --- swift/proxy/controllers/base.py | 2 +- swift/proxy/controllers/obj.py | 13 ++-- test/unit/proxy/controllers/test_obj.py | 88 +++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 8 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 554469cc06..9449607d36 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -1016,7 +1016,7 @@ class ResumingGetter(object): self.statuses.append(possible_source.status) self.reasons.append(possible_source.reason) - self.bodies.append('') + self.bodies.append(None) self.source_headers.append(possible_source.getheaders()) sources.append((possible_source, node)) if not self.newest: # one good source is enough diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index 2aac83f2e5..7ce8463496 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -2029,13 +2029,12 @@ class ECObjectController(BaseObjectController): # EC fragment archives each have different bytes, hence different # etags. However, they all have the original object's etag stored in # sysmeta, so we copy that here so the client gets it. - resp.headers['Etag'] = resp.headers.get( - 'X-Object-Sysmeta-Ec-Etag') - resp.headers['Content-Length'] = resp.headers.get( - 'X-Object-Sysmeta-Ec-Content-Length') - resp.fix_conditional_response() - - return resp + if is_success(resp.status_int): + resp.headers['Etag'] = resp.headers.get( + 'X-Object-Sysmeta-Ec-Etag') + resp.headers['Content-Length'] = resp.headers.get( + 'X-Object-Sysmeta-Ec-Content-Length') + resp.fix_conditional_response() def _connect_put_node(self, node_iter, part, path, headers, logger_thread_locals): diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index 3cc9ce65dc..93c16215c4 100755 --- a/test/unit/proxy/controllers/test_obj.py +++ b/test/unit/proxy/controllers/test_obj.py @@ -1467,6 +1467,34 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): self.assertEqual(1, len(error_lines)) self.assertTrue('retrying' in error_lines[0]) + def test_fix_response_HEAD(self): + headers = {'X-Object-Sysmeta-Ec-Content-Length': '10', + 'X-Object-Sysmeta-Ec-Etag': 'foo'} + + # sucsessful HEAD + responses = [(200, '', headers)] + status_codes, body_iter, headers = zip(*responses) + req = swift.common.swob.Request.blank('/v1/a/c/o', method='HEAD') + with set_http_connect(*status_codes, body_iter=body_iter, + headers=headers): + resp = req.get_response(self.app) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.body, '') + # 200OK shows original object content length + self.assertEquals(resp.headers['Content-Length'], '10') + self.assertEquals(resp.headers['Etag'], 'foo') + + # not found HEAD + responses = [(404, '', {})] * self.replicas() * 2 + status_codes, body_iter, headers = zip(*responses) + req = swift.common.swob.Request.blank('/v1/a/c/o', method='HEAD') + with set_http_connect(*status_codes, body_iter=body_iter, + headers=headers): + resp = req.get_response(self.app) + self.assertEquals(resp.status_int, 404) + # 404 shows actual response body size (i.e. 0 for HEAD) + self.assertEquals(resp.headers['Content-Length'], '0') + def test_PUT_with_slow_commits(self): # It's important that this timeout be much less than the delay in # the slow commit responses so that the slow commits are not waited @@ -1530,6 +1558,66 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): resp = req.get_response(self.app) self.assertEqual(resp.status_int, 201) + def test_GET_with_invalid_ranges(self): + # reall body size is segment_size - 10 (just 1 segment) + segment_size = self.policy.ec_segment_size + real_body = ('a' * segment_size)[:-10] + + # range is out of real body but in segment size + self._test_invalid_ranges('GET', real_body, + segment_size, '%s-' % (segment_size - 10)) + # range is out of both real body and segment size + self._test_invalid_ranges('GET', real_body, + segment_size, '%s-' % (segment_size + 10)) + + def test_COPY_with_invalid_ranges(self): + # reall body size is segment_size - 10 (just 1 segment) + segment_size = self.policy.ec_segment_size + real_body = ('a' * segment_size)[:-10] + + # range is out of real body but in segment size + self._test_invalid_ranges('COPY', real_body, + segment_size, '%s-' % (segment_size - 10)) + # range is out of both real body and segment size + self._test_invalid_ranges('COPY', real_body, + segment_size, '%s-' % (segment_size + 10)) + + def _test_invalid_ranges(self, method, real_body, segment_size, req_range): + # make a request with range starts from more than real size. + req = swift.common.swob.Request.blank( + '/v1/a/c/o', method=method, + headers={'Destination': 'c1/o', + 'Range': 'bytes=%s' % (req_range)}) + + fragments = self.policy.pyeclib_driver.encode(real_body) + fragment_payloads = [fragments] + + node_fragments = zip(*fragment_payloads) + self.assertEqual(len(node_fragments), self.replicas()) # sanity + headers = {'X-Object-Sysmeta-Ec-Content-Length': str(len(real_body))} + start = int(req_range.split('-')[0]) + self.assertTrue(start >= 0) # sanity + title, exp = swob.RESPONSE_REASONS[416] + range_not_satisfiable_body = \ + '

%s

%s

' % (title, exp) + if start >= segment_size: + responses = [(416, range_not_satisfiable_body, headers) + for i in range(POLICIES.default.ec_ndata)] + else: + responses = [(200, ''.join(node_fragments[i]), headers) + for i in range(POLICIES.default.ec_ndata)] + status_codes, body_iter, headers = zip(*responses) + expect_headers = { + 'X-Obj-Metadata-Footer': 'yes', + 'X-Obj-Multiphase-Commit': 'yes' + } + with set_http_connect(*status_codes, body_iter=body_iter, + headers=headers, expect_headers=expect_headers): + resp = req.get_response(self.app) + self.assertEquals(resp.status_int, 416) + self.assertEquals(resp.content_length, len(range_not_satisfiable_body)) + self.assertEquals(resp.body, range_not_satisfiable_body) + if __name__ == '__main__': unittest.main()