From dd64a81e652411ab41406096b836087eacb2c1a0 Mon Sep 17 00:00:00 2001 From: Aymeric Ducroquetz Date: Tue, 1 Mar 2022 13:47:07 +0100 Subject: [PATCH] s3api: Fix multi_delete with object names using non-ASCII characters Co-Authored-By: Florent Vennetier Change-Id: I635bc91faa7709f9df9cdf3aec157a21c08923ca --- .../s3api/controllers/multi_delete.py | 3 +- test/functional/s3api/test_multi_delete.py | 46 +++++++++++++++---- .../middleware/s3api/test_multi_delete.py | 13 +++++- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/swift/common/middleware/s3api/controllers/multi_delete.py b/swift/common/middleware/s3api/controllers/multi_delete.py index 49a51f663c..ddc45c0a2e 100644 --- a/swift/common/middleware/s3api/controllers/multi_delete.py +++ b/swift/common/middleware/s3api/controllers/multi_delete.py @@ -18,6 +18,7 @@ import json from swift.common.constraints import MAX_OBJECT_NAME_LENGTH from swift.common.http import HTTP_NO_CONTENT +from swift.common.swob import str_to_wsgi from swift.common.utils import public, StreamingPile from swift.common.registry import get_swift_info @@ -113,7 +114,7 @@ class MultiObjectDeleteController(Controller): def do_delete(base_req, key, version): req = copy.copy(base_req) req.environ = copy.copy(base_req.environ) - req.object_name = key + req.object_name = str_to_wsgi(key) if version: req.params = {'version-id': version, 'symlink': 'get'} diff --git a/test/functional/s3api/test_multi_delete.py b/test/functional/s3api/test_multi_delete.py index 534f0ea191..ac15927d66 100644 --- a/test/functional/s3api/test_multi_delete.py +++ b/test/functional/s3api/test_multi_delete.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import six import unittest import os import test.functional as tf @@ -56,14 +57,17 @@ class TestS3ApiMultiDelete(S3ApiBase): return tostring(elem, use_s3ns=False) - def test_delete_multi_objects(self): + def _test_delete_multi_objects(self, with_non_ascii=False): bucket = 'bucket' - put_objects = ['obj%s' % var for var in range(4)] + if with_non_ascii: + put_objects = [u'\N{SNOWMAN}obj%s' % var for var in range(4)] + else: + put_objects = ['obj%s' % var for var in range(4)] self._prepare_test_delete_multi_objects(bucket, put_objects) query = 'delete' # Delete an object via MultiDelete API - req_objects = ['obj0'] + req_objects = put_objects[:1] xml = self._gen_multi_delete_xml(req_objects) content_md5 = calculate_md5(xml) status, headers, body = \ @@ -78,10 +82,13 @@ class TestS3ApiMultiDelete(S3ApiBase): resp_objects = elem.findall('Deleted') self.assertEqual(len(resp_objects), len(req_objects)) for o in resp_objects: - self.assertTrue(o.find('Key').text in req_objects) + key = o.find('Key').text + if six.PY2: + key = key.decode('utf-8') + self.assertTrue(key in req_objects) # Delete 2 objects via MultiDelete API - req_objects = ['obj1', 'obj2'] + req_objects = put_objects[1:3] xml = self._gen_multi_delete_xml(req_objects) content_md5 = calculate_md5(xml) status, headers, body = \ @@ -93,10 +100,17 @@ class TestS3ApiMultiDelete(S3ApiBase): resp_objects = elem.findall('Deleted') self.assertEqual(len(resp_objects), len(req_objects)) for o in resp_objects: - self.assertTrue(o.find('Key').text in req_objects) + key = o.find('Key').text + if six.PY2: + key = key.decode('utf-8') + self.assertTrue(key in req_objects) + if with_non_ascii: + fake_objs = [u'\N{SNOWMAN}obj%s' % var for var in range(4, 6)] + else: + fake_objs = ['obj%s' % var for var in range(4, 6)] # Delete 2 objects via MultiDelete API but one (obj4) doesn't exist. - req_objects = ['obj3', 'obj4'] + req_objects = [put_objects[-1], fake_objs[0]] xml = self._gen_multi_delete_xml(req_objects) content_md5 = calculate_md5(xml) status, headers, body = \ @@ -109,10 +123,13 @@ class TestS3ApiMultiDelete(S3ApiBase): # S3 assumes a NoSuchKey object as deleted. self.assertEqual(len(resp_objects), len(req_objects)) for o in resp_objects: - self.assertTrue(o.find('Key').text in req_objects) + key = o.find('Key').text + if six.PY2: + key = key.decode('utf-8') + self.assertTrue(key in req_objects) # Delete 2 objects via MultiDelete API but no objects exist - req_objects = ['obj4', 'obj5'] + req_objects = fake_objs[:2] xml = self._gen_multi_delete_xml(req_objects) content_md5 = calculate_md5(xml) status, headers, body = \ @@ -124,7 +141,16 @@ class TestS3ApiMultiDelete(S3ApiBase): resp_objects = elem.findall('Deleted') self.assertEqual(len(resp_objects), len(req_objects)) for o in resp_objects: - self.assertTrue(o.find('Key').text in req_objects) + key = o.find('Key').text + if six.PY2: + key = key.decode('utf-8') + self.assertTrue(key in req_objects) + + def test_delete_multi_objects(self): + self._test_delete_multi_objects() + + def test_delete_multi_objects_with_non_ascii(self): + self._test_delete_multi_objects(with_non_ascii=True) def test_delete_multi_objects_error(self): bucket = 'bucket' diff --git a/test/unit/common/middleware/s3api/test_multi_delete.py b/test/unit/common/middleware/s3api/test_multi_delete.py index 44af595270..3be972d634 100644 --- a/test/unit/common/middleware/s3api/test_multi_delete.py +++ b/test/unit/common/middleware/s3api/test_multi_delete.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # Copyright (c) 2014 OpenStack Foundation # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -39,6 +40,9 @@ class TestS3ApiMultiDelete(S3ApiTestCase): swob.HTTPOk, {}, None) self.swift.register('HEAD', '/v1/AUTH_test/bucket/Key2', swob.HTTPNotFound, {}, None) + self.swift.register('HEAD', + '/v1/AUTH_test/bucket/business/caf\xc3\xa9', + swob.HTTPOk, {}, None) self.ts = make_timestamp_iter() @s3acl @@ -70,6 +74,9 @@ class TestS3ApiMultiDelete(S3ApiTestCase): swob.HTTPOk, {'x-static-large-object': 'True'}, None) + self.swift.register('DELETE', + '/v1/AUTH_test/bucket/business/caf\xc3\xa9', + swob.HTTPNoContent, {}, None) slo_delete_resp = { 'Number Not Found': 0, 'Response Status': '200 OK', @@ -88,7 +95,7 @@ class TestS3ApiMultiDelete(S3ApiTestCase): swob.HTTPNoContent, {}, None) elem = Element('Delete') - for key in ['Key1', 'Key2', 'Key3', 'Key4']: + for key in ['Key1', 'Key2', 'Key3', 'Key4', 'business/café']: obj = SubElement(elem, 'Object') SubElement(obj, 'Key').text = key body = tostring(elem, use_s3ns=False) @@ -106,7 +113,7 @@ class TestS3ApiMultiDelete(S3ApiTestCase): self.assertEqual(status.split()[0], '200') elem = fromstring(body) - self.assertEqual(len(elem.findall('Deleted')), 4) + self.assertEqual(len(elem.findall('Deleted')), 5) self.assertEqual(len(elem.findall('Error')), 0) self.assertEqual(self.swift.calls, [ ('HEAD', '/v1/AUTH_test/bucket'), @@ -119,6 +126,8 @@ class TestS3ApiMultiDelete(S3ApiTestCase): ('HEAD', '/v1/AUTH_test/bucket/Key4?symlink=get'), ('DELETE', '/v1/AUTH_test/bucket/Key4?async=on&multipart-manifest=delete'), + ('HEAD', '/v1/AUTH_test/bucket/business/caf\xc3\xa9?symlink=get'), + ('DELETE', '/v1/AUTH_test/bucket/business/caf\xc3\xa9'), ]) @s3acl