)]}'
{"swift/common/middleware/domain_remap.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b177c8d5019a8e07d0b2794b97fcf9706d126f71","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                new_path_parts.append(container)"},{"line_number":131,"context_line":"            if path.startswith(self.path_root):"},{"line_number":132,"context_line":"                path \u003d path[len(self.path_root):]"},{"line_number":133,"context_line":"            if path and path[0] \u003d\u003d \u0027/\u0027:"},{"line_number":134,"context_line":"                path \u003d path[1:]"},{"line_number":135,"context_line":"            new_path_parts.append(path)"},{"line_number":136,"context_line":"            new_path \u003d \u0027/\u0027.join(new_path_parts)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_c0620a17","line":133,"updated":"2017-04-24 10:37:13.000000000","message":"Nit - you used startswith two lines earlier, so you could do the same here:\n\n if path.startswith(\u0027/\u0027):","commit_id":"a50a99516f51c624e957a071d4684183fca3c8a3"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"c6e9627b0caab5eaa80cddd82b46c7b19b65dfe2","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                new_path_parts.append(container)"},{"line_number":131,"context_line":"            if path.startswith(self.path_root):"},{"line_number":132,"context_line":"                path \u003d path[len(self.path_root):]"},{"line_number":133,"context_line":"            if path and path[0] \u003d\u003d \u0027/\u0027:"},{"line_number":134,"context_line":"                path \u003d path[1:]"},{"line_number":135,"context_line":"            new_path_parts.append(path)"},{"line_number":136,"context_line":"            new_path \u003d \u0027/\u0027.join(new_path_parts)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_58625240","line":133,"in_reply_to":"5ff73747_c0620a17","updated":"2017-04-25 07:12:24.000000000","message":"Done","commit_id":"a50a99516f51c624e957a071d4684183fca3c8a3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75c79e68e7b26c3163908787e16a3e072ae25f8c","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                               if not s.startswith(\u0027.\u0027)]"},{"line_number":74,"context_line":"        self.storage_domain +\u003d [s for s in list_from_csv(storage_domain)"},{"line_number":75,"context_line":"                                if s.startswith(\u0027.\u0027)]"},{"line_number":76,"context_line":"        self.path_root \u003d \u0027/\u0027 + conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027)"},{"line_number":77,"context_line":"        prefixes \u003d conf.get(\u0027reseller_prefixes\u0027, \u0027AUTH\u0027)"},{"line_number":78,"context_line":"        self.reseller_prefixes \u003d list_from_csv(prefixes)"},{"line_number":79,"context_line":"        self.reseller_prefixes_lower \u003d [x.lower()"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_4a11eb03","line":76,"updated":"2017-05-11 17:43:43.000000000","message":"what am I missing here?\n\n    In [1]: \u0027///asdf////\u0027.strip(\u0027/\u0027)\n    Out[1]: \u0027asdf\u0027\n\nit *looks* like we\u0027re appending a \u0027/\u0027 oh the LHS only to strip it off?\n\nLOL, order of operations!  We\u0027re normalizing to no slashes and *then* adding one slash on the left.\n\nI find this more readable:\n\n    diff --git a/swift/common/middleware/domain_remap.py b/swift/common/middleware/domain_remap.py\n    index 23bd72f..1c79de2 100644\n    --- a/swift/common/middleware/domain_remap.py\n    +++ b/swift/common/middleware/domain_remap.py\n    @@ -73,7 +73,7 @@ class DomainRemapMiddleware(object):\n                                    if not s.startswith(\u0027.\u0027)]\n             self.storage_domain +\u003d [s for s in list_from_csv(storage_domain)\n                                     if s.startswith(\u0027.\u0027)]\n    -        self.path_root \u003d \u0027/\u0027 + conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027)\n    +        self.path_root \u003d \u0027/%s\u0027 % conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027)\n             prefixes \u003d conf.get(\u0027reseller_prefixes\u0027, \u0027AUTH\u0027)\n             self.reseller_prefixes \u003d list_from_csv(prefixes)\n             self.reseller_prefixes_lower \u003d [x.lower()","commit_id":"6db12b87ff6055f1b4d647a440e68a93387f4b24"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f72e0c3b0db90e5f86c713f1c87d51695eae6a7d","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                               if not s.startswith(\u0027.\u0027)]"},{"line_number":74,"context_line":"        self.storage_domain +\u003d [s for s in list_from_csv(storage_domain)"},{"line_number":75,"context_line":"                                if s.startswith(\u0027.\u0027)]"},{"line_number":76,"context_line":"        self.path_root \u003d \u0027/\u0027 + conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027)"},{"line_number":77,"context_line":"        prefixes \u003d conf.get(\u0027reseller_prefixes\u0027, \u0027AUTH\u0027)"},{"line_number":78,"context_line":"        self.reseller_prefixes \u003d list_from_csv(prefixes)"},{"line_number":79,"context_line":"        self.reseller_prefixes_lower \u003d [x.lower()"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_71ba4c6e","line":76,"in_reply_to":"3f044301_4a11eb03","updated":"2017-05-12 14:28:15.000000000","message":"IDK, I like it as-is. Why format when things *aren\u0027t complicated*? If all you want to do is concatenate, just concatenate.","commit_id":"6db12b87ff6055f1b4d647a440e68a93387f4b24"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f72e0c3b0db90e5f86c713f1c87d51695eae6a7d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            if container:"},{"line_number":130,"context_line":"                new_path_parts.append(container)"},{"line_number":131,"context_line":"            if path.startswith(self.path_root):"},{"line_number":132,"context_line":"                path \u003d path[len(self.path_root):]"},{"line_number":133,"context_line":"            if path.startswith(\u0027/\u0027):"},{"line_number":134,"context_line":"                path \u003d path[1:]"},{"line_number":135,"context_line":"            new_path_parts.append(path)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_305729cd","line":132,"updated":"2017-05-12 14:28:15.000000000","message":"Offtopic: I\u0027m confused about why we do this.\n\nSo I\u0027ve got some object like\n\n curl -X PUT http://saio.dev:8090/v1/AUTH_test/test-container/test -d 1234\n\nand I can get it like\n\n curl http://test-container.test.saio.dev:8090/test\n\nGreat. If I go and create some object like\n\n curl -X PUT http://saio.dev:8090/v1/AUTH_test/test-container/v1/wat -d 1234\n\n...then to get it with domain_remap, I need something like\n\n curl http://test-container.test.saio.dev:8090/v1/v1/wat\n\n? Huh?\n\nI guess... we should be returning a 404 if not path.startswith(self.path_root)? Or something?","commit_id":"6db12b87ff6055f1b4d647a440e68a93387f4b24"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75c79e68e7b26c3163908787e16a3e072ae25f8c","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            if path.startswith(self.path_root):"},{"line_number":132,"context_line":"                path \u003d path[len(self.path_root):]"},{"line_number":133,"context_line":"            if path.startswith(\u0027/\u0027):"},{"line_number":134,"context_line":"                path \u003d path[1:]"},{"line_number":135,"context_line":"            new_path_parts.append(path)"},{"line_number":136,"context_line":"            new_path \u003d \u0027/\u0027.join(new_path_parts)"},{"line_number":137,"context_line":"            env[\u0027PATH_INFO\u0027] \u003d new_path"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_ea18b71e","line":134,"updated":"2017-05-11 17:43:43.000000000","message":"this looks correct, but I would think the condition is always true...\n\nah, unless it\u0027s a request to the root, in which case after *stripping* path_root - path is just the empty string (in which case the slice is the identity function).\n\nFWIW this works too:\n\n    diff --git a/swift/common/middleware/domain_remap.py b/swift/common/middleware/domain_remap.py\n    index 23bd72f..199ba83 100644\n    --- a/swift/common/middleware/domain_remap.py\n    +++ b/swift/common/middleware/domain_remap.py\n    @@ -130,8 +130,8 @@ class DomainRemapMiddleware(object):\n                     new_path_parts.append(container)\n                 if path.startswith(self.path_root):\n                     path \u003d path[len(self.path_root):]\n    -            if path.startswith(\u0027/\u0027):\n    -                path \u003d path[1:]\n    +            # remove leading slash\n    +            path \u003d path[1:]\n                 new_path_parts.append(path)\n                 new_path \u003d \u0027/\u0027.join(new_path_parts)\n                 env[\u0027PATH_INFO\u0027] \u003d new_path","commit_id":"6db12b87ff6055f1b4d647a440e68a93387f4b24"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f72e0c3b0db90e5f86c713f1c87d51695eae6a7d","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            if path.startswith(self.path_root):"},{"line_number":132,"context_line":"                path \u003d path[len(self.path_root):]"},{"line_number":133,"context_line":"            if path.startswith(\u0027/\u0027):"},{"line_number":134,"context_line":"                path \u003d path[1:]"},{"line_number":135,"context_line":"            new_path_parts.append(path)"},{"line_number":136,"context_line":"            new_path \u003d \u0027/\u0027.join(new_path_parts)"},{"line_number":137,"context_line":"            env[\u0027PATH_INFO\u0027] \u003d new_path"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_2db5967e","line":134,"in_reply_to":"3f044301_ea18b71e","updated":"2017-05-12 14:28:15.000000000","message":"Dropping the startswith check seems bad:\n\n curl -X PUT http://saio.dev:8090/v1/AUTH_test/test-container/v123/test -d 1234\n curl -X PUT http://saio.dev:8090/v1/AUTH_test/test-container/v1/23/test -d 456\n curl -X PUT http://saio.dev:8090/v1/AUTH_test/test-container/23/test -d 789\n\nCurrently, we\u0027ll get back 789 for all three (!) of\n\n curl http://test-container.test.saio.dev:8090/23/test\n curl http://test-container.test.saio.dev:8090/v1/23/test\n curl http://test-container.test.saio.dev:8090/v123/test\n\n...while without the check the last one would 404 because we translate the path to http://saio.dev:8090/v1/AUTH_test/test-container/3/test. I\u0027m guessing you had a concern, like me, about how all of these conditionals make the code harder to reason about -- it makes me wonder if we\u0027d be better with something like\n\n diff --git a/swift/common/middleware/domain_remap.py b/swift/common/middleware/domain_remap.py\n index 23bd72ff..272c52ec 100644\n --- a/swift/common/middleware/domain_remap.py\n +++ b/swift/common/middleware/domain_remap.py\n @@ -73,7 +73,7 @@ class DomainRemapMiddleware(object):\n                                 if not s.startswith(\u0027.\u0027)]\n          self.storage_domain +\u003d [s for s in list_from_csv(storage_domain)\n                                  if s.startswith(\u0027.\u0027)]\n -        self.path_root \u003d \u0027/\u0027 + conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027)\n +        self.path_root \u003d conf.get(\u0027path_root\u0027, \u0027v1\u0027).strip(\u0027/\u0027) + \u0027/\u0027\n          prefixes \u003d conf.get(\u0027reseller_prefixes\u0027, \u0027AUTH\u0027)\n          self.reseller_prefixes \u003d list_from_csv(prefixes)\n          self.reseller_prefixes_lower \u003d [x.lower()\n @@ -124,14 +124,12 @@ class DomainRemapMiddleware(object):\n                      # account prefix is not in config list. bail.\n                      return self.app(env, start_response)\n  \n -            path \u003d env[\u0027PATH_INFO\u0027]\n -            new_path_parts \u003d [self.path_root, account]\n +            path \u003d env[\u0027PATH_INFO\u0027][1:]\n +            new_path_parts \u003d [\u0027\u0027, self.path_root[:-1], account]\n              if container:\n                  new_path_parts.append(container)\n -            if path.startswith(self.path_root):\n +            if (path + \u0027/\u0027).startswith(self.path_root):\n                  path \u003d path[len(self.path_root):]\n -            if path.startswith(\u0027/\u0027):\n -                path \u003d path[1:]\n              new_path_parts.append(path)\n              new_path \u003d \u0027/\u0027.join(new_path_parts)\n              env[\u0027PATH_INFO\u0027] \u003d new_path\n\nSo normalize path_root to always include a trailing slash (instead of the leading one), always drop the leading slash in the user-supplied path at the point at which we can be certain there *is* a leading slash, then pop off the path_root-with-delimiter if we see it.\n\nMore generally, though, I really wonder whether we ought to just *stop mangling the user-supplied path*, and just slap the path_version and account (and container, if supplied) in front. Maybe that\u0027s yet another bug, though? I\u0027ll write up a follow-up to try to make this all less insane...","commit_id":"6db12b87ff6055f1b4d647a440e68a93387f4b24"}],"test/unit/common/middleware/test_domain_remap.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b177c8d5019a8e07d0b2794b97fcf9706d126f71","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                            headers\u003d{\u0027Host\u0027: \u0027c.AUTH_a.example.com\u0027})"},{"line_number":111,"context_line":"        resp \u003d self.app(req.environ, start_response)"},{"line_number":112,"context_line":"        self.assertEqual(resp, \u0027/v1/AUTH_a/c/obj\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_domain_remap_account_container_with_path_root_and_path(self):"},{"line_number":115,"context_line":"        req \u003d Request.blank(\u0027/v1/obj\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027},"},{"line_number":116,"context_line":"                            headers\u003d{\u0027Host\u0027: \u0027c.AUTH_a.example.com\u0027})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_d43bef99","line":113,"updated":"2017-04-24 10:37:13.000000000","message":"I would have expected a test  that ensure objects with a trailing slash at the end are correctly parsed? Like this:\n\n def test_domain_remap_account_container_with_path_trailing_slash(self):\n     req \u003d Request.blank(\u0027/obj/\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027},\n                         headers\u003d{\u0027Host\u0027: \u0027c.AUTH_a.example.com\u0027})\n     resp \u003d self.app(req.environ, start_response)\n     self.assertEqual(resp, \u0027/v1/AUTH_a/c/obj/\u0027)\n\nThat tests fails on master, but passes with your fix applied (as expected).","commit_id":"a50a99516f51c624e957a071d4684183fca3c8a3"},{"author":{"_account_id":13852,"name":"Romain LE DISEZ","email":"romain.le-disez@corp.ovh.com","username":"rledisez"},"change_message_id":"c6e9627b0caab5eaa80cddd82b46c7b19b65dfe2","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                            headers\u003d{\u0027Host\u0027: \u0027c.AUTH_a.example.com\u0027})"},{"line_number":111,"context_line":"        resp \u003d self.app(req.environ, start_response)"},{"line_number":112,"context_line":"        self.assertEqual(resp, \u0027/v1/AUTH_a/c/obj\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_domain_remap_account_container_with_path_root_and_path(self):"},{"line_number":115,"context_line":"        req \u003d Request.blank(\u0027/v1/obj\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027},"},{"line_number":116,"context_line":"                            headers\u003d{\u0027Host\u0027: \u0027c.AUTH_a.example.com\u0027})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_38678650","line":113,"in_reply_to":"5ff73747_d43bef99","updated":"2017-04-25 07:12:24.000000000","message":"Done","commit_id":"a50a99516f51c624e957a071d4684183fca3c8a3"}]}