)]}'
{"swift/common/middleware/gatekeeper.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e51b935aeadfdf9dc30ca29d487f3679b6824639","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        def gatekeeper_response(status, response_headers, exc_info\u003dNone):"},{"line_number":93,"context_line":"            def fixed_response_headers():"},{"line_number":94,"context_line":"                return ["},{"line_number":95,"context_line":"                    (k, v) if not env.get(\u0027swift.leave_relative_location\u0027)"},{"line_number":96,"context_line":"                    or k.lower() !\u003d \u0027location\u0027 else"},{"line_number":97,"context_line":"                    (k, urlparse(v).path) for (k, v) in response_headers"},{"line_number":98,"context_line":"                ]"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f2577a7_f61e52b9","line":95,"range":{"start_line":95,"start_character":27,"end_line":95,"end_character":74},"updated":"2017-09-15 20:33:28.000000000","message":"Move this check up, before the comprehension?","commit_id":"e3300b440f20ca71e6a91f4f9db0dc08fe2a3df3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e51b935aeadfdf9dc30ca29d487f3679b6824639","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                return ["},{"line_number":95,"context_line":"                    (k, v) if not env.get(\u0027swift.leave_relative_location\u0027)"},{"line_number":96,"context_line":"                    or k.lower() !\u003d \u0027location\u0027 else"},{"line_number":97,"context_line":"                    (k, urlparse(v).path) for (k, v) in response_headers"},{"line_number":98,"context_line":"                ]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"            response_headers \u003d fixed_response_headers()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f2577a7_768022c0","line":97,"range":{"start_line":97,"start_character":36,"end_line":97,"end_character":40},"updated":"2017-09-15 20:33:28.000000000","message":"We lost any query params / url fragments that may have been included.","commit_id":"e3300b440f20ca71e6a91f4f9db0dc08fe2a3df3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c709621d80fa1f1b628ed29e79dd3a4b3858a7cb","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        def gatekeeper_response(status, response_headers, exc_info\u003dNone):"},{"line_number":93,"context_line":"            def fixed_response_headers():"},{"line_number":94,"context_line":"                def relative_path(value):"},{"line_number":95,"context_line":"                    parsed \u003d urlparse(v)"},{"line_number":96,"context_line":"                    new_path \u003d parsed.path"},{"line_number":97,"context_line":"                    if parsed.query:"},{"line_number":98,"context_line":"                        new_path +\u003d (\u0027?%s\u0027 % parsed.query)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_5ea8988a","line":95,"range":{"start_line":95,"start_character":29,"end_line":95,"end_character":37},"updated":"2017-09-16 06:11:41.000000000","message":"Ok, one more :-/\n\nThere\u0027s another function that similar to, but subtly different from urlparse: urlsplit.\n\n \u003e\u003e\u003e urlparse(\u0027/some/path?with\u003dquery#and-frag\u0027)\n ParseResult(scheme\u003d\u0027\u0027, netloc\u003d\u0027\u0027, path\u003d\u0027/some/path\u0027, params\u003d\u0027\u0027,\n             query\u003d\u0027with\u003dquery\u0027, fragment\u003d\u0027and-frag\u0027)\n \u003e\u003e\u003e urlsplit(\u0027/some/path?with\u003dquery#and-frag\u0027)\n SplitResult(scheme\u003d\u0027\u0027, netloc\u003d\u0027\u0027, path\u003d\u0027/some/path\u0027,\n             query\u003d\u0027with\u003dquery\u0027, fragment\u003d\u0027and-frag\u0027)\n\nSo, what\u0027s \"params\"? Something... weird...\n\n \u003e\u003e\u003e urlparse(\u0027/some/path;with-params?and\u003dquery#and-frag\u0027)\n ParseResult(scheme\u003d\u0027\u0027, netloc\u003d\u0027\u0027, path\u003d\u0027/some/path\u0027, params\u003d\u0027with-params\u0027,\n             query\u003d\u0027and\u003dquery\u0027, fragment\u003d\u0027and-frag\u0027)\n \u003e\u003e\u003e urlsplit(\u0027/some/path;with-params?and\u003dquery#and-frag\u0027)\n SplitResult(scheme\u003d\u0027\u0027, netloc\u003d\u0027\u0027, path\u003d\u0027/some/path;with-params\u0027,\n             query\u003d\u0027and\u003dquery\u0027, fragment\u003d\u0027and-frag\u0027)\n\nProbably some holdover from the CGI days? But I don\u0027t want Swift paths that include semicolons to break...","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0c9b7a6e467b23619e1a83ed50e8dff94ca1d547","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        def gatekeeper_response(status, response_headers, exc_info\u003dNone):"},{"line_number":93,"context_line":"            def fixed_response_headers():"},{"line_number":94,"context_line":"                def relative_path(value):"},{"line_number":95,"context_line":"                    parsed \u003d urlparse(v)"},{"line_number":96,"context_line":"                    new_path \u003d parsed.path"},{"line_number":97,"context_line":"                    if parsed.query:"},{"line_number":98,"context_line":"                        new_path +\u003d (\u0027?%s\u0027 % parsed.query)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_d9edc2f0","line":95,"range":{"start_line":95,"start_character":29,"end_line":95,"end_character":37},"in_reply_to":"5f2577a7_5ea8988a","updated":"2017-09-16 13:04:26.000000000","message":"oh my. urlsplit could be better to save if statement here. will change.","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c709621d80fa1f1b628ed29e79dd3a4b3858a7cb","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                    return response_headers"},{"line_number":105,"context_line":"                else:"},{"line_number":106,"context_line":"                    return ["},{"line_number":107,"context_line":"                        (k, v) if k.lower() !\u003d \u0027location\u0027 else"},{"line_number":108,"context_line":"                        (k, relative_path(v)) for (k, v) in response_headers"},{"line_number":109,"context_line":"                    ]"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_1e96904d","line":107,"range":{"start_line":107,"start_character":44,"end_line":107,"end_character":57},"updated":"2017-09-16 06:11:41.000000000","message":"Worth being defensive and doing\n\n in (\u0027location\u0027, \u0027content-location\u0027)\n\n? Not that I\u0027m aware of anything that would translate a Content-Location header...","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0c9b7a6e467b23619e1a83ed50e8dff94ca1d547","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                    return response_headers"},{"line_number":105,"context_line":"                else:"},{"line_number":106,"context_line":"                    return ["},{"line_number":107,"context_line":"                        (k, v) if k.lower() !\u003d \u0027location\u0027 else"},{"line_number":108,"context_line":"                        (k, relative_path(v)) for (k, v) in response_headers"},{"line_number":109,"context_line":"                    ]"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_19e8eadf","line":107,"range":{"start_line":107,"start_character":44,"end_line":107,"end_character":57},"in_reply_to":"5f2577a7_1e96904d","updated":"2017-09-16 13:04:26.000000000","message":"I suppose content-location has no chance to be changed by any middlewares right now. Even in the swob, it only cares of *location* header so that i think this is safe.","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"}],"test/unit/common/middleware/test_gatekeeper.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c709621d80fa1f1b628ed29e79dd3a4b3858a7cb","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                self._test_duplicate_headers_not_removed(method, app_hdrs)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def test_location_header_fixed(self):"},{"line_number":219,"context_line":"        location_path \u003d \u0027/v/a/c/o2?query\u003dpath#test\u0027"},{"line_number":220,"context_line":"        headers \u003d {\u0027Location\u0027: location_path}"},{"line_number":221,"context_line":"        req \u003d Request.blank("},{"line_number":222,"context_line":"            \u0027/v/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_de9b6870","line":219,"updated":"2017-09-16 06:11:41.000000000","message":"Better to just supply an absolute URL here? It\u0027s not obvious that\n\n resp.location \u003d resp.absolute_location()\n\nactually has any effect in tests... what should be the host/scheme?","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0c9b7a6e467b23619e1a83ed50e8dff94ca1d547","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                self._test_duplicate_headers_not_removed(method, app_hdrs)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def test_location_header_fixed(self):"},{"line_number":219,"context_line":"        location_path \u003d \u0027/v/a/c/o2?query\u003dpath#test\u0027"},{"line_number":220,"context_line":"        headers \u003d {\u0027Location\u0027: location_path}"},{"line_number":221,"context_line":"        req \u003d Request.blank("},{"line_number":222,"context_line":"            \u0027/v/a/c\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027GET\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f2577a7_b9eaf6d4","line":219,"in_reply_to":"5f2577a7_de9b6870","updated":"2017-09-16 13:04:26.000000000","message":"It means we should avoid SelfishApp below? Hmm... currently I\u0027d like to keep this context here because setting the location with absolute path here is NOT consistent with \u0027swift.leave_relative_location\u0027 \u003d True in the request env.\n\nTo call the alarm that some of middlewares using webob may affect, current test seems better to me.","commit_id":"20fa8ab72b073bc9cae0eeb72b26691cc3cb0689"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e22136d38342d747638d9bf0284e7881f891ac1b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                headers\u003dself.headers)"},{"line_number":229,"context_line":"                # like webob, middlewares in the pipeline may rewrite"},{"line_number":230,"context_line":"                # location header from relative to absolute"},{"line_number":231,"context_line":"                resp.location \u003d resp.absolute_location()"},{"line_number":232,"context_line":"                return resp(env, start_response)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        selfish_app \u003d SelfishApp(headers\u003dheaders)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_b4a67daa","line":231,"updated":"2017-09-16 16:10:32.000000000","message":"OK, yeah, this *does* set things up the way that we want. But I had to do something like\n\n diff --git a/test/unit/common/middleware/test_gatekeeper.py b/test/unit/common/middlew\n index d8c0936..a98d6b9 100644\n --- a/test/unit/common/middleware/test_gatekeeper.py\n +++ b/test/unit/common/middleware/test_gatekeeper.py\n @@ -222,13 +222,13 @@ class TestGatekeeper(unittest.TestCase):\n                                 \u0027swift.leave_relative_location\u0027: True})\n \n          class SelfishApp(FakeApp):\n -            def __call__(self, env, start_response):\n -                self.req \u003d Request(env)\n -                resp \u003d Response(request\u003dself.req, body\u003d\u0027FAKE APP\u0027,\n -                                headers\u003dself.headers)\n +            def __call__(fake_app, env, start_response):\n +                resp \u003d Response(request\u003dRequest(env), body\u003d\u0027FAKE APP\u0027,\n +                                headers\u003dfake_app.headers)\n                  # like webob, middlewares in the pipeline may rewrite\n                  # location header from relative to absolute\n                  resp.location \u003d resp.absolute_location()\n +                self.assertTrue(resp.headers[\u0027Location\u0027].startswith(\u0027http\u0027))\n                  return resp(env, start_response)\n \n          selfish_app \u003d SelfishApp(headers\u003dheaders)\n\nto convince myself that swob wasn\u0027t going to \"helpfully\" return a non-absolute location on account of the environ[\u0027swift.leave_relative_location\u0027] setting.","commit_id":"d6fcf7459435077b525123e8b78e553070d5c070"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"569237193334488a683dcd1bf786d4950cff5f68","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                headers\u003dself.headers)"},{"line_number":229,"context_line":"                # like webob, middlewares in the pipeline may rewrite"},{"line_number":230,"context_line":"                # location header from relative to absolute"},{"line_number":231,"context_line":"                resp.location \u003d resp.absolute_location()"},{"line_number":232,"context_line":"                return resp(env, start_response)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        selfish_app \u003d SelfishApp(headers\u003dheaders)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff5d4b2f_c2bc31bd","line":231,"in_reply_to":"5f2577a7_b4a67daa","updated":"2017-09-19 21:03:24.000000000","message":"agree it would be nice to have a \u0027just for sanity sake\u0027 assert that shows how the location is absolute.","commit_id":"d6fcf7459435077b525123e8b78e553070d5c070"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e22136d38342d747638d9bf0284e7881f891ac1b","unresolved":false,"context_lines":[{"line_number":231,"context_line":"                resp.location \u003d resp.absolute_location()"},{"line_number":232,"context_line":"                return resp(env, start_response)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        selfish_app \u003d SelfishApp(headers\u003dheaders)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        app \u003d self.get_app(selfish_app, {})"},{"line_number":237,"context_line":"        resp \u003d req.get_response(app)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f2577a7_746935f7","line":234,"updated":"2017-09-16 16:10:32.000000000","message":"I\u0027m still not sure that SelfishApp is the clearest way to demonstrate the change we want. Yes, this came up because there\u0027s some WebOb-using middleware in the pipeline, but the core thing that we\u0027re fixing is\n\n- there is a full URL in the Location header, but\n- somewhere in the pipeline, someone said to expect a relative location.\n\nThat seems like it could come up in other ways (like a single middleware that did both of these things all by itself) and a story like\n\n        req \u003d Request.blank(\u0027/v/a/c\u0027)\n        # So we\u0027ve made a response that includes a relative Location header\n        inner_resp \u003d Response(request\u003dreq, headers\u003d{\n            \u0027Location\u0027: relative_location})\n        # And we\u0027ve explicitly said \"I want this to be relative\"\n        inner_resp.environ[\u0027swift.leave_relative_location\u0027] \u003d True\n\n        # Yet some middleware in the pipeline, maybe using webob, rewrote the\n        # location header to be a complete URL\n        inner_resp.headers[\u0027Location\u0027] \u003d (\n            \u0027some+scheme://host:port\u0027 + relative_location)\n\n        # As gatekeeper, we should make it host-relative again\n        final_resp \u003d req.get_response(self.get_app(inner_resp, {}))\n        self.assertIn(\u0027Location\u0027, final_resp.headers)\n        self.assertEqual(final_resp.headers[\u0027Location\u0027], relative_location)\n\nseems to demonstrate it just as well, without needing to hold quite as much in your head.","commit_id":"d6fcf7459435077b525123e8b78e553070d5c070"}]}