)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"7969d2c3b7aad34ee42f05b63e2b96fb463ab341","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"32d5d0c1_449e3775","updated":"2024-03-20 00:22:03.000000000","message":"Do we need tests ?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f5ca95d56b7d7feb3e1a6c307422985fec0b0db9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7729ce05_09dbf9e6","updated":"2024-03-20 02:13:06.000000000","message":"I have no doubt that this is an issue and is and will cause issues. (darn another str_to_wsgi call). It would be nice to throw in a quick test showing the unicode traceback.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"03df37ab64408d0fce13247412aafbe8ef2ad3dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f727bb1b_2de8aaaa","updated":"2024-03-20 14:00:55.000000000","message":"Maybe the real question is why didn\u0027t the existing test already protect us from a traceback?\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/common/middleware/s3api/test_s3api.py#L707","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9975a5680a3683e86144013887ebdbcd95085e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"13f8f00b_37bdc6f8","updated":"2024-03-20 16:11:29.000000000","message":"i\u0027m trying to dig into this and better understand what \"correct\" handling of the sw_req.environ should look like and it\u0027s only making me frustrated.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dbd92cb7805e7c0d0e7a9ccd08205abe68c01432","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"309a6776_99eb6332","updated":"2024-03-20 14:31:12.000000000","message":"ok, i revert Tim\u0027s change in s3request \n\n```\npytest swift/test/unit/common/middleware/s3api/test_s3api.py::TestS3ApiMiddleware::test_non_ascii_user\n```\n\nFAILS\n\n```\nTraceback (most recent call last):\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3api.py\", line 349, in __call__\n    resp \u003d self.handle_request(req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3api.py\", line 389, in handle_request\n    res \u003d handler(req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/controllers/base.py\", line 60, in wrapped\n    return func(self, req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/controllers/base.py\", line 71, in check_container\n    req.get_container_info(self.app)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3request.py\", line 1556, in get_container_info\n    sw_resp \u003d sw_req.get_response(app)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 1165, in get_response\n    status, headers, app_iter \u003d self.call_application(application)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 1149, in call_application\n    app_iter \u003d application(self.environ, start_response)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3api.py\", line 182, in __call__\n    return self.app(env, start_response)\n  File \"/home/vagrant/swift/test/unit/common/middleware/s3api/__init__.py\", line 83, in __call__\n    self.handle(env)\n  File \"/home/vagrant/swift/test/unit/common/middleware/s3api/__init__.py\", line 67, in handle\n    self._update_s3_path_info(env)\n  File \"/home/vagrant/swift/test/unit/common/middleware/s3api/__init__.py\", line 54, in _update_s3_path_info\n    swob.wsgi_to_str(path)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 292, in wsgi_to_str\n    return wsgi_to_bytes(wsgi_str).decode(\u0027utf8\u0027, errors\u003d\u0027surrogateescape\u0027)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 284, in wsgi_to_bytes\n    return wsgi_str.encode(\u0027latin1\u0027)\nUnicodeEncodeError: \u0027latin-1\u0027 codec can\u0027t encode character \u0027\\u2603\u0027 in position 9: ordinal not in range(256)\n```\n\n^ with a traceback that looks *kind of* like prod\n\nindicating the problem is our \"fake auth\" solution in `s3api/__init__.py` wasn\u0027t properly emulating the contract that s3api actually expects auth mw to perform?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e8c6811a_376a1295","updated":"2024-03-22 14:15:34.000000000","message":"I think as a side-effect this makes unicode tempauth users not work with s3api anymore (even with valid creds you get rejected).  But it fixes a traceback in prod (where our auth system doesn\u0027t even *support* non-ascii usernames) so I think it\u0027s a net win - hopefully no other mw besides tempauth were counting on this \"native string in the PATH_INFO\" behavior because we can\u0027t let it continue - it\u0027s a major footgun everywhere in the pipeline - compatibility be damned.\n\nI filed a bug which this changes fixes:\n\nhttps://bugs.launchpad.net/swift/+bug/2058748\n\nI updated the commit message.\n\nI think we can knock around ideas on how to proceed in a follow-on if we have the stamina for it:\n\n913977: s3api: replace_s3api_access_key_in_path_with_account | https://review.opendev.org/c/openstack/swift/+/913977","commit_id":"fdaabf9a3d77a2772f48944555bb6f2d3bb0bd88"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9026504a0b59aa7689ee605f398037450d149b2e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4f2efa72_6f441f81","updated":"2024-03-22 15:58:34.000000000","message":"Thanks for writing up the bug.","commit_id":"fdaabf9a3d77a2772f48944555bb6f2d3bb0bd88"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9026504a0b59aa7689ee605f398037450d149b2e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fd3560c2_e1c9610c","in_reply_to":"e8c6811a_376a1295","updated":"2024-03-22 15:58:34.000000000","message":"\u003e I think as a side-effect this makes unicode tempauth users not work with s3api anymore (even with valid creds you get rejected).\n\nGood reason to -1.","commit_id":"fdaabf9a3d77a2772f48944555bb6f2d3bb0bd88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f55116d2_c1a6e642","updated":"2024-03-25 20:06:18.000000000","message":"the s3api change is necessary to prevent the 5XX on unicode access_key with invalid signatures.\n\nThe corresponding tempauth change to match the new s3api PATH_INFO format works correctly and allows unicode tempauth users to continue to work with s3api.\n\nThe s3token change looks correct.\n\nI don\u0027t find the new tempauth/s3token unittests particuarlly inspiring... although they\u0027re better than the nothing we have.  The problem is they just sort of half-emulate what they think a \"real\" S3Request should do.\n\nI think it would be better to have a well tested s3api-auth *contract* so that mw can assume if they use the helpers we provide they\u0027re unlikely to have these kind of bugs (or if they do; we\u0027ll fix them in an upcoming swift release and maintain the existing interface), i.e.\n\n913977: s3api: replace_s3api_access_key_in_path_with_account | https://review.opendev.org/c/openstack/swift/+/913977\n\n... but I understand that approach has some different trade-offs and limitations.","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"}],"swift/common/middleware/s3api/s3request.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9975a5680a3683e86144013887ebdbcd95085e7","unresolved":true,"context_lines":[{"line_number":1144,"context_line":"        if self.account is None:"},{"line_number":1145,"context_line":"            account \u003d swob.str_to_wsgi(self.access_key)"},{"line_number":1146,"context_line":"        else:"},{"line_number":1147,"context_line":"            account \u003d self.account"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        env \u003d self.environ.copy()"},{"line_number":1150,"context_line":"        env[\u0027swift.infocache\u0027] \u003d self.environ.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"483a639d_ae9d1473","line":1147,"updated":"2024-03-20 16:11:29.000000000","message":"why do we get the \"type\" right on self.account but self.access_key needs special treatment?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af1a095ec4c98c8212458be38e10637f0f558690","unresolved":true,"context_lines":[{"line_number":1144,"context_line":"        if self.account is None:"},{"line_number":1145,"context_line":"            account \u003d swob.str_to_wsgi(self.access_key)"},{"line_number":1146,"context_line":"        else:"},{"line_number":1147,"context_line":"            account \u003d self.account"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        env \u003d self.environ.copy()"},{"line_number":1150,"context_line":"        env[\u0027swift.infocache\u0027] \u003d self.environ.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"e457558c_5ba4620f","line":1147,"in_reply_to":"483a639d_ae9d1473","updated":"2024-03-20 16:22:16.000000000","message":"Every time we set `self.account`, it\u0027s coming out of the environment, with no `wsgi_to_str` treatment.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"        if self.account is None:"},{"line_number":1145,"context_line":"            account \u003d swob.str_to_wsgi(self.access_key)"},{"line_number":1146,"context_line":"        else:"},{"line_number":1147,"context_line":"            account \u003d self.account"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        env \u003d self.environ.copy()"},{"line_number":1150,"context_line":"        env[\u0027swift.infocache\u0027] \u003d self.environ.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"efd47ad9_6fd72ae3","line":1147,"in_reply_to":"e457558c_5ba4620f","updated":"2024-03-22 14:15:34.000000000","message":"ok, I couldn\u0027t actually convince myself I\u0027d tracked down all the places we set self.account with a simple `grep \"self.account \u003d \" swift/common/middleware/s3api`\n\nI think I can see that now:\n\n```\n(vagrant-swift-all-in-one) clayg@banana:~/Workspace/vagrant-swift-all-in-one/swift$ for f in $(find swift/common/middleware/s3api/ -name \\*.py); do echo $f; grep -R -e \"self.account.*\u003d \" $f; done\nswift/common/middleware/s3api/etree.py\nswift/common/middleware/s3api/subresource.py\nswift/common/middleware/s3api/__init__.py\nswift/common/middleware/s3api/s3token.py\nswift/common/middleware/s3api/s3api.py\nswift/common/middleware/s3api/controllers/logging.py\nswift/common/middleware/s3api/controllers/tagging.py\nswift/common/middleware/s3api/controllers/__init__.py\nswift/common/middleware/s3api/controllers/acl.py\nswift/common/middleware/s3api/controllers/object_lock.py\nswift/common/middleware/s3api/controllers/multi_delete.py\nswift/common/middleware/s3api/controllers/versioning.py\nswift/common/middleware/s3api/controllers/base.py\nswift/common/middleware/s3api/controllers/service.py\nswift/common/middleware/s3api/controllers/s3_acl.py\nswift/common/middleware/s3api/controllers/multi_upload.py\nswift/common/middleware/s3api/controllers/obj.py\nswift/common/middleware/s3api/controllers/location.py\nswift/common/middleware/s3api/controllers/bucket.py\nswift/common/middleware/s3api/acl_handlers.py\nswift/common/middleware/s3api/s3request.py\n        self.account \u003d None\n            _, self.account, _ \u003d split_path(sw_resp.environ[\u0027PATH_INFO\u0027],\n            _, self.account, _ \u003d split_path(sw_resp.environ[\u0027PATH_INFO\u0027],\n        _, self.account, _ \u003d split_path(sw_resp.environ[\u0027PATH_INFO\u0027],\nswift/common/middleware/s3api/utils.py\nswift/common/middleware/s3api/exception.py\nswift/common/middleware/s3api/s3response.py\nswift/common/middleware/s3api/acl_utils.py\n```","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9975a5680a3683e86144013887ebdbcd95085e7","unresolved":true,"context_lines":[{"line_number":1227,"context_line":"            path \u003d \u0027/v1/%s/%s\u0027 % (account, container)"},{"line_number":1228,"context_line":"        else:"},{"line_number":1229,"context_line":"            path \u003d \u0027/v1/%s\u0027 % (account)"},{"line_number":1230,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        params \u003d []"},{"line_number":1233,"context_line":"        if query is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c14b1f4_5c341f77","line":1230,"updated":"2024-03-20 16:11:29.000000000","message":"wouldn\u0027t it be more clear to turn the full path into a wsgi-str HERE before we push it into the environ?  Aren\u0027t the container and obj vars just as likely to be utf8 decoded unicode?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af1a095ec4c98c8212458be38e10637f0f558690","unresolved":true,"context_lines":[{"line_number":1227,"context_line":"            path \u003d \u0027/v1/%s/%s\u0027 % (account, container)"},{"line_number":1228,"context_line":"        else:"},{"line_number":1229,"context_line":"            path \u003d \u0027/v1/%s\u0027 % (account)"},{"line_number":1230,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        params \u003d []"},{"line_number":1233,"context_line":"        if query is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fbdd75cb_b06fcf8e","line":1230,"in_reply_to":"3c14b1f4_5c341f77","updated":"2024-03-20 16:22:16.000000000","message":"Everything is already assumed to be WSGI strings.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":true,"context_lines":[{"line_number":1227,"context_line":"            path \u003d \u0027/v1/%s/%s\u0027 % (account, container)"},{"line_number":1228,"context_line":"        else:"},{"line_number":1229,"context_line":"            path \u003d \u0027/v1/%s\u0027 % (account)"},{"line_number":1230,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":"        params \u003d []"},{"line_number":1233,"context_line":"        if query is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"aebd5ccd_49c55df4","line":1230,"in_reply_to":"fbdd75cb_b06fcf8e","updated":"2024-03-22 14:15:34.000000000","message":"I don\u0027t think an assumption is *good* justification not to handle the translation as close as possible to the point where we\u0027re leaving \"normal py3 native string land\" and going into \"wsgi string crazy town\"\n\nBut if we\u0027re carrying around wsgi strings as variables in \"normal py3 native string land\" I guess that explains why trying to str_to_wsgi the whole path here double-encode-decodes everything into a hot mess:\n\n```\n\u003e\u003e\u003e a \u003d \u0027auth☃\u0027\n\u003e\u003e\u003e c \u003d swob.str_to_wsgi(\u0027cont☃\u0027)\n\u003e\u003e\u003e c\n\u0027contâ\\x98\\x83\u0027\n\u003e\u003e\u003e \u0027%s/%s\u0027 % (a, c)\n\u0027auth☃/contâ\\x98\\x83\u0027\n\u003e\u003e\u003e swob.str_to_wsgi(\u0027%s/%s\u0027 % (a, c))\n\u0027authâ\\x98\\x83/contâÂ\\x98Â\\x83\u0027\n```\n\nso \"that would break everything\" is a terrifying, but acceptable reason to ONLY str_to_wsgi on the self.access_key.\n\nHow are mere mortals supposed to know \"this attribute is a normal py3 native string\" and \"this variable is a wsgi string\" when the damn TYPE is the same?  Should we start annotating variables/attributes we don\u0027t wsgi_to_str as `ws_\u003cname\u003e` [1] so that we can dream of day we don\u0027t keep writing these kinds of bugs anymore?\n\nhttps://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"}],"swift/common/middleware/s3api/s3token.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":true,"context_lines":[{"line_number":404,"context_line":"        self._logger.debug(\u0027Connecting with tenant: %s\u0027, tenant_to_connect)"},{"line_number":405,"context_line":"        new_tenant_name \u003d \u0027%s%s\u0027 % (self._reseller_prefix, tenant_to_connect)"},{"line_number":406,"context_line":"        environ[\u0027PATH_INFO\u0027] \u003d environ[\u0027PATH_INFO\u0027].replace("},{"line_number":407,"context_line":"            str_to_wsgi(account), str_to_wsgi(new_tenant_name), 1)"},{"line_number":408,"context_line":"        return self._app(environ, start_response)"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3654093c_f3166e45","line":407,"updated":"2024-03-25 20:06:18.000000000","message":"it appears that account is a real utf-8 decoded string; but the life-cycle is a little hard to follow - it\u0027s original source *is* the req.path\n\nBut swob \"fixes\" req.path:\n\n    \u003e\u003e\u003e swob.Request.blank(\u0027/v1/test☃/c/o\u0027.encode(\u0027utf8\u0027)).path\n    \u0027/v1/test%E2%98%83/c/o\u0027\n    \n... and urllib.parse.unquote seems to (surprisingly?) assumed utf8:\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/common/middleware/s3api/s3token.py#L254\n\n    \u003e\u003e\u003e urllib.parse.unquote(\u0027/v1/test%E2%98%83/c/o\u0027)\n    \u0027/v1/test☃/c/o\u0027\n    \u003e\u003e\u003e urllib.parse.unquote(\u0027/v1/acc%E9sskey/c/o\u0027)\n    \u0027/v1/acc�sskey/c/o\u0027\n    \n... and since tenant comes from either the tenant (json response) or s3api.auth_details data structures it *appears* it\u0027s also always consistenantly a normal utf-8 decoded string.  Certainly we should make it wsgi-string before we sub it into the path (although it would probably also be valid to use `urllib.parse.quote` since we have a normal utf8 decoded string and urllib.parse.quote assumes utf8)","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"}],"swift/common/middleware/tempauth.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":true,"context_lines":[{"line_number":471,"context_line":"            if not s3_auth_details[\u0027check_signature\u0027](user[\u0027key\u0027]):"},{"line_number":472,"context_line":"                return None"},{"line_number":473,"context_line":"            env[\u0027PATH_INFO\u0027] \u003d env[\u0027PATH_INFO\u0027].replace("},{"line_number":474,"context_line":"                str_to_wsgi(account_user), wsgi_unquote(account_id), 1)"},{"line_number":475,"context_line":"            groups \u003d self._get_user_groups(account, account_user, account_id)"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        return groups"}],"source_content_type":"text/x-python","patch_set":3,"id":"f869d23b_f84c451b","line":474,"updated":"2024-03-25 20:06:18.000000000","message":"I\u0027m not sure I fully understand why s3api feels it necessary to quote the wsgi_string \"path\" before it creates a swob.Request.blank:\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/common/middleware/s3api/s3request.py#L1243\n\nI wonder if it was mostly because that\u0027s how tempauth represents it\u0027s user database:\nhttps://github.com/NVIDIA/swift/blob/master/swift/common/middleware/tempauth.py#L261\n\nRegardless as best I can tell the quoted path was (and still is) a totally valid way to encode arbitrary bytes in PATH_INFO, and the fix here \"works\" even if continue to let tempauth continue to sub in it\u0027s quoted account_id into the path:\n\n    AssertionError: \u0027/v1/AUTH_t%C3%A9st\u0027 !\u003d \u0027/v1/AUTH_tést\u0027\n\nFor example, the existing tempauth/swift-auth user tests makes a request with a quoted path from the auth-response/user-db:\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/common/middleware/test_tempauth.py#L1037\n\nBut I guess we don\u0027t actually *test* that request because of normalization in swob.Request.blank:\n\n    \u003e\u003e\u003e Request.blank(\u0027/v1/AUTH_t%C3%A9st\u0027).environ[\u0027PATH_INFO\u0027]\n    \u0027/v1/AUTH_tést\u0027\n\nI think the ambiguity of what our tests say and what our code is actually reacting to is why I went for the \"SocketlessRequestHandler\" style test in my follow-on:\n\nhttps://review.opendev.org/c/openstack/swift/+/913977/1/test/unit/common/middleware/s3api/test_s3request.py\n\nWhich also \"abstracts\" the tempauth behavior to a well-tested and supported helper provided by s3api that \"always gets this right\" so we don\u0027t have to keep fixing these bugs in all s3 aware auth middlewares.\n\nThat is to say, I think it\u0027s actually sort of confusing to add the wsgi_unquote here - it\u0027s kind of an unneeded drive-by change.","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"26761bd7e1d5e6948fef7523b026a86a702b1763","unresolved":true,"context_lines":[{"line_number":471,"context_line":"            if not s3_auth_details[\u0027check_signature\u0027](user[\u0027key\u0027]):"},{"line_number":472,"context_line":"                return None"},{"line_number":473,"context_line":"            env[\u0027PATH_INFO\u0027] \u003d env[\u0027PATH_INFO\u0027].replace("},{"line_number":474,"context_line":"                str_to_wsgi(account_user), wsgi_unquote(account_id), 1)"},{"line_number":475,"context_line":"            groups \u003d self._get_user_groups(account, account_user, account_id)"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        return groups"}],"source_content_type":"text/x-python","patch_set":3,"id":"1870a52a_a0573c99","line":474,"in_reply_to":"f869d23b_f84c451b","updated":"2024-03-26 19:51:42.000000000","message":"\u003e I think it\u0027s actually sort of confusing to add the wsgi_unquote here - it\u0027s kind of an unneeded drive-by change.\n\nEh? No -- drop it out you get the\n```\nAssertionError: \u0027/v1/AUTH_t%C3%A9st\u0027 !\u003d \u0027/v1/AUTH_tést\u0027\n```\nyou mentioned. A `PATH_INFO` like `/v1/AUTH_t%C3%A9st` would mean bytes-on-the-wire looking like `/v1/AUTH_t%25C3%25A9st`, because (from [the CGI spec](https://datatracker.ietf.org/doc/html/draft-coar-cgi-v11-03#section-4.1.5))\n\n\u003e Unlike a URI path, the PATH_INFO is not URL-encoded","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"}],"test/unit/common/middleware/s3api/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"da50e150e2b8ad72521c9bb788b3f62a7759b27b","unresolved":true,"context_lines":[{"line_number":39,"context_line":"        self.remote_user \u003d \u0027authorized\u0027"},{"line_number":40,"context_line":"        self.app \u003d app"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def _update_s3_path_info(self, env):"},{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        For S3 requests, Swift auth middleware replaces a user name in"},{"line_number":45,"context_line":"        env[\u0027PATH_INFO\u0027] with a valid tenant id."}],"source_content_type":"text/x-python","patch_set":1,"id":"e8f4c1b1_dd8a0694","line":42,"updated":"2024-03-20 14:43:54.000000000","message":"maybe this should actually be a helper method in tempauth that gets imported here, so that the tests do exactly what tempauth would do?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":true,"context_lines":[{"line_number":39,"context_line":"        self.remote_user \u003d \u0027authorized\u0027"},{"line_number":40,"context_line":"        self.app \u003d app"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def _update_s3_path_info(self, env):"},{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        For S3 requests, Swift auth middleware replaces a user name in"},{"line_number":45,"context_line":"        env[\u0027PATH_INFO\u0027] with a valid tenant id."}],"source_content_type":"text/x-python","patch_set":1,"id":"fbb77b8e_b3bcf9df","line":42,"in_reply_to":"e8f4c1b1_dd8a0694","updated":"2024-03-22 14:15:34.000000000","message":"great freaking idea!","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dbd92cb7805e7c0d0e7a9ccd08205abe68c01432","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        \u0027/v1/AUTH_test/bucket/object\u0027. This method emulates the behavior."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        tenant_user \u003d swob.str_to_wsgi(env[\u0027s3api.auth_details\u0027][\u0027access_key\u0027])"},{"line_number":50,"context_line":"        tenant, user \u003d tenant_user.rsplit(\u0027:\u0027, 1)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"}],"source_content_type":"text/x-python","patch_set":1,"id":"27ac8b82_27889881","line":50,"updated":"2024-03-20 14:31:12.000000000","message":"if we\u0027re going to continue to assume it\u0027s reasonable for this \"auth_details\" structure in the environ to pass around native-strings - shouldn\u0027t we expect any manipulation (i.e. an rsplit) to operate on native strings?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af1a095ec4c98c8212458be38e10637f0f558690","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        \u0027/v1/AUTH_test/bucket/object\u0027. This method emulates the behavior."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        tenant_user \u003d swob.str_to_wsgi(env[\u0027s3api.auth_details\u0027][\u0027access_key\u0027])"},{"line_number":50,"context_line":"        tenant, user \u003d tenant_user.rsplit(\u0027:\u0027, 1)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"}],"source_content_type":"text/x-python","patch_set":1,"id":"b2280e26_c72a626a","line":50,"in_reply_to":"27ac8b82_27889881","updated":"2024-03-20 16:22:16.000000000","message":"There\u0027s no distinction between a `\u0027:\u0027` that\u0027s a native string vs a `\u0027:\u0027` that\u0027s a WSGI string -- they are exactly the same types and data.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        \u0027/v1/AUTH_test/bucket/object\u0027. This method emulates the behavior."},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        tenant_user \u003d swob.str_to_wsgi(env[\u0027s3api.auth_details\u0027][\u0027access_key\u0027])"},{"line_number":50,"context_line":"        tenant, user \u003d tenant_user.rsplit(\u0027:\u0027, 1)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"}],"source_content_type":"text/x-python","patch_set":1,"id":"9ea08032_d1e51908","line":50,"in_reply_to":"b2280e26_c72a626a","updated":"2024-03-22 14:15:34.000000000","message":"\"it\u0027s the same thing\" doesn\u0027t excuse us from addressing \"it would be more clear\"\n\nI don\u0027t think we should HAVE variables that are wsgi-str - I think the only place latin-1 decoded unicode should exist is in the environ (out-side of `env[\u0027swift.*\u0027]`).  I\u0027m curious how many of our bugs simply come from loosing track of what variable/attribute is a regular python native str (utf8 decoded unicode) or a wsgi str (latin-1 decoded unicode).  It\u0027s the same damn *type* and we don\u0027t have any naming convention for passing around things like `ws_tenant_user` so we should EXPECT (to keep writing/fixing) these kind of bugs.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dbd92cb7805e7c0d0e7a9ccd08205abe68c01432","unresolved":true,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"284ce693_620ff990","line":54,"updated":"2024-03-20 14:31:12.000000000","message":"should we do this *after* we perform the path replacement?  When I remove this line the non-ascii-user test still fails; but in a different spot.\n\n```\nTraceback (most recent call last):\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3api.py\", line 349, in __call__\n    resp \u003d self.handle_request(req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3api.py\", line 389, in handle_request\n    res \u003d handler(req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/controllers/base.py\", line 60, in wrapped\n    return func(self, req)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/controllers/base.py\", line 71, in check_container\n    req.get_container_info(self.app)\n  File \"/home/vagrant/swift/swift/common/middleware/s3api/s3request.py\", line 1565, in get_container_info\n    info \u003d get_container_info(sw_req.environ, app, swift_source\u003d\u0027S3\u0027)\n  File \"/home/vagrant/swift/swift/proxy/controllers/base.py\", line 458, in get_container_info\n    account \u003d wsgi_to_str(wsgi_account)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 292, in wsgi_to_str\n    return wsgi_to_bytes(wsgi_str).decode(\u0027utf8\u0027, errors\u003d\u0027surrogateescape\u0027)\n  File \"/home/vagrant/swift/swift/common/swob.py\", line 284, in wsgi_to_bytes\n    return wsgi_str.encode(\u0027latin1\u0027)\nUnicodeEncodeError: \u0027latin-1\u0027 codec can\u0027t encode character \u0027\\u2603\u0027 in position 5: ordinal not in range(256)\n```","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af1a095ec4c98c8212458be38e10637f0f558690","unresolved":true,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"86da0d51_f1f44128","line":54,"in_reply_to":"284ce693_620ff990","updated":"2024-03-20 16:22:16.000000000","message":"Arguably, we should do both. Doing it *before* checks that s3api has done reasonable things, though, which is what I was mainly interested in. Putting it after has the test code check that the *test code* has done reasonable things.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"87f9682b_875e42d4","line":54,"in_reply_to":"86da0d51_f1f44128","updated":"2024-03-22 14:15:34.000000000","message":"oic, that\u0027s significant!\n\n```\n\u003e\u003e\u003e split_path(\u0027/%s/%s\u0027 % (\u0027auth☃\u0027, swob.str_to_wsgi(\u0027cont☃\u0027)), 1, 2)\n[\u0027auth☃\u0027, \u0027contâ\\x98\\x83\u0027]\n```\nso it\u0027s of course *possible* to smuggle a native string through the wsgi PATH_INFO, but presumably at somepoint someone is going to try wsgi_to_str and get upset:\n\n\n```\n\u003e\u003e\u003e [swob.wsgi_to_str(p) for p in split_path(\u0027/%s/%s\u0027 % (\u0027auth☃\u0027, swob.str_to_wsgi(\u0027cont☃\u0027)), 1, 2)]\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"\u003cstdin\u003e\", line 1, in \u003clistcomp\u003e\n  File \"/vagrant/swift/swift/common/swob.py\", line 292, in wsgi_to_str\n    return wsgi_to_bytes(wsgi_str).decode(\u0027utf8\u0027, errors\u003d\u0027surrogateescape\u0027)\n  File \"/vagrant/swift/swift/common/swob.py\", line 284, in wsgi_to_bytes\n    return wsgi_str.encode(\u0027latin1\u0027)\nUnicodeEncodeError: \u0027latin-1\u0027 codec can\u0027t encode character \u0027\\u2603\u0027 in position 4: ordinal not in range(256)\n```","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dbd92cb7805e7c0d0e7a9ccd08205abe68c01432","unresolved":true,"context_lines":[{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def authorize_cb(req):"}],"source_content_type":"text/x-python","patch_set":1,"id":"a6fa8631_c7dcc3d4","line":55,"updated":"2024-03-20 14:31:12.000000000","message":"to me it looks like the bug is *here* - we manipulate `env[\u0027PATH_INFO\u0027]` (which is supposed to *always* be bytes!?) but \"replace\" a part of the path with a unicode object (tenant came from the \"auth_details\" structure) and then don\u0027t turn it back into bytes before updating `env[\u0027PATH_INFO\u0027]`?","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af1a095ec4c98c8212458be38e10637f0f558690","unresolved":true,"context_lines":[{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def authorize_cb(req):"}],"source_content_type":"text/x-python","patch_set":1,"id":"f9835094_5ff4663c","line":55,"in_reply_to":"a6fa8631_c7dcc3d4","updated":"2024-03-20 16:22:16.000000000","message":"\u003e `env[\u0027PATH_INFO\u0027]` (which is supposed to always be bytes!?)\n\nNope -- it\u0027s supposed to always be [WSGI\u0027s notion of a \"native\" string](https://peps.python.org/pep-3333/#a-note-on-string-types) which comes with a caveat:\n\n\u003e Do not be confused however: even if Python’s `str` type is actually Unicode “under the hood”, the *content* of native strings must still be translatable to bytes via the Latin-1 encoding!\n\nI generally have a contrary view of what should be called a \"native\" string, though, and call these \"WSGI strings\". My mental glossary looks something like this:\n\n* \"byte string\" -- `bytes` on py3, `str` or `bytes` on py2 (where `bytes is str`)\n* \"unicode string\" -- `str` on py3, `unicode` on py2\n* \"native string\" -- `str` on either\n  * on py2, these bytes are typically expected to be UTF-8-encoded\n  * on py3, these can use the range of unicode characters, minus surrogates which should only be used to represent bytes that could not be decoded using UTF-8\n* \"WSGI string\" -- `str` on either, but limited to the Latin-1 range on py3\n\nI find this model a little more useful given the prevalence of UTF-8 as default encoding, not just in Swift but stdlib, too. Everything in [urllib.parse](https://docs.python.org/3/library/urllib.parse.html) defaults to UTF-8; if you open a file in text mode, it\u0027s using UTF-8, so you can typically expect configs (for example) to be (my notion of) native strings. (Actually, `open` [decides based on locale](https://docs.python.org/3/library/locale.html#locale.getencoding), but we *de facto* require that it the locale encoding be UTF-8.)\n\n---\n\n\u003e \"replace\" a part of the path with a unicode object (tenant came from the \"auth_details\" structure)\n\nEh, sure -- I suppose it\u0027d be better if I had `str_to_wsgi(tenant)` here. It doesn\u0027t matter for the test, though, since `str_to_wsgi(\u0027test\u0027) \u003d\u003d \u0027test\u0027`.","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ce176b22207d1a49d44245cdcc9dfecc0782cbe0","unresolved":true,"context_lines":[{"line_number":52,"context_line":"        path \u003d env[\u0027PATH_INFO\u0027]"},{"line_number":53,"context_line":"        # Make sure it\u0027s valid WSGI"},{"line_number":54,"context_line":"        swob.wsgi_to_str(path)"},{"line_number":55,"context_line":"        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @staticmethod"},{"line_number":58,"context_line":"    def authorize_cb(req):"}],"source_content_type":"text/x-python","patch_set":1,"id":"f491f3f9_638b55f9","line":55,"in_reply_to":"f9835094_5ff4663c","updated":"2024-03-22 14:15:34.000000000","message":"oh wait; that makes it sound like the str_to_wsgi when pulling the value out of the auth_details environ structure doesn\u0027t even matter since the test only cares about the left hand side of the `:` - that can\u0027t be right...\n\nok both sides DO matter, we use `path.replace(whole_thing, \u0027AUTH_\u0027 + acct)`\n\nin your change `whole_thing` becomes a wsgi_str (again we\u0027re using wsgi-str variables to do wsgi-string manipulation even tho we\u0027re in py3 native string land and it would be way more consistent for our vars to just be native strings) i.e.\n\nif this was the ONLY change to master:\n\n```\ndiff --git a/test/unit/common/middleware/s3api/__init__.py b/test/unit/common/middleware/s3api/__init__.py\nindex d34d7e83b..7fab51bc8 100644\n--- a/test/unit/common/middleware/s3api/__init__.py\n+++ b/test/unit/common/middleware/s3api/__init__.py\n@@ -49,8 +49,9 @@ class FakeAuthApp(object):\n         tenant_user \u003d env[\u0027s3api.auth_details\u0027][\u0027access_key\u0027]\n         tenant, user \u003d tenant_user.rsplit(\u0027:\u0027, 1)\n \n-        path \u003d env[\u0027PATH_INFO\u0027]\n-        env[\u0027PATH_INFO\u0027] \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)\n+        path \u003d swob.wsgi_to_str(env[\u0027PATH_INFO\u0027])\n+        fixed_path \u003d path.replace(tenant_user, \u0027AUTH_\u0027 + tenant)\n+        env[\u0027PATH_INFO\u0027] \u003d swob.str_to_wsgi(fixed_path)\n \n     @staticmethod\n     def authorize_cb(req):\n```\n\nour *existing* test would fail.  N.B. we wsgi_to_str coming out of the environment, and then str_to_wsgi when we put it back in.  Adopting this pattern consistently and avoiding wsgi-str variables would cause code that doesn\u0027t wsgi_to_str when coming out of the environment and str_to_wsgi when putting back in to start to smell!\n\nIf we\u0027re worried about efficiency (encoding and decoding all over the place) we could maybe someday say \"WSGI be damned\" and change the rules in SwiftHttpProtocol to override get_environ except I don\u0027t know how we handle legacy middlewares... since we put callbacks in the environ sometimes, so there\u0027s no clear \"entry\" and \"exit\" from a given middleware for a Latin1ContextWrapper.\n\nSo encode/decode all over the place is probably the best plan.  Let wsgi be wsgi - but let swift be regular py3 uniocde native strings!  No latin-1 decoded unicode strings in variables or attributes!  Only the environ!  Except `env[\u0027swift.*\u0027]` (he\u0027s cool).","commit_id":"b37c858f5f8011145c85ff185483024e68a89715"}],"test/unit/common/middleware/s3api/test_s3token.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":true,"context_lines":[{"line_number":503,"context_line":"        self.assertEqual(req.environ[\u0027PATH_INFO\u0027], \u0027/v1/AUTH_TENANT_ID/c/o\u0027)"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    def test_authorize_with_unicode_access_key(self):"},{"line_number":506,"context_line":"        req \u003d Request.blank(\u0027/v1/acc\\xc3\\xa9sskey/c/o\u0027)"},{"line_number":507,"context_line":"        req.environ[\u0027s3api.auth_details\u0027] \u003d {"},{"line_number":508,"context_line":"            \u0027access_key\u0027: u\u0027acc\\u00e9ss\u0027,"},{"line_number":509,"context_line":"            \u0027signature\u0027: u\u0027signature\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7bcff777_2d893fbb","line":506,"updated":"2024-03-25 20:06:18.000000000","message":"I *hate* WSGI string literals.  What path are you *actually* trying to test?\n\n    (Pdb) !b\u0027/v1/acc\\xc3\\xa9sskey/c/o\u0027.decode(\u0027utf8\u0027)\n    \u0027/v1/accésskey/c/o\u0027\n    (Pdb) !b\u0027/v1/acc\\xc3\\xa9sskey/c/o\u0027.decode(\u0027latin1\u0027)\n    \u0027/v1/accésskey/c/o\u0027\n\nI *assume* it\u0027s accésskey; but in modern python this string is *clearly* something else:\n\n    $ python3 -c \"print(\u0027/v1/acc\\xc3\\xa9sskey/c/o\u0027)\"\n    /v1/accésskey/c/o\n    \nAnd this particular character is *extra* annoying because it *has* a valid latin-1 encoding, and `swob.Request.blank` seems to prefer latin-1 strings:\n\n    \u003e\u003e\u003e swob.Request.blank(\u0027/v1/test☃/c/o\u0027)\n    Traceback (most recent call last):\n      File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n      File \"/vagrant/swift/swift/common/swob.py\", line 943, in blank\n        path.encode(\u0027latin1\u0027)\n    UnicodeEncodeError: \u0027latin-1\u0027 codec can\u0027t encode character \u0027\\u2603\u0027 in position 8: ordinal not in range(256)\n    \u003e\u003e\u003e swob.Request.blank(\u0027/v1/accésskey/c/o\u0027).path\n    \u0027/v1/acc%E9sskey/c/o\u0027","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":true,"context_lines":[{"line_number":512,"context_line":"        req.get_response(self.middleware)"},{"line_number":513,"context_line":"        self._assert_authorized(req, account_path\u003d\u0027/v1/\u0027,"},{"line_number":514,"context_line":"                                access_key\u003du\u0027acc\\u00e9ss\u0027)"},{"line_number":515,"context_line":"        self.assertEqual(req.environ[\u0027PATH_INFO\u0027], \u0027/v1/AUTH_TENANT_ID/c/o\u0027)"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":"    def test_authorize_with_access_key_and_unquote_chars(self):"},{"line_number":518,"context_line":"        req \u003d Request.blank(\u0027/v1/access%key\u003d/c/o\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f881bb58_48afd2e9","line":515,"updated":"2024-03-25 20:06:18.000000000","message":"it\u0027s a little confusing to me that the value that gets matched in the replace is just \u0027whateverever is currently in this slot\u0027 and not really in anyway related to the access_key.\n\nIn *real* s3api we\u0027d never get a request like:\n\n    \u0027/v1/accesskey/c/o\u0027 with [\u0027s3api.auth_details\u0027][\u0027access_key\u0027] \u003d \u0027access\u0027\n    \nsince s3request uses the same `self.access_key` when building the `auth_details` AND as part of the `PATH_INFO` rewrite.  But for the sake of this test you could change the account in the path at L506 to whatever you want and it will still pass.\n\n    diff --git a/test/unit/common/middleware/s3api/test_s3token.py b/test/unit/common/middleware/s3api/test_s3token.py\n    index 6a467bd3a..ef4ef35ef 100644\n    --- a/test/unit/common/middleware/s3api/test_s3token.py\n    +++ b/test/unit/common/middleware/s3api/test_s3token.py\n    @@ -503,7 +503,7 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):\n             self.assertEqual(req.environ[\u0027PATH_INFO\u0027], \u0027/v1/AUTH_TENANT_ID/c/o\u0027)\n     \n         def test_authorize_with_unicode_access_key(self):\n    -        req \u003d Request.blank(\u0027/v1/acc\\xc3\\xa9sskey/c/o\u0027)\n    +        req \u003d Request.blank(\u0027/v1/foo/c/o\u0027)\n             req.environ[\u0027s3api.auth_details\u0027] \u003d {\n                 \u0027access_key\u0027: u\u0027acc\\u00e9ss\u0027,\n                 \u0027signature\u0027: u\u0027signature\u0027,\n\nI\u0027m not sure that\u0027s such a good thing; it\u0027s obviously cribbing from the test above which does a similarly poor job at being a good s3request fake.","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"}],"test/unit/common/middleware/test_tempauth.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"908d40308d527a742933dedf0b1ace4760749320","unresolved":true,"context_lines":[{"line_number":317,"context_line":"                    for k, v in conf.items()}"},{"line_number":318,"context_line":"            access_key \u003d access_key.encode(\u0027utf8\u0027)"},{"line_number":319,"context_line":"        local_auth \u003d auth.filter_factory(conf)(local_app)"},{"line_number":320,"context_line":"        req \u003d self._make_request(\u0027/v1/t\\xc3\\xa9st:t\\xc3\\xa9ster\u0027, environ\u003d{"},{"line_number":321,"context_line":"            \u0027s3api.auth_details\u0027: {"},{"line_number":322,"context_line":"                \u0027access_key\u0027: access_key,"},{"line_number":323,"context_line":"                \u0027signature\u0027: b64encode(\u0027sig\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"f793ce8f_c37097c8","line":320,"updated":"2024-03-25 20:06:18.000000000","message":"I expected this test to pass with the changes in `swift/` reverted since utf-8 code points in the tempauth config actually already work on master - but it turns out it still fails because this \"stub\" is sending a latin-1 encoded wsgi string into tempauth (where as the real s3api on master would, incorrectly, send the utf-8 encoded account name in PATH_INFO).  Since reverting the change to s3api doesn\u0027t effect this test it still fails on master - and is totally dependent on this stub being a good fake for what real s3api would do... which it *is* NOW - but that is kind of how we got into this mess in `test.s3api`","commit_id":"8424b02290c75a7e1eb2e36296b41926f041249a"}]}