From fb3692c9bb662d9cadc4238920f86676857a7f1f Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Wed, 13 Apr 2016 11:07:44 -0700 Subject: [PATCH] Don't include conditional headers in SLO HEAD requests Previously, attempting to PUT a SLO manifest with `If-None-Match: *` would include the header when validating the segments, causing the upload to fail. Now when SLO validates segments, no conditional headers will be included in the HEAD request. Change-Id: I03ad454092d3caa73d29e6d30d8033b45bc96136 Closes-Bug: #1569253 --- swift/common/middleware/slo.py | 22 ++++++++-------------- test/functional/tests.py | 21 +++++++++++++++++++++ test/unit/common/middleware/helpers.py | 4 ++-- test/unit/common/middleware/test_slo.py | 20 ++++++++++++++++++++ 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/swift/common/middleware/slo.py b/swift/common/middleware/slo.py index a3291dd7fb..0216264b99 100644 --- a/swift/common/middleware/slo.py +++ b/swift/common/middleware/slo.py @@ -399,7 +399,7 @@ class SloGetContext(WSGIContext): req.environ, path='/'.join(['', version, acc, con, obj]), method='GET', headers={'x-auth-token': req.headers.get('x-auth-token')}, - agent=('%(orig)s ' + 'SLO MultipartGET'), swift_source='SLO') + agent='%(orig)s SLO MultipartGET', swift_source='SLO') sub_resp = sub_req.get_response(self.slo.app) if not is_success(sub_resp.status_int): @@ -603,7 +603,7 @@ class SloGetContext(WSGIContext): get_req = make_subrequest( req.environ, method='GET', headers={'x-auth-token': req.headers.get('x-auth-token')}, - agent=('%(orig)s ' + 'SLO MultipartGET'), swift_source='SLO') + agent='%(orig)s SLO MultipartGET', swift_source='SLO') resp_iter = self._app_call(get_req.environ) # Any Content-Range from a manifest is almost certainly wrong for the @@ -857,20 +857,14 @@ class StaticLargeObject(object): obj_name = obj_name.encode('utf-8') obj_path = '/'.join(['', vrs, account, obj_name.lstrip('/')]) - new_env = req.environ.copy() - new_env['PATH_INFO'] = obj_path - new_env['REQUEST_METHOD'] = 'HEAD' - new_env['swift.source'] = 'SLO' - del(new_env['wsgi.input']) - del(new_env['QUERY_STRING']) - new_env['CONTENT_LENGTH'] = 0 - new_env['HTTP_USER_AGENT'] = \ - '%s MultipartPUT' % req.environ.get('HTTP_USER_AGENT') - if obj_path != last_obj_path: last_obj_path = obj_path - head_seg_resp = \ - Request.blank(obj_path, new_env).get_response(self) + sub_req = make_subrequest( + req.environ, path=obj_path + '?', # kill the query string + method='HEAD', + headers={'x-auth-token': req.headers.get('x-auth-token')}, + agent='%(orig)s SLO MultipartPUT', swift_source='SLO') + head_seg_resp = sub_req.get_response(self) if head_seg_resp.is_success: segment_length = head_seg_resp.content_length diff --git a/test/functional/tests.py b/test/functional/tests.py index 35339e68ec..f7430a69b6 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -3307,6 +3307,27 @@ class TestSlo(Base): manifest.read(hdrs={'If-Match': etag}) self.assert_status(200) + def test_slo_if_none_match_put(self): + file_item = self.env.container.file("manifest-if-none-match") + manifest = json.dumps([{ + 'size_bytes': 1024 * 1024, + 'etag': None, + 'path': '/%s/%s' % (self.env.container.name, 'seg_a')}]) + + self.assertRaises(ResponseError, file_item.write, manifest, + parms={'multipart-manifest': 'put'}, + hdrs={'If-None-Match': '"not-star"'}) + self.assert_status(400) + + file_item.write(manifest, parms={'multipart-manifest': 'put'}, + hdrs={'If-None-Match': '*'}) + self.assert_status(201) + + self.assertRaises(ResponseError, file_item.write, manifest, + parms={'multipart-manifest': 'put'}, + hdrs={'If-None-Match': '*'}) + self.assert_status(412) + def test_slo_if_none_match_get(self): manifest = self.env.container.file("manifest-abcde") etag = manifest.info()['etag'] diff --git a/test/unit/common/middleware/helpers.py b/test/unit/common/middleware/helpers.py index 0847a1cbcf..2c5328ba26 100644 --- a/test/unit/common/middleware/helpers.py +++ b/test/unit/common/middleware/helpers.py @@ -119,10 +119,10 @@ class FakeSwift(object): if "CONTENT_TYPE" in env: self.uploaded[path][0]['Content-Type'] = env["CONTENT_TYPE"] - # range requests ought to work, hence conditional_response=True + # range requests ought to work, which require conditional_response=True req = swob.Request(env) resp = resp_class(req=req, headers=headers, body=body, - conditional_response=True) + conditional_response=req.method in ('GET', 'HEAD')) wsgi_iter = resp(env, start_response) self.mark_opened(path) return LeakTrackingIter(wsgi_iter, self, path) diff --git a/test/unit/common/middleware/test_slo.py b/test/unit/common/middleware/test_slo.py index 830892a26c..03f5c23213 100644 --- a/test/unit/common/middleware/test_slo.py +++ b/test/unit/common/middleware/test_slo.py @@ -621,6 +621,26 @@ class TestSloPutManifest(SloTestCase): self.assertEqual(400, catcher.exception.status_int) self.assertIn("Unsatisfiable Range", catcher.exception.body) + def test_handle_multipart_put_success_conditional(self): + test_json_data = json.dumps([{'path': u'/cont/object', + 'etag': 'etagoftheobjectsegment', + 'size_bytes': 100}]) + req = Request.blank( + '/v1/AUTH_test/c/man?multipart-manifest=put', + environ={'REQUEST_METHOD': 'PUT'}, headers={'If-None-Match': '*'}, + body=test_json_data) + status, headers, body = self.call_slo(req) + self.assertEqual(('201 Created', ''), (status, body)) + self.assertEqual([ + ('HEAD', '/v1/AUTH_test/cont/object'), + ('PUT', '/v1/AUTH_test/c/man?multipart-manifest=put'), + ], self.app.calls) + # HEAD shouldn't be conditional + self.assertNotIn('If-None-Match', self.app.headers[0]) + # But the PUT should be + self.assertIn('If-None-Match', self.app.headers[1]) + self.assertEqual('*', self.app.headers[1]['If-None-Match']) + def test_handle_single_ranges(self): good_data = json.dumps( [{'path': '/checktest/a_1', 'etag': None,