From 3b4cd1c27bb2fa0e06ba9d84be70b67e1c4f0198 Mon Sep 17 00:00:00 2001
From: Jesse Vogt <jesse.vogt@gmail.com>
Date: Tue, 24 Sep 2019 17:07:58 -0500
Subject: [PATCH] switch from calling clean in loop to undoing clean in
 delete_keys

---
 moto/s3/responses.py           |  4 ++--
 moto/s3/utils.py               | 24 +++++++++---------------
 tests/test_s3/test_s3_utils.py | 20 +++++++++++++++++---
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/moto/s3/responses.py b/moto/s3/responses.py
index 4c546c595..61ebff9d0 100644
--- a/moto/s3/responses.py
+++ b/moto/s3/responses.py
@@ -20,7 +20,7 @@ from .exceptions import BucketAlreadyExists, S3ClientError, MissingBucket, Missi
     MalformedACLError, InvalidNotificationARN, InvalidNotificationEvent, ObjectNotInActiveTierError
 from .models import s3_backend, get_canned_acl, FakeGrantee, FakeGrant, FakeAcl, FakeKey, FakeTagging, FakeTagSet, \
     FakeTag
-from .utils import bucket_name_from_url, clean_key_name, metadata_from_headers, parse_region_from_url
+from .utils import bucket_name_from_url, clean_key_name, undo_clean_key_name, metadata_from_headers, parse_region_from_url
 from xml.dom import minidom
 
 
@@ -711,7 +711,7 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin):
 
         for k in keys:
             key_name = k.firstChild.nodeValue
-            success = self.backend.delete_key(bucket_name, key_name)
+            success = self.backend.delete_key(bucket_name, undo_clean_key_name(key_name))
             if success:
                 deleted_names.append(key_name)
             else:
diff --git a/moto/s3/utils.py b/moto/s3/utils.py
index 811e44f71..3bdd24cc4 100644
--- a/moto/s3/utils.py
+++ b/moto/s3/utils.py
@@ -5,7 +5,7 @@ import os
 from boto.s3.key import Key
 import re
 import six
-from six.moves.urllib.parse import urlparse, unquote
+from six.moves.urllib.parse import urlparse, unquote, quote
 import sys
 
 
@@ -68,22 +68,16 @@ def metadata_from_headers(headers):
     return metadata
 
 
-def clean_key_name(key_name, attempts=4):
+def clean_key_name(key_name):
     if six.PY2:
-        def uq(k):
-            return unquote(k.encode('utf-8')).decode('utf-8')
-    else:
-        uq = unquote
+        return unquote(key_name.encode('utf-8')).decode('utf-8')
+    return unquote(key_name)
 
-    original = cleaned = key_name
-    last_attempt = attempts - 1
-    for attempt in range(attempts):
-        cleaned = uq(key_name)
-        if cleaned == key_name:
-            return cleaned
-        if attempt != last_attempt:
-            key_name = cleaned
-    raise Exception('unable to fully clean name: original %s, last clean %s prior clean %s' % (original, cleaned, key_name))
+
+def undo_clean_key_name(key_name):
+    if six.PY2:
+        return quote(key_name.encode('utf-8')).decode('utf-8')
+    return quote(key_name)
 
 
 class _VersionedKeyStore(dict):
diff --git a/tests/test_s3/test_s3_utils.py b/tests/test_s3/test_s3_utils.py
index d55b28b6d..93a4683e6 100644
--- a/tests/test_s3/test_s3_utils.py
+++ b/tests/test_s3/test_s3_utils.py
@@ -1,7 +1,7 @@
 from __future__ import unicode_literals
 import os
 from sure import expect
-from moto.s3.utils import bucket_name_from_url, _VersionedKeyStore, parse_region_from_url, clean_key_name
+from moto.s3.utils import bucket_name_from_url, _VersionedKeyStore, parse_region_from_url, clean_key_name, undo_clean_key_name
 from parameterized import parameterized
 
 
@@ -87,7 +87,21 @@ def test_parse_region_from_url():
     ('foo',
      'foo'),
     ('foo/run_dt%3D2019-01-01%252012%253A30%253A00',
-     'foo/run_dt=2019-01-01 12:30:00'),
+     'foo/run_dt=2019-01-01%2012%3A30%3A00'),
 ])
 def test_clean_key_name(key, expected):
-    clean_key_name(key).should.equal(expected) 
+    clean_key_name(key).should.equal(expected)
+
+
+@parameterized([
+    ('foo/bar/baz',
+     'foo/bar/baz'),
+    ('foo',
+     'foo'),
+    ('foo/run_dt%3D2019-01-01%252012%253A30%253A00',
+     'foo/run_dt%253D2019-01-01%25252012%25253A30%25253A00'),
+])
+def test_undo_clean_key_name(key, expected):
+    undo_clean_key_name(key).should.equal(expected)
+
+