)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1e2c4a91bfb2f55d6b27e45825883c70214c6a84","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the __call__() method of formpost.py, the body received by self._translate_form() method for the POST request is used as it is."},{"line_number":10,"context_line":"In _translate_form(), `status` returned from the self._perform_subrequest() method is used, and the body is created using that `status`,e.g., `400 Bad Request`"},{"line_number":11,"context_line":"Therefore, even if an error response is received from other middleware, the error response body received from other middleware is ignored when passing through the formpost middleware."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1990942"},{"line_number":14,"context_line":"Change-Id: I4020e90dfaf7370a7941d123e9bea920c09b1aa0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9b41f4d6_e3b10b86","line":11,"updated":"2022-10-11 13:20:06.000000000","message":"this mostly seems to say what currently happens, I think you should try to say what SHOULD happen and especially WHY","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":35351,"name":"Donggyu Choi","display_name":"dongu","email":"gmj03003@gmail.com","username":"donguu"},"change_message_id":"1128a1e375dd046a46a86df761510de567aa31de","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the __call__() method of formpost.py, the body received by self._translate_form() method for the POST request is used as it is."},{"line_number":10,"context_line":"In _translate_form(), `status` returned from the self._perform_subrequest() method is used, and the body is created using that `status`,e.g., `400 Bad Request`"},{"line_number":11,"context_line":"Therefore, even if an error response is received from other middleware, the error response body received from other middleware is ignored when passing through the formpost middleware."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1990942"},{"line_number":14,"context_line":"Change-Id: I4020e90dfaf7370a7941d123e9bea920c09b1aa0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"78e21dec_916512e0","line":11,"in_reply_to":"9b41f4d6_e3b10b86","updated":"2022-10-18 01:21:35.000000000","message":"I rewritten the commit message. Please check again. Thank you.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1e2c4a91bfb2f55d6b27e45825883c70214c6a84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"28c8833d_23e5fa24","updated":"2022-10-11 13:20:06.000000000","message":"I think this change should to have new tests added that demonstrate the behavior you\u0027d like to maintain before it\u0027s suitable to merge as closing the the bug.\n\nDo you want to keep working on this change, or would you like some more help?","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fc1a1c8335f5207b692f8dab6abeb12f8f81e134","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"075694ad_fe6ee570","updated":"2022-09-30 15:04:51.000000000","message":"needs tests\n\n\tdiff --git a/test/unit/common/middleware/test_formpost.py b/test/unit/common/middleware/test_formpost.py\n\tindex d751062e8..4ebada2fe 100644\n\t--- a/test/unit/common/middleware/test_formpost.py\n\t+++ b/test/unit/common/middleware/test_formpost.py\n\t@@ -1046,7 +1046,7 @@ class TestFormPost(unittest.TestCase):\n\t\t self.assertTrue(b\u0027201 Created\u0027 in body)\n\t\t self.assertEqual(len(self.app.requests), 2)\n\t \n\t-    def test_subrequest_fails(self):\n\t+    def test_subrequest_fails_redirect_404(self):\n\t\t key \u003d b\u0027abc\u0027\n\t\t sig, env, body \u003d self._make_sig_env_body(\n\t\t     \u0027/v1/AUTH_test/container\u0027, \u0027http://brim.net\u0027, 1024, 10,\n\t@@ -1083,6 +1083,36 @@ class TestFormPost(unittest.TestCase):\n\t\t self.assertTrue(b\u0027http://brim.net?status\u003d404\u0026message\u003d\u0027 in body)\n\t\t self.assertEqual(len(self.app.requests), 1)\n\t \n\t+    def test_subrequest_fails_no_redirect_503(self):\n\t+        key \u003d b\u0027abc\u0027\n\t+        sig, env, body \u003d self._make_sig_env_body(\n\t+            \u0027/v1/AUTH_test/container\u0027, \u0027\u0027, 1024, 10,\n\t+            int(time() + 86400), key)\n\t+        env[\u0027wsgi.input\u0027] \u003d BytesIO(b\u0027\\r\\n\u0027.join(body))\n\t+        env[\u0027swift.infocache\u0027][get_cache_key(\u0027AUTH_test\u0027)] \u003d (\n\t+            self._fake_cache_env(\u0027AUTH_test\u0027, [key]))\n\t+        env[\u0027swift.infocache\u0027][get_cache_key(\n\t+            \u0027AUTH_test\u0027, \u0027container\u0027)] \u003d {\u0027meta\u0027: {}}\n\t+        self.app \u003d FakeApp(iter([(\u0027503 Server Error\u0027, {}, b\u0027some bad news\u0027)]))\n\t+        self.auth \u003d tempauth.filter_factory({})(self.app)\n\t+        self.formpost \u003d formpost.filter_factory({})(self.auth)\n\t+        status \u003d [None]\n\t+        headers \u003d [None]\n\t+        exc_info \u003d [None]\n\t+\n\t+        def start_response(s, h, e\u003dNone):\n\t+            status[0] \u003d s\n\t+            headers[0] \u003d h\n\t+            exc_info[0] \u003d e\n\t+\n\t+        body \u003d b\u0027\u0027.join(self.formpost(env, start_response))\n\t+        status \u003d status[0]\n\t+        headers \u003d headers[0]\n\t+        exc_info \u003d exc_info[0]\n\t+        self.assertEqual(status, \u0027503 Server Error\u0027)\n\t+        self.assertTrue(b\u0027bad news\u0027 in body)\n\t+        self.assertEqual(len(self.app.requests), 1)\n\t+\n\t     def test_truncated_attr_value(self):\n\t\t key \u003d b\u0027abc\u0027\n\t\t redirect \u003d \u0027a\u0027 * formpost.MAX_VALUE_LENGTH\n\n","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1e2c4a91bfb2f55d6b27e45825883c70214c6a84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"db918ad5_25e129f4","in_reply_to":"075694ad_fe6ee570","updated":"2022-10-11 13:20:06.000000000","message":"Can you add a test like this to this change?\n\nhttps://opendev.org/openstack/swift/src/branch/master/REVIEW_GUIDELINES.rst#new-tests","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":35351,"name":"Donggyu Choi","display_name":"dongu","email":"gmj03003@gmail.com","username":"donguu"},"change_message_id":"1128a1e375dd046a46a86df761510de567aa31de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c08cffb7_98390eb3","in_reply_to":"28c8833d_23e5fa24","updated":"2022-10-18 01:21:35.000000000","message":"I add this. Thank you.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":23279,"name":"Seongsoo Cho","display_name":"Seongsoo Cho","email":"ppiyakk2@printf.kr","username":"seongsoo.cho"},"change_message_id":"178a80347c7c3650d005201bf4a94a5dbe603364","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3a857b3d_2405f2c5","updated":"2022-10-19 23:02:35.000000000","message":"RECHECK","commit_id":"4cafa5c535ff43b03279787eb8683f352b9be27b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3a016514_05695e17","updated":"2022-10-20 15:36:56.000000000","message":"I think *maybe* it\u0027s NBD to do the \u0027\u0027.join(body) because it *should* only ever be smallish 400/201 style responses to a form *POST* (actually a swift PUT) - but I\u0027m not 100% sure.","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"32989872635c82da0fb410cd2294ba26bc737501","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a0a7f008_6129ff53","updated":"2022-11-01 16:16:52.000000000","message":"Seems reasonable. Thanks!","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"}],"swift/common/middleware/formpost.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fc1a1c8335f5207b692f8dab6abeb12f8f81e134","unresolved":true,"context_lines":[{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp and isinstance(resp, list):"},{"line_number":343,"context_line":"                body \u003d resp[0]"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"},{"line_number":346,"context_line":"            return status, headers, body"}],"source_content_type":"text/x-python","patch_set":2,"id":"4d9832ca_07f72176","line":343,"updated":"2022-09-30 15:04:51.000000000","message":"hrere i think next(resp) might be better because some times WSGI applications return a generator instead of a list.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":35351,"name":"Donggyu Choi","display_name":"dongu","email":"gmj03003@gmail.com","username":"donguu"},"change_message_id":"e8bb468839c33284c966f4a5cb86941e8c71d24d","unresolved":false,"context_lines":[{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp and isinstance(resp, list):"},{"line_number":343,"context_line":"                body \u003d resp[0]"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"},{"line_number":346,"context_line":"            return status, headers, body"}],"source_content_type":"text/x-python","patch_set":2,"id":"a25113db_0f24b2ba","line":343,"in_reply_to":"4d9832ca_07f72176","updated":"2022-10-11 01:18:35.000000000","message":"I check the type of resp through `isinstance(resp, list)`. Because resp is a list type when other middleware has an error response body.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1e2c4a91bfb2f55d6b27e45825883c70214c6a84","unresolved":false,"context_lines":[{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp and isinstance(resp, list):"},{"line_number":343,"context_line":"                body \u003d resp[0]"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"},{"line_number":346,"context_line":"            return status, headers, body"}],"source_content_type":"text/x-python","patch_set":2,"id":"d5609a5b_b6472988","line":343,"in_reply_to":"a25113db_0f24b2ba","updated":"2022-10-11 13:20:06.000000000","message":"I can see the isinstance check.  I probably wasn\u0027t clear: this change would look BETTER if you handle \"fixing the error response body\" on ANY type of error resp iterator (list or generator).\n\nI don\u0027t think that \"resp is a list type when other middleware has an error response body\" is exahustively correct.  Since the WSGI interface supports generators the MOST correct change would support lists AND iterators.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":35351,"name":"Donggyu Choi","display_name":"dongu","email":"gmj03003@gmail.com","username":"donguu"},"change_message_id":"5993a99943e7fe2140f968d6d21de56b7772a176","unresolved":true,"context_lines":[{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp and isinstance(resp, list):"},{"line_number":343,"context_line":"                body \u003d resp[0]"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"},{"line_number":346,"context_line":"            return status, headers, body"}],"source_content_type":"text/x-python","patch_set":2,"id":"91fef37d_b14b8dc6","line":343,"in_reply_to":"d5609a5b_b6472988","updated":"2022-10-19 00:22:41.000000000","message":"I modified it to work not only for lists but also for iterators.","commit_id":"8d8959df4a0812423baeb292da8714f61c6be407"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":328,"context_line":"            status \u003d \u0027400 Bad Request\u0027"},{"line_number":329,"context_line":"            message \u003d \u0027no files to process\u0027"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        status_code \u003d int(status.split(\u0027 \u0027, 1)[0])"},{"line_number":332,"context_line":"        headers \u003d [(k, v) for k, v in subheaders"},{"line_number":333,"context_line":"                   if k.lower().startswith(\u0027access-control\u0027)]"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e63963e4_9c8f9b48","line":331,"updated":"2022-10-20 15:36:56.000000000","message":"it kinda bugs me that we may have *just* done this right before we break","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                body \u003d status + \u0027\\r\\nFormPost: \u0027 + message.title()"},{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp_body:"},{"line_number":343,"context_line":"                body \u003d resp_body"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"}],"source_content_type":"text/x-python","patch_set":5,"id":"b08b4369_9123337b","line":342,"updated":"2022-10-20 15:36:56.000000000","message":"sometimes I find it makes the binding more obvious to write this like:\n\n    if resp_body and not is_success:\n    \nso someone doesn\u0027t accidently think this code is actually:\n\n    if not (is_success and resp_body):","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":340,"context_line":"            if six.PY3:"},{"line_number":341,"context_line":"                body \u003d body.encode(\u0027utf-8\u0027)"},{"line_number":342,"context_line":"            if not is_success(status_code) and resp_body:"},{"line_number":343,"context_line":"                body \u003d resp_body"},{"line_number":344,"context_line":"            headers.extend([(\u0027Content-Type\u0027, \u0027text/plain\u0027),"},{"line_number":345,"context_line":"                            (\u0027Content-Length\u0027, len(body))])"},{"line_number":346,"context_line":"            return status, headers, body"}],"source_content_type":"text/x-python","patch_set":5,"id":"0c2d1bd5_a48c5623","line":343,"updated":"2022-10-20 15:36:56.000000000","message":"since we only *use* resp_body on errors - do you think we could avoid the \u0027\u0027.join(resp) on large file 200 success?\n\nI guess the current interface of _perform_subrequest closes the request before handing it back to the loop in translate_form; but we could change that as long as we always close the resp iterator of every request.  But it gets kind of hairy.\n\nMaybe update the interface to return:\n\n    status, subheaders, error_body \u003d _perform_subrequest\n    \nthen you can bounce out of the loop when\n\n    if not error_body:\n        break","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":456,"context_line":"        resp \u003d self.app(subenv, _start_response)"},{"line_number":457,"context_line":"        with closing_if_possible(reiterate(resp)):"},{"line_number":458,"context_line":"            body \u003d b\u0027\u0027.join(resp)"},{"line_number":459,"context_line":"        return substatus[0], subheaders[0], body"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _get_keys(self, env):"},{"line_number":462,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"8a09f2b9_7ac7534e","line":459,"updated":"2022-10-20 15:36:56.000000000","message":"ok, so _perform_subrequest will always return a (closed) body\n\nis there ever a concern this body might be HUGE!?  Like on a 200 response?\n\n... maybe this is not a concern in practice because this is always a PUT request","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"32989872635c82da0fb410cd2294ba26bc737501","unresolved":true,"context_lines":[{"line_number":456,"context_line":"        resp \u003d self.app(subenv, _start_response)"},{"line_number":457,"context_line":"        with closing_if_possible(reiterate(resp)):"},{"line_number":458,"context_line":"            body \u003d b\u0027\u0027.join(resp)"},{"line_number":459,"context_line":"        return substatus[0], subheaders[0], body"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _get_keys(self, env):"},{"line_number":462,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"78240c67_5fc32e7a","line":459,"in_reply_to":"8a09f2b9_7ac7534e","updated":"2022-11-01 16:16:52.000000000","message":"Yeah, I think it\u0027s reasonable to assume these should always be fairly small. Makes me kinda want to write a helper like\n\n take_first(app_iter, n_bytes)\n\nso we could say\n\n app_iter \u003d self.app(subenv, _start_response)\n with closing_if_possible(app_iter):\n     body \u003d take_first(app_iter, 1024)\n return ...\n\nand have it consume the whole body if it\u0027s small but leave it to 499 if it\u0027s big.\n\n(Note that the reiterate() is no longer necessary, but it doesn\u0027t really hurt much either.)","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"}],"test/unit/common/middleware/test_formpost.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":82,"context_line":"                    return resp(env, start_response)"},{"line_number":83,"context_line":"            status, headers, body \u003d next(self.status_headers_body_iter)"},{"line_number":84,"context_line":"            return Response(status\u003dstatus, headers\u003dheaders,"},{"line_number":85,"context_line":"                            body\u003dbody)(env, start_response)"},{"line_number":86,"context_line":"        except EOFError:"},{"line_number":87,"context_line":"            start_response(\u0027499 Client Disconnect\u0027,"},{"line_number":88,"context_line":"                           [(\u0027Content-Type\u0027, \u0027text/plain\u0027)])"}],"source_content_type":"text/x-python","patch_set":5,"id":"d3541d7d_6946a8fb","line":85,"updated":"2022-10-20 15:36:56.000000000","message":"Hrm... doesn\u0027t seem like Response can take an interator as the body param, you\u0027d have to pass it in like:\n\n    if isinstance(body, six.string_types):\n        kwargs[\u0027body\u0027] \u003d body\n    else:\n        kwargs[\u0027app_iter\u0027] \u003d body\n    return Response(**kwargs)()","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bf7000baea4ff9cb14a6ec278118543ecc2eaba0","unresolved":true,"context_lines":[{"line_number":1093,"context_line":"            self._fake_cache_env(\u0027AUTH_test\u0027, [key]))"},{"line_number":1094,"context_line":"        env[\u0027swift.infocache\u0027][get_cache_key("},{"line_number":1095,"context_line":"            \u0027AUTH_test\u0027, \u0027container\u0027)] \u003d {\u0027meta\u0027: {}}"},{"line_number":1096,"context_line":"        self.app \u003d FakeApp(iter([(\u0027503 Server Error\u0027, {}, b\u0027some bad news\u0027)]))"},{"line_number":1097,"context_line":"        self.auth \u003d tempauth.filter_factory({})(self.app)"},{"line_number":1098,"context_line":"        self.formpost \u003d formpost.filter_factory({})(self.auth)"},{"line_number":1099,"context_line":"        status \u003d [None]"}],"source_content_type":"text/x-python","patch_set":5,"id":"f7dab7de_732cd9c3","line":1096,"updated":"2022-10-20 15:36:56.000000000","message":"I wonder if FakeApp can take a body_resp generator instead of just a byte string","commit_id":"326e68d70d235c7d90e2b07a49621e086e6632c9"}]}