From fbb8d7ebb52038fe445e2ca7e5c0459e8ff5ac5e Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 24 May 2018 13:00:58 -0700 Subject: [PATCH] Clarify that archive location headers should be URL-encoded Fix up function tests to actually *do* that quoting, and fix _listing_pages_iter to respect that. Change-Id: I1554042510819ea878b4c70417721944115e17f4 Related-Bug: 1229142 Related-Change: I425440f76b8328f8e119d390bfa4c7022181e89e Related-Bug: 1755554 Related-Change: Ibcd90cc633c68973929ee5249c6598c22b342e3e --- swift/common/middleware/versioned_writes.py | 8 +++---- test/functional/test_versioned_writes.py | 24 ++++++++++----------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/swift/common/middleware/versioned_writes.py b/swift/common/middleware/versioned_writes.py index 25c7eee927..6022930d5c 100644 --- a/swift/common/middleware/versioned_writes.py +++ b/swift/common/middleware/versioned_writes.py @@ -16,9 +16,9 @@ """ Object versioning in swift is implemented by setting a flag on the container to tell swift to version all objects in the container. The value of the flag is -the container where the versions are stored (commonly referred to as the -"archive container"). The flag itself is one of two headers, which determines -how object ``DELETE`` requests are handled: +the URL-encoded container name where the versions are stored (commonly referred +to as the "archive container"). The flag itself is one of two headers, which +determines how object ``DELETE`` requests are handled: * ``X-History-Location`` @@ -327,7 +327,7 @@ class VersionedWritesContext(WSGIContext): while True: lreq = make_pre_authed_request( env, method='GET', swift_source='VW', - path='/v1/%s/%s' % (account_name, lcontainer)) + path=quote('/v1/%s/%s' % (account_name, lcontainer))) lreq.environ['QUERY_STRING'] = \ 'prefix=%s&marker=%s' % (quote(lprefix), quote(marker)) if end_marker: diff --git a/test/functional/test_versioned_writes.py b/test/functional/test_versioned_writes.py index a4d13a7708..5de994e558 100644 --- a/test/functional/test_versioned_writes.py +++ b/test/functional/test_versioned_writes.py @@ -14,12 +14,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +from copy import deepcopy import json import time import unittest2 +from six.moves.urllib.parse import quote import test.functional as tf -from copy import deepcopy from swift.common.utils import MD5_OF_EMPTY_STRING from test.functional.tests import Base, Base2, BaseEnv, Utils @@ -62,7 +63,7 @@ class TestObjectVersioningEnv(BaseEnv): cls.container = cls.account.container(prefix + "-objs") container_headers = { - cls.location_header_key: cls.versions_container.name} + cls.location_header_key: quote(cls.versions_container.name)} if not cls.container.create(hdrs=container_headers): if cls.conn.response.status == 412: cls.versioning_enabled = False @@ -224,24 +225,22 @@ class TestObjectVersioning(Base): def test_clear_version_option(self): # sanity - self.assertEqual(self.env.container.info()['versions'], - self.env.versions_container.name) + header_val = quote(self.env.versions_container.name) + self.assertEqual(self.env.container.info()['versions'], header_val) self.env.container.update_metadata( hdrs={self.env.location_header_key: ''}) self.assertIsNone(self.env.container.info().get('versions')) # set location back to the way it was self.env.container.update_metadata( - hdrs={self.env.location_header_key: - self.env.versions_container.name}) - self.assertEqual(self.env.container.info()['versions'], - self.env.versions_container.name) + hdrs={self.env.location_header_key: header_val}) + self.assertEqual(self.env.container.info()['versions'], header_val) def _test_overwriting_setup(self, obj_name=None): container = self.env.container versions_container = self.env.versions_container cont_info = container.info() - self.assertEqual(cont_info['versions'], versions_container.name) + self.assertEqual(cont_info['versions'], quote(versions_container.name)) expected_content_types = [] obj_name = obj_name or Utils.create_name() @@ -488,6 +487,7 @@ class TestObjectVersioning(Base): def test_versioning_container_acl(self): # create versions container and DO NOT give write access to account2 versions_container = self.env.account.container(Utils.create_name()) + location_header_val = quote(str(versions_container)) self.assertTrue(versions_container.create(hdrs={ 'X-Container-Write': '' })) @@ -506,7 +506,7 @@ class TestObjectVersioning(Base): # check account2 cannot set X-Versions-Location on container self.assertRaises(ResponseError, container.update_metadata, hdrs={ - self.env.location_header_key: versions_container}, + self.env.location_header_key: location_header_val}, cfg={'use_token': self.env.storage_token2}) # good! now let admin set the X-Versions-Location @@ -514,8 +514,8 @@ class TestObjectVersioning(Base): # of both headers. Setting the location should succeed. self.assertTrue(container.update_metadata(hdrs={ 'X-Remove-' + self.env.location_header_key[len('X-'):]: - versions_container, - self.env.location_header_key: versions_container})) + location_header_val, + self.env.location_header_key: location_header_val})) # write object twice to container and check version obj_name = Utils.create_name()