diff --git a/swift/common/swob.py b/swift/common/swob.py index fdcbaf3e9b..e6b3d42947 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -1331,7 +1331,7 @@ class Response(object): object length and body or app_iter to reset the content_length properties on the request. - It is ok to not call this method, the conditional resposne will be + It is ok to not call this method, the conditional response will be maintained for you when you __call__ the response. """ self.response_iter = self._response_iter(self.app_iter, self._body) diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index bdb2028f36..e6e3982f88 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -56,9 +56,10 @@ from swift.common.exceptions import ChunkReadTimeout, \ PutterConnectError, ChunkReadError from swift.common.http import ( is_informational, is_success, is_client_error, is_server_error, - HTTP_CONTINUE, HTTP_CREATED, HTTP_MULTIPLE_CHOICES, + is_redirection, HTTP_CONTINUE, HTTP_CREATED, HTTP_MULTIPLE_CHOICES, HTTP_INTERNAL_SERVER_ERROR, HTTP_SERVICE_UNAVAILABLE, - HTTP_INSUFFICIENT_STORAGE, HTTP_PRECONDITION_FAILED, HTTP_CONFLICT) + HTTP_INSUFFICIENT_STORAGE, HTTP_PRECONDITION_FAILED, HTTP_CONFLICT, + HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) from swift.common.storage_policy import (POLICIES, REPL_POLICY, EC_POLICY, ECDriverError, PolicyError) from swift.proxy.controllers.base import Controller, delay_denial, \ @@ -2040,7 +2041,12 @@ class ECObjectController(BaseObjectController): headers=resp_headers, conditional_response=True, app_iter=app_iter) - app_iter.kickoff(req, resp) + try: + app_iter.kickoff(req, resp) + except HTTPException as err_resp: + # catch any HTTPException response here so that we can + # process response headers uniformly in _fix_response + resp = err_resp else: statuses = [] reasons = [] @@ -2060,10 +2066,12 @@ class ECObjectController(BaseObjectController): def _fix_response(self, resp): # 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. + # sysmeta, so we copy that here (if it exists) so the client gets it. + resp.headers['Etag'] = resp.headers.get('X-Object-Sysmeta-Ec-Etag') + if (is_success(resp.status_int) or is_redirection(resp.status_int) or + resp.status_int == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE): + resp.accept_ranges = 'bytes' 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() diff --git a/test/functional/tests.py b/test/functional/tests.py index 09b2068ca0..9e284a3370 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -81,6 +81,14 @@ class Base(unittest.TestCase): 'Status returned: %d Expected: %s' % (self.env.conn.response.status, status_or_statuses)) + def assert_header(self, header_name, expected_value): + try: + actual_value = self.env.conn.response.getheader(header_name) + except KeyError: + self.fail( + 'Expected header name %r not found in response.' % header_name) + self.assertEqual(expected_value, actual_value) + class Base2(object): def setUp(self): @@ -1377,32 +1385,35 @@ class TestFile(Base): self.assert_status(416) else: self.assertEqual(file_item.read(hdrs=hdrs), data[-i:]) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') range_string = 'bytes=%d-' % (i) hdrs = {'Range': range_string} - self.assertTrue( - file_item.read(hdrs=hdrs) == data[i - file_length:], + self.assertEqual( + file_item.read(hdrs=hdrs), data[i - file_length:], range_string) range_string = 'bytes=%d-%d' % (file_length + 1000, file_length + 2000) hdrs = {'Range': range_string} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(416) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') range_string = 'bytes=%d-%d' % (file_length - 1000, file_length + 2000) hdrs = {'Range': range_string} - self.assertTrue( - file_item.read(hdrs=hdrs) == data[-1000:], range_string) + self.assertEqual(file_item.read(hdrs=hdrs), data[-1000:], range_string) hdrs = {'Range': '0-4'} - self.assertTrue(file_item.read(hdrs=hdrs) == data, range_string) + self.assertEqual(file_item.read(hdrs=hdrs), data, '0-4') # RFC 2616 14.35.1 # "If the entity is shorter than the specified suffix-length, the # entire entity-body is used." range_string = 'bytes=-%d' % (file_length + 10) hdrs = {'Range': range_string} - self.assertTrue(file_item.read(hdrs=hdrs) == data, range_string) + self.assertEqual(file_item.read(hdrs=hdrs), data, range_string) def testRangedGetsWithLWSinHeader(self): # Skip this test until webob 1.2 can tolerate LWS in Range header. @@ -2031,6 +2042,17 @@ class TestFileComparison(Base): hdrs = {'If-Match': 'bogus'} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(412) + self.assert_header('etag', file_item.md5) + + def testIfMatchMultipleEtags(self): + for file_item in self.env.files: + hdrs = {'If-Match': '"bogus1", "%s", "bogus2"' % file_item.md5} + self.assertTrue(file_item.read(hdrs=hdrs)) + + hdrs = {'If-Match': '"bogus1", "bogus2", "bogus3"'} + self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) + self.assert_status(412) + self.assert_header('etag', file_item.md5) def testIfNoneMatch(self): for file_item in self.env.files: @@ -2040,6 +2062,20 @@ class TestFileComparison(Base): hdrs = {'If-None-Match': file_item.md5} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(304) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') + + def testIfNoneMatchMultipleEtags(self): + for file_item in self.env.files: + hdrs = {'If-None-Match': '"bogus1", "bogus2", "bogus3"'} + self.assertTrue(file_item.read(hdrs=hdrs)) + + hdrs = {'If-None-Match': + '"bogus1", "bogus2", "%s"' % file_item.md5} + self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) + self.assert_status(304) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') def testIfModifiedSince(self): for file_item in self.env.files: @@ -2050,8 +2086,12 @@ class TestFileComparison(Base): hdrs = {'If-Modified-Since': self.env.time_new} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(304) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') self.assertRaises(ResponseError, file_item.info, hdrs=hdrs) self.assert_status(304) + self.assert_header('etag', file_item.md5) + self.assert_header('accept-ranges', 'bytes') def testIfUnmodifiedSince(self): for file_item in self.env.files: @@ -2062,8 +2102,10 @@ class TestFileComparison(Base): hdrs = {'If-Unmodified-Since': self.env.time_old_f2} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(412) + self.assert_header('etag', file_item.md5) self.assertRaises(ResponseError, file_item.info, hdrs=hdrs) self.assert_status(412) + self.assert_header('etag', file_item.md5) def testIfMatchAndUnmodified(self): for file_item in self.env.files: @@ -2075,33 +2117,38 @@ class TestFileComparison(Base): 'If-Unmodified-Since': self.env.time_new} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(412) + self.assert_header('etag', file_item.md5) hdrs = {'If-Match': file_item.md5, 'If-Unmodified-Since': self.env.time_old_f3} self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(412) + self.assert_header('etag', file_item.md5) def testLastModified(self): file_name = Utils.create_name() content_type = Utils.create_name() - file = self.env.container.file(file_name) - file.content_type = content_type - resp = file.write_random_return_resp(self.env.file_size) + file_item = self.env.container.file(file_name) + file_item.content_type = content_type + resp = file_item.write_random_return_resp(self.env.file_size) put_last_modified = resp.getheader('last-modified') + etag = file_item.md5 - file = self.env.container.file(file_name) - info = file.info() + file_item = self.env.container.file(file_name) + info = file_item.info() self.assertIn('last_modified', info) last_modified = info['last_modified'] self.assertEqual(put_last_modified, info['last_modified']) hdrs = {'If-Modified-Since': last_modified} - self.assertRaises(ResponseError, file.read, hdrs=hdrs) + self.assertRaises(ResponseError, file_item.read, hdrs=hdrs) self.assert_status(304) + self.assert_header('etag', etag) + self.assert_header('accept-ranges', 'bytes') hdrs = {'If-Unmodified-Since': last_modified} - self.assertTrue(file.read(hdrs=hdrs)) + self.assertTrue(file_item.read(hdrs=hdrs)) class TestFileComparisonUTF8(Base2, TestFileComparison): diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index da3abd0c82..8141e46349 100755 --- a/test/unit/proxy/controllers/test_obj.py +++ b/test/unit/proxy/controllers/test_obj.py @@ -2193,7 +2193,7 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): self.assertEqual(resp.status_int, 201) def test_GET_with_invalid_ranges(self): - # reall body size is segment_size - 10 (just 1 segment) + # real body size is segment_size - 10 (just 1 segment) segment_size = self.policy.ec_segment_size real_body = ('a' * segment_size)[:-10] @@ -2205,7 +2205,7 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): segment_size, '%s-' % (segment_size + 10)) def test_COPY_with_invalid_ranges(self): - # reall body size is segment_size - 10 (just 1 segment) + # real body size is segment_size - 10 (just 1 segment) segment_size = self.policy.ec_segment_size real_body = ('a' * segment_size)[:-10] @@ -2218,6 +2218,7 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): def _test_invalid_ranges(self, method, real_body, segment_size, req_range): # make a request with range starts from more than real size. + body_etag = md5(real_body).hexdigest() req = swift.common.swob.Request.blank( '/v1/a/c/o', method=method, headers={'Destination': 'c1/o', @@ -2228,7 +2229,8 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): node_fragments = zip(*fragment_payloads) self.assertEqual(len(node_fragments), self.replicas()) # sanity - headers = {'X-Object-Sysmeta-Ec-Content-Length': str(len(real_body))} + headers = {'X-Object-Sysmeta-Ec-Content-Length': str(len(real_body)), + 'X-Object-Sysmeta-Ec-Etag': body_etag} start = int(req_range.split('-')[0]) self.assertTrue(start >= 0) # sanity title, exp = swob.RESPONSE_REASONS[416] @@ -2248,9 +2250,11 @@ class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): 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) + self.assertEqual(resp.status_int, 416) + self.assertEqual(resp.content_length, len(range_not_satisfiable_body)) + self.assertEqual(resp.body, range_not_satisfiable_body) + self.assertEqual(resp.etag, body_etag) + self.assertEqual(resp.headers['Accept-Ranges'], 'bytes') if __name__ == '__main__': diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 94613f6ea6..6909660eab 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -40,6 +40,7 @@ from swift.obj import diskfile import re import random from collections import defaultdict +import uuid import mock from eventlet import sleep, spawn, wsgi, listen, Timeout, debug @@ -2220,9 +2221,10 @@ class TestObjectController(unittest.TestCase): self.assertEquals(len(error_lines), 0) # sanity self.assertEquals(len(warn_lines), 0) # sanity - @unpatch_policies - def test_conditional_GET_ec(self): - self.put_container("ec", "ec-con") + def _test_conditional_GET(self, policy): + container_name = uuid.uuid4().hex + object_path = '/v1/a/%s/conditionals' % container_name + self.put_container(policy.name, container_name) obj = 'this object has an etag and is otherwise unimportant' etag = md5(obj).hexdigest() @@ -2232,13 +2234,13 @@ class TestObjectController(unittest.TestCase): prosrv = _test_servers[0] sock = connect_tcp(('localhost', prolis.getsockname()[1])) fd = sock.makefile() - fd.write('PUT /v1/a/ec-con/conditionals HTTP/1.1\r\n' + fd.write('PUT %s HTTP/1.1\r\n' 'Host: localhost\r\n' 'Connection: close\r\n' 'Content-Length: %d\r\n' 'X-Storage-Token: t\r\n' 'Content-Type: application/octet-stream\r\n' - '\r\n%s' % (len(obj), obj)) + '\r\n%s' % (object_path, len(obj), obj)) fd.flush() headers = readuntil2crlfs(fd) exp = 'HTTP/1.1 201' @@ -2247,55 +2249,79 @@ class TestObjectController(unittest.TestCase): for verb, body in (('GET', obj), ('HEAD', '')): # If-Match req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-Match': etag}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 200) self.assertEqual(resp.body, body) + self.assertEqual(etag, resp.headers.get('etag')) + self.assertEqual('bytes', resp.headers.get('accept-ranges')) req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-Match': not_etag}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 412) + self.assertEqual(etag, resp.headers.get('etag')) req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-Match': "*"}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 200) self.assertEqual(resp.body, body) + self.assertEqual(etag, resp.headers.get('etag')) + self.assertEqual('bytes', resp.headers.get('accept-ranges')) # If-None-Match req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-None-Match': etag}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 304) + self.assertEqual(etag, resp.headers.get('etag')) + self.assertEqual('bytes', resp.headers.get('accept-ranges')) req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-None-Match': not_etag}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 200) self.assertEqual(resp.body, body) + self.assertEqual(etag, resp.headers.get('etag')) + self.assertEqual('bytes', resp.headers.get('accept-ranges')) req = Request.blank( - '/v1/a/ec-con/conditionals', + object_path, environ={'REQUEST_METHOD': verb}, headers={'If-None-Match': "*"}) resp = req.get_response(prosrv) self.assertEqual(resp.status_int, 304) + self.assertEqual(etag, resp.headers.get('etag')) + self.assertEqual('bytes', resp.headers.get('accept-ranges')) + error_lines = prosrv.logger.get_lines_for_level('error') warn_lines = prosrv.logger.get_lines_for_level('warning') self.assertEquals(len(error_lines), 0) # sanity self.assertEquals(len(warn_lines), 0) # sanity + @unpatch_policies + def test_conditional_GET_ec(self): + policy = POLICIES[3] + self.assertEqual('erasure_coding', policy.policy_type) # sanity + self._test_conditional_GET(policy) + + @unpatch_policies + def test_conditional_GET_replication(self): + policy = POLICIES[0] + self.assertEqual('replication', policy.policy_type) # sanity + self._test_conditional_GET(policy) + @unpatch_policies def test_GET_ec_big(self): self.put_container("ec", "ec-con") @@ -6347,7 +6373,7 @@ class TestObjectECRangedGET(unittest.TestCase): str(s) for s in range(431)) assert seg_size * 4 > len(cls.obj) > seg_size * 3, \ "object is wrong number of segments" - + cls.obj_etag = md5(cls.obj).hexdigest() cls.tiny_obj = 'tiny, tiny object' assert len(cls.tiny_obj) < seg_size, "tiny_obj too large" @@ -6495,9 +6521,11 @@ class TestObjectECRangedGET(unittest.TestCase): def test_unsatisfiable(self): # Goes just one byte too far off the end of the object, so it's # unsatisfiable - status, _junk, _junk = self._get_obj( + status, headers, _junk = self._get_obj( "bytes=%d-%d" % (len(self.obj), len(self.obj) + 100)) self.assertEqual(status, 416) + self.assertEqual(self.obj_etag, headers.get('Etag')) + self.assertEqual('bytes', headers.get('Accept-Ranges')) def test_off_end(self): # Ranged GET that's mostly off the end of the object, but overlaps