From c1c65a7e9f43f1395c63fb7bf22f812962b53714 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 18 Oct 2018 21:32:36 +0000 Subject: [PATCH] Only url-quote Keys when encoding-type=url Previously, we'd even url-quote ETags, leading to weird things like %22a27a822070b843ed6407f4f38afdaa20%22 in listings, where those '%22's should be double quotes. Change-Id: I0f5e0e410c8297a4898caae00c196fa3b3862100 --- .../middleware/s3api/controllers/bucket.py | 30 ++++++++++++------- .../s3api/controllers/multi_upload.py | 16 ++++++---- swift/common/middleware/s3api/etree.py | 13 +------- .../common/middleware/s3api/test_bucket.py | 30 ++++++++++++++++++- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/swift/common/middleware/s3api/controllers/bucket.py b/swift/common/middleware/s3api/controllers/bucket.py index 7b0a28a507..c3ef11cc1f 100644 --- a/swift/common/middleware/s3api/controllers/bucket.py +++ b/swift/common/middleware/s3api/controllers/bucket.py @@ -16,6 +16,8 @@ from base64 import standard_b64encode as b64encode from base64 import standard_b64decode as b64decode +from six.moves.urllib.parse import quote + from swift.common.http import HTTP_OK from swift.common.utils import json, public, config_true_value @@ -171,11 +173,12 @@ class BucketController(Controller): SubElement(elem, 'Marker').text = req.params.get('marker') if is_truncated and 'delimiter' in req.params: if 'name' in objects[-1]: - SubElement(elem, 'NextMarker').text = \ - objects[-1]['name'] - if 'subdir' in objects[-1]: - SubElement(elem, 'NextMarker').text = \ - objects[-1]['subdir'] + name = objects[-1]['name'] + else: + name = objects[-1]['subdir'] + if encoding_type == 'url': + name = quote(name) + SubElement(elem, 'NextMarker').text = name elif listing_type == 'version-2': if is_truncated: if 'name' in objects[-1]: @@ -197,7 +200,7 @@ class BucketController(Controller): if 'delimiter' in req.params: SubElement(elem, 'Delimiter').text = req.params['delimiter'] - if encoding_type is not None: + if encoding_type == 'url': SubElement(elem, 'EncodingType').text = encoding_type SubElement(elem, 'IsTruncated').text = \ @@ -205,14 +208,18 @@ class BucketController(Controller): for o in objects: if 'subdir' not in o: + name = o['name'] + if encoding_type == 'url': + name = quote(name) + if listing_type == 'object-versions': contents = SubElement(elem, 'Version') - SubElement(contents, 'Key').text = o['name'] + SubElement(contents, 'Key').text = name SubElement(contents, 'VersionId').text = 'null' SubElement(contents, 'IsLatest').text = 'true' else: contents = SubElement(elem, 'Contents') - SubElement(contents, 'Key').text = o['name'] + SubElement(contents, 'Key').text = name SubElement(contents, 'LastModified').text = \ o['last_modified'][:-3] + 'Z' if 's3_etag' in o: @@ -231,9 +238,12 @@ class BucketController(Controller): for o in objects: if 'subdir' in o: common_prefixes = SubElement(elem, 'CommonPrefixes') - SubElement(common_prefixes, 'Prefix').text = o['subdir'] + name = o['subdir'] + if encoding_type == 'url': + name = quote(name) + SubElement(common_prefixes, 'Prefix').text = name - body = tostring(elem, encoding_type=encoding_type) + body = tostring(elem) return HTTPOk(body=body, content_type='application/xml') diff --git a/swift/common/middleware/s3api/controllers/multi_upload.py b/swift/common/middleware/s3api/controllers/multi_upload.py index 0e78cff782..f589ec3bf7 100644 --- a/swift/common/middleware/s3api/controllers/multi_upload.py +++ b/swift/common/middleware/s3api/controllers/multi_upload.py @@ -67,7 +67,7 @@ from swift.common.swob import Range from swift.common.utils import json, public from swift.common.db import utf8encode -from six.moves.urllib.parse import urlparse # pylint: disable=F0401 +from six.moves.urllib.parse import quote, urlparse from swift.common.middleware.s3api.controllers.base import Controller, \ bucket_operation, object_operation, check_container_existence @@ -319,7 +319,10 @@ class UploadsController(Controller): # created. for u in uploads: upload_elem = SubElement(result_elem, 'Upload') - SubElement(upload_elem, 'Key').text = u['key'] + name = u['key'] + if encoding_type == 'url': + name = quote(name) + SubElement(upload_elem, 'Key').text = name SubElement(upload_elem, 'UploadId').text = u['upload_id'] initiator_elem = SubElement(upload_elem, 'Initiator') SubElement(initiator_elem, 'ID').text = req.user_id @@ -335,7 +338,7 @@ class UploadsController(Controller): elem = SubElement(result_elem, 'CommonPrefixes') SubElement(elem, 'Prefix').text = p - body = tostring(result_elem, encoding_type=encoding_type) + body = tostring(result_elem) return HTTPOk(body=body, content_type='application/xml') @@ -456,7 +459,10 @@ class UploadController(Controller): result_elem = Element('ListPartsResult') SubElement(result_elem, 'Bucket').text = req.container_name - SubElement(result_elem, 'Key').text = req.object_name + name = req.object_name + if encoding_type == 'url': + name = quote(name) + SubElement(result_elem, 'Key').text = name SubElement(result_elem, 'UploadId').text = upload_id initiator_elem = SubElement(result_elem, 'Initiator') @@ -484,7 +490,7 @@ class UploadController(Controller): SubElement(part_elem, 'ETag').text = '"%s"' % i['hash'] SubElement(part_elem, 'Size').text = str(i['bytes']) - body = tostring(result_elem, encoding_type=encoding_type) + body = tostring(result_elem) return HTTPOk(body=body, content_type='application/xml') diff --git a/swift/common/middleware/s3api/etree.py b/swift/common/middleware/s3api/etree.py index e81af15133..ac4997c692 100644 --- a/swift/common/middleware/s3api/etree.py +++ b/swift/common/middleware/s3api/etree.py @@ -14,7 +14,6 @@ # limitations under the License. import lxml.etree -from urllib import quote from copy import deepcopy from pkg_resources import resource_stream # pylint: disable-msg=E0611 import six @@ -86,7 +85,7 @@ def fromstring(text, root_tag=None, logger=None): return elem -def tostring(tree, encoding_type=None, use_s3ns=True): +def tostring(tree, use_s3ns=True): if use_s3ns: nsmap = tree.nsmap.copy() nsmap[None] = XMLNS_S3 @@ -96,16 +95,6 @@ def tostring(tree, encoding_type=None, use_s3ns=True): root.extend(deepcopy(tree.getchildren())) tree = root - if encoding_type == 'url': - tree = deepcopy(tree) - for e in tree.iter(): - # Some elements are not url-encoded even when we specify - # encoding_type=url. - blacklist = ['LastModified', 'ID', 'DisplayName', 'Initiated'] - if e.tag not in blacklist: - if isinstance(e.text, six.string_types): - e.text = quote(e.text) - return lxml.etree.tostring(tree, xml_declaration=True, encoding='UTF-8') diff --git a/test/unit/common/middleware/s3api/test_bucket.py b/test/unit/common/middleware/s3api/test_bucket.py index 4ecff9eb71..3c68fcebee 100644 --- a/test/unit/common/middleware/s3api/test_bucket.py +++ b/test/unit/common/middleware/s3api/test_bucket.py @@ -17,6 +17,8 @@ import unittest import cgi import mock +from six.moves.urllib.parse import quote + from swift.common import swob from swift.common.swob import Request from swift.common.utils import json @@ -164,7 +166,33 @@ class TestS3ApiBucket(S3ApiTestCase): self.assertEqual(len(names), len(self.objects)) for i in self.objects: - self.assertTrue(i[0] in names) + self.assertIn(i[0], names) + + def test_bucket_GET_url_encoded(self): + bucket_name = 'junk' + req = Request.blank('/%s?encoding-type=url' % bucket_name, + environ={'REQUEST_METHOD': 'GET'}, + headers={'Authorization': 'AWS test:tester:hmac', + 'Date': self.get_date_header()}) + status, headers, body = self.call_s3api(req) + self.assertEqual(status.split()[0], '200') + + elem = fromstring(body, 'ListBucketResult') + name = elem.find('./Name').text + self.assertEqual(name, bucket_name) + + objects = elem.iterchildren('Contents') + + names = [] + for o in objects: + names.append(o.find('./Key').text) + self.assertEqual('2011-01-05T02:19:14.275Z', + o.find('./LastModified').text) + self.assertEqual('"0"', o.find('./ETag').text) + + self.assertEqual(len(names), len(self.objects)) + for i in self.objects: + self.assertIn(quote(i[0]), names) def test_bucket_GET_subdir(self): bucket_name = 'junk-subdir'