Fix handling of non-ASCII accounts

Related-Change: I4ecfae2bca6ffa08ad15e584579ebce707f4628d
Related-Change: I1e244c231753b8f4b6f1cf95cb0ae4c3c959ae0f
Change-Id: Ia386736b9b283858931794690538871b6e1ad9c8

swift/common/middleware/tempauth.py

@@ -804,24 +804,23 @@ class TempAuth(object):
                 key = req.headers.get('x-storage-pass')
         else:
             return HTTPBadRequest(request=req)
+        unauthed_headers = {
+            'Www-Authenticate': 'Swift realm="%s"' % (account or 'unknown'),
+        }
         if not all((account, user, key)):
             self.logger.increment('token_denied')
-            realm = account or 'unknown'
+            return HTTPUnauthorized(request=req, headers=unauthed_headers)
-            return HTTPUnauthorized(request=req, headers={'Www-Authenticate':
-                                                          'Swift realm="%s"' %
-                                                          realm})
         # Authenticate user
+        account = wsgi_to_str(account)
+        user = wsgi_to_str(user)
+        key = wsgi_to_str(key)
         account_user = account + ':' + user
         if account_user not in self.users:
             self.logger.increment('token_denied')
-            auth = 'Swift realm="%s"' % account
+            return HTTPUnauthorized(request=req, headers=unauthed_headers)
-            return HTTPUnauthorized(request=req,
-                                    headers={'Www-Authenticate': auth})
         if self.users[account_user]['key'] != key:
             self.logger.increment('token_denied')
-            auth = 'Swift realm="unknown"'
+            return HTTPUnauthorized(request=req, headers=unauthed_headers)
-            return HTTPUnauthorized(request=req,
-                                    headers={'Www-Authenticate': auth})
         account_id = self.users[account_user]['url'].rsplit('/', 1)[-1]
         # Get memcache client
         memcache_client = cache_from_env(req.environ)

swift/proxy/controllers/base.py

@@ -2152,7 +2152,7 @@ class Controller(object):
         headers.update((k, v)
                        for k, v in req.headers.items()
                        if is_sys_meta('account', k))
-        resp = self.make_requests(Request.blank('/v1' + path),
+        resp = self.make_requests(Request.blank(str_to_wsgi('/v1' + path)),
                                   self.app.account_ring, partition, 'PUT',
                                   path, [headers] * len(nodes))
         if is_success(resp.status_int):

swift/proxy/controllers/container.py

@@ -433,7 +433,7 @@ class ContainerController(Controller):
             set_info_cache(self.app, req.environ, self.account_name,
                            self.container_name, resp)
         if 'swift.authorize' in req.environ:
-            req.acl = resp.headers.get('x-container-read')
+            req.acl = wsgi_to_str(resp.headers.get('x-container-read'))
             aresp = req.environ['swift.authorize'](req)
             if aresp:
                 # Don't cache this. It doesn't reflect the state of the

test/functional/test_dlo.py

@@ -14,6 +14,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from six.moves import urllib
+
 from swift.common.swob import str_to_wsgi
 import test.functional as tf
 from test.functional.tests import Utils, Base, Base2, BaseEnv
@@ -157,7 +159,7 @@ class TestDlo(Base):
 
     def test_copy_account(self):
         # dlo use same account and same container only
-        acct = self.env.conn.account_name
+        acct = urllib.parse.unquote(self.env.conn.account_name)
         # Adding a new segment, copying the manifest, and then deleting the
         # segment proves that the new object is really the concatenated
         # segments and not just a manifest.

test/functional/test_domain_remap.py

@@ -34,6 +34,7 @@ def tearDownModule():
 
 class TestDomainRemapEnv(BaseEnv):
     domain_remap_enabled = None  # tri-state: None initially, then True/False
+    dns_safe_account_name = None  # tri-state: None initially, then True/False
 
     @classmethod
     def setUp(cls):
@@ -45,6 +46,11 @@ class TestDomainRemapEnv(BaseEnv):
             if not cls.domain_remap_enabled:
                 return
 
+        if cls.dns_safe_account_name is None:
+            cls.dns_safe_account_name = ('%' not in cls.conn.account_name)
+            if not cls.dns_safe_account_name:
+                return
+
         cls.account = Account(
             cls.conn, tf.config.get('account', tf.config['username']))
         cls.account.delete_containers()
@@ -73,6 +79,14 @@ class TestDomainRemap(Base):
             raise Exception(
                 "Expected domain_remap_enabled to be True/False, got %r" %
                 (self.env.domain_remap_enabled,))
+        if self.env.dns_safe_account_name is False:
+            raise SkipTest("Account name %r cannot work with Domain Remap" %
+                           (self.env.conn.account_name,))
+        elif self.env.dns_safe_account_name is not True:
+            # just some sanity checking
+            raise Exception(
+                "Expected domain_remap_enabled to be True/False, got %r" %
+                (self.env.domain_remap_enabled,))
         # domain_remap middleware does not advertise its storage_domain values
         # in swift /info responses so a storage_domain must be configured in
         # test.conf for these tests to succeed

test/functional/test_object_versioning.py

@@ -190,7 +190,8 @@ class TestObjectVersioning(TestObjectVersioningBase):
     @property
     def account_name(self):
         if not self._account_name:
-            self._account_name = self.env.conn.storage_path.rsplit('/', 1)[-1]
+            self._account_name = unquote(
+                self.env.conn.storage_path.rsplit('/', 1)[-1])
         return self._account_name
 
     def test_disable_version(self):

test/functional/test_slo.py

@@ -22,6 +22,7 @@ from copy import deepcopy
 from unittest import SkipTest
 
 import six
+from six.moves import urllib
 
 from swift.common.swob import normalize_etag
 from swift.common.utils import md5
@@ -709,7 +710,7 @@ class TestSlo(Base):
         self.assertEqual(4 * 1024 * 1024 + 1, len(copied_contents))
 
     def test_slo_copy_account(self):
-        acct = self.env.conn.account_name
+        acct = urllib.parse.unquote(self.env.conn.account_name)
         # same account copy
         file_item = self.env.container.file("manifest-abcde")
         file_item.copy_account(acct, self.env.container.name, "copied-abcde")
@@ -720,7 +721,7 @@ class TestSlo(Base):
 
         if not tf.skip2:
             # copy to different account
-            acct = self.env.conn2.account_name
+            acct = urllib.parse.unquote(self.env.conn2.account_name)
             dest_cont = self.env.account2.container(Utils.create_name())
             self.assertTrue(dest_cont.create(hdrs={
                 'X-Container-Write': self.env.conn.user_acl
@@ -871,7 +872,7 @@ class TestSlo(Base):
     def test_slo_copy_the_manifest_account(self):
         if tf.skip2:
             raise SkipTest('Account2 not set')
-        acct = self.env.conn.account_name
+        acct = urllib.parse.unquote(self.env.conn.account_name)
         # same account
         file_item = self.env.container.file("manifest-abcde")
         file_item.copy_account(acct,
@@ -887,7 +888,7 @@ class TestSlo(Base):
             self.fail("COPY didn't copy the manifest (invalid json on GET)")
 
         # different account
-        acct = self.env.conn2.account_name
+        acct = urllib.parse.unquote(self.env.conn2.account_name)
         dest_cont = self.env.account2.container(Utils.create_name())
         self.assertTrue(dest_cont.create(hdrs={
             'X-Container-Write': self.env.conn.user_acl

test/unit/common/middleware/test_tempauth.py

@@ -23,7 +23,7 @@ import six
 from six.moves.urllib.parse import quote, urlparse
 from swift.common.middleware import tempauth as auth
 from swift.common.middleware.acl import format_acl
-from swift.common.swob import Request, Response
+from swift.common.swob import Request, Response, bytes_to_wsgi
 from swift.common.utils import split_path, StatsdClient
 from test.unit import FakeMemcache
 
@@ -1009,10 +1009,11 @@ class TestAuth(unittest.TestCase):
         quoted_acct = quote(u'/v1/AUTH_t\u00e9st'.encode('utf8'))
         memcache = FakeMemcache()
 
+        wsgi_user = bytes_to_wsgi(u't\u00e9st:t\u00e9ster'.encode('utf8'))
         req = self._make_request(
             '/auth/v1.0',
-            headers={'X-Auth-User': u't\u00e9st:t\u00e9ster',
+            headers={'X-Auth-User': wsgi_user,
-                     'X-Auth-Key': u'p\u00e1ss'})
+                     'X-Auth-Key': bytes_to_wsgi(u'p\u00e1ss'.encode('utf8'))})
         req.environ['swift.cache'] = memcache
         resp = req.get_response(ath)
         self.assertEqual(resp.status_int, 200)
@@ -1022,8 +1023,8 @@ class TestAuth(unittest.TestCase):
 
         req = self._make_request(
             '/auth/v1.0',
-            headers={'X-Auth-User': u't\u00e9st:t\u00e9ster',
+            headers={'X-Auth-User': wsgi_user,
-                     'X-Auth-Key': u'p\u00e1ss'})
+                     'X-Auth-Key': bytes_to_wsgi(u'p\u00e1ss'.encode('utf8'))})
         req.environ['swift.cache'] = memcache
         resp = req.get_response(ath)
         self.assertEqual(resp.status_int, 200)

test/unit/proxy/test_server.py

@@ -10045,6 +10045,38 @@ class TestContainerController(unittest.TestCase):
                                   call['method'], key, call['headers']))
                 self.assertEqual(value, call['headers'][key])
 
+    def test_PUT_autocreate_account_utf8(self):
+        with save_globals():
+            controller = proxy_server.ContainerController(
+                self.app, wsgi_to_str('\xe2\x98\x83'),
+                wsgi_to_str('\xe2\x98\x83'))
+
+            def test_status_map(statuses, expected, headers=None, **kwargs):
+                set_http_connect(*statuses, **kwargs)
+                req = Request.blank('/v1/a/c', {}, headers=headers)
+                req.content_length = 0
+                self.app.update_request(req)
+                res = controller.PUT(req)
+                expected = str(expected)
+                self.assertEqual(res.status[:len(expected)], expected)
+
+            self.app.account_autocreate = True
+            calls = []
+            callback = _make_callback_func(calls)
+
+            # all goes according to plan
+            test_status_map(
+                (404, 404, 404,   # account_info fails on 404
+                 201, 201, 201,   # PUT account
+                 200,             # account_info success
+                 201, 201, 201),  # put container success
+                201, missing_container=True,
+                give_connect=callback)
+
+            self.assertEqual(10, len(calls))
+            for call in calls[3:6]:
+                self.assertEqual(wsgi_to_str('/\xe2\x98\x83'), call['path'])
+
     def test_POST(self):
         with save_globals():
             controller = proxy_server.ContainerController(self.app, 'account',
@@ -11581,6 +11613,15 @@ class TestAccountControllerFakeGetResponse(unittest.TestCase):
             resp = req.get_response(self.app)
             self.assertEqual(400, resp.status_int)
 
+    def test_GET_autocreate_utf8(self):
+        with save_globals():
+            set_http_connect(*([404] * 100))  # nonexistent: all backends 404
+            req = Request.blank('/v1/\xe2\x98\x83',
+                                environ={'REQUEST_METHOD': 'GET',
+                                         'PATH_INFO': '/v1/\xe2\x98\x83'})
+            resp = req.get_response(self.app)
+            self.assertEqual(204, resp.status_int)
+
     def test_account_acl_header_access(self):
         acl = {
             'admin': ['AUTH_alice'],