)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":30,"id":"464f72b0_d47327b5","updated":"2024-10-09 18:31:52.000000000","message":"I have done an initial pass on this patch while we are also working on addressing the functest failures on https://review.opendev.org/c/openstack/swift/+/909801/29, maybe after that we can revisit some of the unresolved comments on this one","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2404696e459788e5eb5d2727a89043104b1f5d35","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"588ba520_8d9b2333","in_reply_to":"464f72b0_d47327b5","updated":"2024-12-05 16:37:36.000000000","message":"Done","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"12100387109685fe02b2a7949ef89b87fa3f7e9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"e5520d4d_a2a34860","updated":"2024-10-10 15:15:56.000000000","message":"recheck","commit_id":"506199f441141912444661dfe3043a56b23b7589"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"8a872b93f03650497aac4672ad117d64d2592ddf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"aa56768a_14671ea2","updated":"2024-10-24 20:58:09.000000000","message":"working on addressing the functest failures","commit_id":"78b7e79ed80af5b34539a438f3eb5087591b29c1"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2404696e459788e5eb5d2727a89043104b1f5d35","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"d6fe1fae_4367a736","in_reply_to":"8d21410f_cd08d89f","updated":"2024-12-05 16:37:36.000000000","message":"Done","commit_id":"78b7e79ed80af5b34539a438f3eb5087591b29c1"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"45e87dfa353964e47684ee4297bef5479fe1f137","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":32,"id":"8d21410f_cd08d89f","in_reply_to":"aa56768a_14671ea2","updated":"2024-10-24 20:58:51.000000000","message":"i meant cross-compat tests*","commit_id":"78b7e79ed80af5b34539a438f3eb5087591b29c1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fa6eafef08936aa2e3635ff448301bbfffc714cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"3722b79a_60459904","updated":"2024-12-18 22:04:47.000000000","message":"will test it some in my saio today. I\u0027ve tested the other patches, but not this one yet. Then will come back and score :)","commit_id":"1bc2b591b9d83a48e6ee89fa08e76c5f46cfe381"}],"swift/common/middleware/s3api/controllers/multi_upload.py":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[{"line_number":209,"context_line":"        resp, _ \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        algo \u003d resp.sysmeta_headers.get(sysmeta_header("},{"line_number":212,"context_line":"            \u0027object\u0027, \u0027checksum-algorithm\u0027), \u0027\u0027)"},{"line_number":213,"context_line":"        req_checksums \u003d ["},{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"}],"source_content_type":"text/x-python","patch_set":30,"id":"d486afcd_0ba0f5c1","line":212,"updated":"2024-10-09 18:31:52.000000000","message":"We get the checksum algorithm from the response we got above for our part upload","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2404696e459788e5eb5d2727a89043104b1f5d35","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        resp, _ \u003d _get_upload_info(req, self.app, upload_id)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        algo \u003d resp.sysmeta_headers.get(sysmeta_header("},{"line_number":212,"context_line":"            \u0027object\u0027, \u0027checksum-algorithm\u0027), \u0027\u0027)"},{"line_number":213,"context_line":"        req_checksums \u003d ["},{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"}],"source_content_type":"text/x-python","patch_set":30,"id":"b1e4b954_b52cc9cb","line":212,"in_reply_to":"d486afcd_0ba0f5c1","updated":"2024-12-05 16:37:36.000000000","message":"Done","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        algo \u003d resp.sysmeta_headers.get(sysmeta_header("},{"line_number":212,"context_line":"            \u0027object\u0027, \u0027checksum-algorithm\u0027), \u0027\u0027)"},{"line_number":213,"context_line":"        req_checksums \u003d ["},{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"}],"source_content_type":"text/x-python","patch_set":30,"id":"a674c62a_1957013f","line":213,"updated":"2024-10-09 18:31:52.000000000","message":"This extracts whatever checksum we are using in the req.headers","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2404696e459788e5eb5d2727a89043104b1f5d35","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        algo \u003d resp.sysmeta_headers.get(sysmeta_header("},{"line_number":212,"context_line":"            \u0027object\u0027, \u0027checksum-algorithm\u0027), \u0027\u0027)"},{"line_number":213,"context_line":"        req_checksums \u003d ["},{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"}],"source_content_type":"text/x-python","patch_set":30,"id":"8b0081b1_1404b2a6","line":213,"in_reply_to":"a674c62a_1957013f","updated":"2024-12-05 16:37:36.000000000","message":"Done","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"},{"line_number":217,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":218,"context_line":"            raise InvalidRequest("},{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"}],"source_content_type":"text/x-python","patch_set":30,"id":"27ca50d4_2a25da6e","line":217,"updated":"2024-10-09 18:31:52.000000000","message":"This o/p is used to consume 1 byte form the input stream. Can we ensure that this doesn\u0027t interfere w/t other parts of the request processing ?\n\nNote to self, this code block also handles the situation for a missing algorithm.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b562b700adf4727454d633f4c6eb8ecc7527d7e5","unresolved":true,"context_lines":[{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"},{"line_number":217,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":218,"context_line":"            raise InvalidRequest("},{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"}],"source_content_type":"text/x-python","patch_set":30,"id":"55bd6f40_c0aa4e82","line":217,"in_reply_to":"27ca50d4_2a25da6e","updated":"2024-11-15 20:05:05.000000000","message":"So it *solves* some issues with request processing -- I saw boto3 running into trouble if we respond too quickly. I *think* it comes down to requests/urllib3 not handling immediate errors properly in response to requests with `Expect: 100-continue` -- they think they can continue using the connection without sending the bytes that [eventlet wants to discard](https://github.com/eventlet/eventlet/blob/0.37.0/eventlet/wsgi.py#L667), so they eventually send a request then hang waiting on a response that will never come. By doing the read, we get eventlet to [send the `100 Continue`](https://github.com/eventlet/eventlet/blob/0.37.0/eventlet/wsgi.py#L140-L143), so the client commits to sending the body.\n\nWe should revisit https://review.opendev.org/c/openstack/swift/+/799866 -- we should be able to send this response *and close the connection* so it can\u0027t be re-used.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"dd3b8409c0341a9a54f238097653aa4445b98128","unresolved":true,"context_lines":[{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"},{"line_number":217,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":218,"context_line":"            raise InvalidRequest("},{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"}],"source_content_type":"text/x-python","patch_set":30,"id":"c1f2015b_3d0811cb","line":217,"in_reply_to":"55bd6f40_c0aa4e82","updated":"2024-12-02 22:02:11.000000000","message":"Thanks for the detailed response, would you like me to revive https://review.opendev.org/c/openstack/swift/+/799866?","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"23d7d06e2c7d289b7e35a2866411aa2d0178f5f6","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            h.lower()[len(\u0027x-amz-checksum-\u0027):] for h in req.headers"},{"line_number":215,"context_line":"            if h.lower().startswith(\u0027x-amz-checksum-\u0027)]"},{"line_number":216,"context_line":"        if req_checksums and not algo:"},{"line_number":217,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":218,"context_line":"            raise InvalidRequest("},{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"}],"source_content_type":"text/x-python","patch_set":30,"id":"a6a19d90_70437bcd","line":217,"in_reply_to":"c1f2015b_3d0811cb","updated":"2024-12-18 17:01:45.000000000","message":"We\u0027ll get to it \"eventually\" I suppose.\n\nAdded a comment about why the `read(1)` is necessary.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[{"line_number":820,"context_line":"                            \u0027match what we received.\u0027"},{"line_number":821,"context_line":"                            % (algo, part_number))"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"                    checksums.append((part_number, etag, part_chksums[algo]))"},{"line_number":824,"context_line":"                    chksum.update(strict_b64decode(part_chksums[algo]))"},{"line_number":825,"context_line":"                elif part_chksums:"},{"line_number":826,"context_line":"                    raise InvalidPart(upload_id\u003dupload_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"612a3f78_1cfd9ae0","line":823,"updated":"2024-10-09 18:31:52.000000000","message":"should this also be in a try block with something like:\n```\ntry:\n        decoded_value \u003d strict_b64decode(part_chksums[algo])\n        chksum.update(decoded_value)\n    except (binascii.Error, ValueError):\n        raise InvalidArgument(\u0027Checksum\u0027, part_chksums[algo], \u0027Invalid Base64 string\u0027)\n    except KeyError as e:\n        raise InvalidRequest(f\"Missing checksum for algorithm: {algo}\")\n    except Exception as e:\n        raise InvalidRequest(f\"Checksum update failed: {str(e)}\")\n    \n    checksums.append((part_number, etag, part_chksums[algo]))\n\nelif part_chksums:\n    raise InvalidPart(upload_id\u003dupload_id,\n                      part_number\u003dpart_number,\n                      e_tag\u003detag)\n```\n\nThis could help the code be more robust and handling potential errors ? (note to self look into the correct errors)","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b562b700adf4727454d633f4c6eb8ecc7527d7e5","unresolved":true,"context_lines":[{"line_number":820,"context_line":"                            \u0027match what we received.\u0027"},{"line_number":821,"context_line":"                            % (algo, part_number))"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"                    checksums.append((part_number, etag, part_chksums[algo]))"},{"line_number":824,"context_line":"                    chksum.update(strict_b64decode(part_chksums[algo]))"},{"line_number":825,"context_line":"                elif part_chksums:"},{"line_number":826,"context_line":"                    raise InvalidPart(upload_id\u003dupload_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"894068be_58341a85","line":823,"in_reply_to":"612a3f78_1cfd9ae0","updated":"2024-11-15 20:05:05.000000000","message":"We already know `algo` is in `part_chksums`, or we would have raised `BadDigest` just above on L818.\n\nWe already know `part_chksums[algo]` is valid base64 because we would have raised `InvalidArgument` if *any* value in `part_chksums` was invalid at L802.\n\n`binascii.Error` is a subclass of `ValueError`, so we shouldn\u0027t need to check for both.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"dd3b8409c0341a9a54f238097653aa4445b98128","unresolved":false,"context_lines":[{"line_number":820,"context_line":"                            \u0027match what we received.\u0027"},{"line_number":821,"context_line":"                            % (algo, part_number))"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"                    checksums.append((part_number, etag, part_chksums[algo]))"},{"line_number":824,"context_line":"                    chksum.update(strict_b64decode(part_chksums[algo]))"},{"line_number":825,"context_line":"                elif part_chksums:"},{"line_number":826,"context_line":"                    raise InvalidPart(upload_id\u003dupload_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"245dbc61_70412da6","line":823,"in_reply_to":"894068be_58341a85","updated":"2024-12-02 22:02:11.000000000","message":"Acknowledged","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"25da4a6677e41566292d528ba09fbd50d07c3c7d","unresolved":true,"context_lines":[{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"},{"line_number":221,"context_line":"        req_checksums.append(\u0027null\u0027)"},{"line_number":222,"context_line":"        if algo and \u0027x-amz-checksum-\u0027 + algo not in req.headers:"},{"line_number":223,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":224,"context_line":"            raise InvalidRequest("},{"line_number":225,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"d15601b9_0de1d285","line":222,"updated":"2024-12-18 02:53:40.000000000","message":"There is no need to go check in headers again here, we just processed these into req_checksums. so we should be able to get away with:\n\n```\nif algo and algo not in req_checksums\n```","commit_id":"45f99ca364908b132c818a48503848b0d8b01181"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"23d7d06e2c7d289b7e35a2866411aa2d0178f5f6","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"},{"line_number":220,"context_line":"                \u0027null, actual checksum Type: %s\u0027 % (req_checksums[0]))"},{"line_number":221,"context_line":"        req_checksums.append(\u0027null\u0027)"},{"line_number":222,"context_line":"        if algo and \u0027x-amz-checksum-\u0027 + algo not in req.headers:"},{"line_number":223,"context_line":"            req.environ[\u0027wsgi.input\u0027].read(1)"},{"line_number":224,"context_line":"            raise InvalidRequest("},{"line_number":225,"context_line":"                \u0027Checksum Type mismatch occurred, expected checksum Type: \u0027"}],"source_content_type":"text/x-python","patch_set":38,"id":"95110ebf_1c435225","line":222,"in_reply_to":"d15601b9_0de1d285","updated":"2024-12-18 17:01:45.000000000","message":"Good call -- I also found I could combine these two `Checksum Type mismatch occurred` cases pretty easily.","commit_id":"45f99ca364908b132c818a48503848b0d8b01181"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"25da4a6677e41566292d528ba09fbd50d07c3c7d","unresolved":true,"context_lines":[{"line_number":480,"context_line":"            if algo not in checksum_to_client_name:"},{"line_number":481,"context_line":"                raise InvalidRequest("},{"line_number":482,"context_line":"                    \u0027Checksum algorithm provided is unsupported. Please \u0027"},{"line_number":483,"context_line":"                    \u0027try again with any of the valid types: [CRC32, \u0027"},{"line_number":484,"context_line":"                    \u0027CRC32C, SHA1, SHA256]\u0027)"},{"line_number":485,"context_line":"            req.headers[sysmeta_header(\u0027object\u0027, \u0027checksum-algorithm\u0027)] \u003d algo"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":38,"id":"8399631f_e575dfaa","line":484,"range":{"start_line":483,"start_character":61,"end_line":484,"end_character":42},"updated":"2024-12-18 02:53:40.000000000","message":"While we have the checksum_to_client_name at hand, do we want to dynamically add them to the list here, so if we ever add support for a new algorithm we don\u0027t have to remember to change this text? Just the thought, not a blocker.","commit_id":"45f99ca364908b132c818a48503848b0d8b01181"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"20e70376a3827278d8725c1fc41f62b67dbf9ad3","unresolved":false,"context_lines":[{"line_number":480,"context_line":"            if algo not in checksum_to_client_name:"},{"line_number":481,"context_line":"                raise InvalidRequest("},{"line_number":482,"context_line":"                    \u0027Checksum algorithm provided is unsupported. Please \u0027"},{"line_number":483,"context_line":"                    \u0027try again with any of the valid types: [CRC32, \u0027"},{"line_number":484,"context_line":"                    \u0027CRC32C, SHA1, SHA256]\u0027)"},{"line_number":485,"context_line":"            req.headers[sysmeta_header(\u0027object\u0027, \u0027checksum-algorithm\u0027)] \u003d algo"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":38,"id":"89db6cc6_6817aabc","line":484,"range":{"start_line":483,"start_character":61,"end_line":484,"end_character":42},"in_reply_to":"001a1792_7b2e2bbd","updated":"2024-12-18 22:04:07.000000000","message":"Acknowledged","commit_id":"45f99ca364908b132c818a48503848b0d8b01181"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"23d7d06e2c7d289b7e35a2866411aa2d0178f5f6","unresolved":true,"context_lines":[{"line_number":480,"context_line":"            if algo not in checksum_to_client_name:"},{"line_number":481,"context_line":"                raise InvalidRequest("},{"line_number":482,"context_line":"                    \u0027Checksum algorithm provided is unsupported. Please \u0027"},{"line_number":483,"context_line":"                    \u0027try again with any of the valid types: [CRC32, \u0027"},{"line_number":484,"context_line":"                    \u0027CRC32C, SHA1, SHA256]\u0027)"},{"line_number":485,"context_line":"            req.headers[sysmeta_header(\u0027object\u0027, \u0027checksum-algorithm\u0027)] \u003d algo"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":38,"id":"001a1792_7b2e2bbd","line":484,"range":{"start_line":483,"start_character":61,"end_line":484,"end_character":42},"in_reply_to":"8399631f_e575dfaa","updated":"2024-12-18 17:01:45.000000000","message":"I\u0027m inclined to leave it as-is, since it matches how AWS responds (see `test_mpu_create_bad_checksum_algorithm`).","commit_id":"45f99ca364908b132c818a48503848b0d8b01181"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e0fbe4fd38db1d10e07ef0a48f41622516c4e60","unresolved":true,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":"        algo \u003d req.headers.get(\u0027x-amz-checksum-algorithm\u0027, \u0027\u0027).lower()"},{"line_number":468,"context_line":"        if algo:"},{"line_number":469,"context_line":"            if \u0027,\u0027 in algo:"},{"line_number":470,"context_line":"                raise InvalidRequest("},{"line_number":471,"context_line":"                    \u0027Invalid types are specified in \u0027"},{"line_number":472,"context_line":"                    \u0027x-amz-checksum-algorithm header.\u0027)"}],"source_content_type":"text/x-python","patch_set":41,"id":"696e707d_dc762ae8","line":469,"updated":"2025-01-22 14:42:39.000000000","message":"there\u0027s similar validation in ChecksummingInput, but not quite the same - ChecksummingInput reads a byte before raising the error, and the error response bodies are different\n\nCan these checks be encapsulated in one place and re-used?\n\nAlso in S3Request._validate_headers there is cross-checks w.r.t. ``x-amz-sdk-checksum-algorithm`` for PUTs - should the same be here? Again, can it all be encapuslated in one place?","commit_id":"1ba3ff2398a451782839aa68634c34dda3d6529c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e0fbe4fd38db1d10e07ef0a48f41622516c4e60","unresolved":true,"context_lines":[{"line_number":474,"context_line":"                raise InvalidRequest("},{"line_number":475,"context_line":"                    \u0027Checksum algorithm provided is unsupported. Please \u0027"},{"line_number":476,"context_line":"                    \u0027try again with any of the valid types: [CRC32, \u0027"},{"line_number":477,"context_line":"                    \u0027CRC32C, CRC64NVME, SHA1, SHA256]\u0027)"},{"line_number":478,"context_line":"            if checksum_to_hasher[self._checksum_name] is None:"},{"line_number":479,"context_line":"                raise S3NotImplemented(\u0027The %s checksum is not available\u0027"},{"line_number":480,"context_line":"                                       % self._checksum_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"fe4b9b2e_20461479","line":477,"updated":"2025-01-22 14:42:39.000000000","message":"but depending on the platform there\u0027s no guarantee that all of the algorithms are supported e.g. CRC64NVME with older or no isa-l, so the client could retry then hit the next error response. Shouldn\u0027t the list be of only those algo\u0027s that are supported?","commit_id":"1ba3ff2398a451782839aa68634c34dda3d6529c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e0fbe4fd38db1d10e07ef0a48f41622516c4e60","unresolved":true,"context_lines":[{"line_number":475,"context_line":"                    \u0027Checksum algorithm provided is unsupported. Please \u0027"},{"line_number":476,"context_line":"                    \u0027try again with any of the valid types: [CRC32, \u0027"},{"line_number":477,"context_line":"                    \u0027CRC32C, CRC64NVME, SHA1, SHA256]\u0027)"},{"line_number":478,"context_line":"            if checksum_to_hasher[self._checksum_name] is None:"},{"line_number":479,"context_line":"                raise S3NotImplemented(\u0027The %s checksum is not available\u0027"},{"line_number":480,"context_line":"                                       % self._checksum_name)"},{"line_number":481,"context_line":"            req.headers[sysmeta_header(\u0027object\u0027, \u0027checksum-algorithm\u0027)] \u003d algo"}],"source_content_type":"text/x-python","patch_set":41,"id":"233b703c_f404ac15","line":478,"range":{"start_line":478,"start_character":34,"end_line":478,"end_character":53},"updated":"2025-01-22 14:42:39.000000000","message":"``self._checksum_header`` isn\u0027t a thing","commit_id":"1ba3ff2398a451782839aa68634c34dda3d6529c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e0fbe4fd38db1d10e07ef0a48f41622516c4e60","unresolved":true,"context_lines":[{"line_number":478,"context_line":"            if checksum_to_hasher[self._checksum_name] is None:"},{"line_number":479,"context_line":"                raise S3NotImplemented(\u0027The %s checksum is not available\u0027"},{"line_number":480,"context_line":"                                       % self._checksum_name)"},{"line_number":481,"context_line":"            req.headers[sysmeta_header(\u0027object\u0027, \u0027checksum-algorithm\u0027)] \u003d algo"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"        try:"},{"line_number":484,"context_line":"            seg_req \u003d copy.copy(req)"}],"source_content_type":"text/x-python","patch_set":41,"id":"0fb01b59_0f806a02","line":481,"updated":"2025-01-22 14:42:39.000000000","message":"this clearly needs some unit test coverage - at least on zuul there is one failing probe test, but on my vsaio even the probe test passes presumably because boto3 is older ?? ok, so the cross-compat tests fail too.\n\nhttps://b44627d9bd531a273822-f9df0d57f58290ba7ec7037bd79eda07.ssl.cf5.rackcdn.com/909802/41/check/swift-probetests-centos-9-stream/1ba3d48/proxy.error\n\n```\nJan 17 22:26:02 np0039606514 proxy-server[28169]: \u0027UploadsController\u0027 object has no attribute \u0027_checksum_name\u0027: #012Traceback (most recent call last):#012  File \"/home/zuul/src/opendev.org/openstack/swift/swift/common/middleware/s3api/s3api.py\", line 359, in __call__#012    resp \u003d self.handle_request(req)#012  File \"/home/zuul/src/opendev.org/openstack/swift/swift/common/middleware/s3api/s3api.py\", line 400, in handle_request#012    res \u003d handler(req)#012  File \"/home/zuul/src/opendev.org/openstack/swift/swift/common/middleware/s3api/controllers/base.py\", line 60, in wrapped#012    return func(self, req)#012  File \"/home/zuul/src/opendev.org/openstack/swift/swift/common/middleware/s3api/controllers/base.py\", line 72, in check_container#012    return func(self, req)#012  File \"/home/zuul/src/opendev.org/openstack/swift/swift/common/middleware/s3api/controllers/multi_upload.py\", line 478, in POST#012    if checksum_to_hasher[self._checksum_name] is None:#012AttributeError: \u0027UploadsController\u0027 object has no attribute \u0027_checksum_name\u0027 (txn: tx92b1f0e44e7842d286c64-00678ad8fa) (client_ip: 127.0.0.1)\n```","commit_id":"1ba3ff2398a451782839aa68634c34dda3d6529c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e0fbe4fd38db1d10e07ef0a48f41622516c4e60","unresolved":true,"context_lines":[{"line_number":272,"context_line":"                                       req_timestamp.s3xmlformat)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        resp.status \u003d 200"},{"line_number":275,"context_line":"        return resp"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"class UploadsController(Controller):"}],"source_content_type":"text/x-python","patch_set":42,"id":"caaeb6c5_e768da10","line":275,"updated":"2025-01-22 14:42:39.000000000","message":"the test_mpu.py probe test fails because the completeUpload does not include the checksum in the parts manifest which it cannot do because the checksum is not returned with the uploadPart response. I hacked it up to pass, but I think aa more general solution is needed to return the checksum header with any PUT response:\n\n```\ndiff --git a/swift/common/middleware/s3api/controllers/multi_upload.py b/swift/common/middleware/s3api/controllers/multi_upload.py\nindex ccd811626..d6ab8b5f9 100644\n--- a/swift/common/middleware/s3api/controllers/multi_upload.py\n+++ b/swift/common/middleware/s3api/controllers/multi_upload.py\n@@ -272,6 +272,8 @@ class PartController(Controller):\n                                        req_timestamp.s3xmlformat)\n\n         resp.status \u003d 200\n+        # TODO: needs to adapt to whatever algorithm was used in req\n+        resp.headers[\u0027x-amz-checksum-crc32\u0027] \u003d req.headers[\u0027x-amz-checksum-crc32\u0027]\n         return resp\n\n\ndiff --git a/test/probe/test_mpu.py b/test/probe/test_mpu.py\nindex ed6944bc8..905ef0271 100644\n--- a/test/probe/test_mpu.py\n+++ b/test/probe/test_mpu.py\n@@ -133,6 +133,7 @@ class TestMixedPolicyMPU(ReplProbeTest):\n             u\u0027Size\u0027: expected_size,\n             u\u0027Owner\u0027: {u\u0027DisplayName\u0027: \u0027test:tester\u0027, u\u0027ID\u0027: \u0027test:tester\u0027},\n             u\u0027StorageClass\u0027: \u0027STANDARD\u0027,\n+            u\u0027ChecksumAlgorithm\u0027: [\u0027CRC32\u0027],\n         }], resp[\u0027Contents\u0027])\n\n         # swift listing is the same\n@@ -147,6 +148,7 @@ class TestMixedPolicyMPU(ReplProbeTest):\n             \u0027name\u0027: self.mpu_name,\n             \u0027s3_etag\u0027: s3_head_resp[\u0027ETag\u0027],\n             \u0027slo_etag\u0027: swift_obj_headers[\u0027etag\u0027],\n+            \u0027s3_crc32\u0027: \u0027VdI8SA\u003d\u003d-3\u0027,\n         }])\n         # check segments\n         stat, listing \u003d swiftclient.get_container(\n```","commit_id":"45d665c488f44d9898dfc2b776a0cc319dc9a618"}],"test/s3api/test_object_checksums.py":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"3adb9b0b05bbb4ddc4e33eb0f12d315a3ae36d30","unresolved":true,"context_lines":[{"line_number":777,"context_line":"            \u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":778,"context_line":"        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {"},{"line_number":779,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":780,"context_line":"            \u0027Message\u0027: (\u0027The crc32c you specified for part 1 did not match \u0027"},{"line_number":781,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":782,"context_line":"            # but... we *didn\u0027t* specify the crc32c..."},{"line_number":783,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a64aa90_32dd38b7","line":780,"updated":"2024-10-09 18:31:52.000000000","message":"nit: the correct assert statement in this case should be:\n```\n        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {\n            \u0027Code\u0027: \u0027BadDigest\u0027,\n            \u0027Message\u0027: (\u0027The crc32 you specified for part 1 did not match \u0027\n                        \u0027what we received.\u0027),\n            # but... we *didn\u0027t* specify the crc32c...\n        })\n```\n\nwe have `test_multipart_mpu` failing against swift but not against a pure aws deployment when i make the above change and when i let the patch stay in its original state; the test passes against my vsaio but not against AWS\n\nThis means somewhere in our error reporting we are going wrong, i am gonna attempt to locate the right spot so as to answer the \"where exactly\", but the above suggestions is the correct assertion since we wan to be S3 compatible.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"48ccdff764ea4ae8b41b2df241eea89fb2ecd3a2","unresolved":true,"context_lines":[{"line_number":777,"context_line":"            \u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":778,"context_line":"        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {"},{"line_number":779,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":780,"context_line":"            \u0027Message\u0027: (\u0027The crc32c you specified for part 1 did not match \u0027"},{"line_number":781,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":782,"context_line":"            # but... we *didn\u0027t* specify the crc32c..."},{"line_number":783,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":30,"id":"7bf51042_5e619871","line":780,"in_reply_to":"1a64aa90_32dd38b7","updated":"2024-10-16 23:15:01.000000000","message":"By the time we reach L776, we have the following parts:\n\n```\n[{\u0027ChecksumCRC32\u0027: \u0027yTuzdQ\u003d\u003d\u0027, \u0027ETag\u0027: \u0027\"5f363e0e58a95f06cbe9bbc662c5dfb6\"\u0027, \u0027PartNumber\u0027: 1}, \n{\u0027ChecksumCRC32C\u0027: \u0027zqsFQg\u003d\u003d\u0027, \u0027ETag\u0027: \u0027\"5f363e0e58a95f06cbe9bbc662c5dfb6\"\u0027, \u0027PartNumber\u0027: 2}]\n```\n\nand since we have `crc32` for PartNumber\u003d1 and `crc32c` for PartNumber\u003d2, that is what substantiates my reasoning behind the assert statement being what it should be, although **this is a bad complete mpu repsonse with both parts having different checksum algos**, so i am unsure which part should take precedence?!\n\nAt the very least from what i can tell i guess the *AWS* behavior is to default to the checksum used in the first part and swift for now is reporting the checksum of the last part upload, computationally speaking we will have to compare the lookup times for either. I do not mind diverging from the default aws behaviour as long as we are faster!!","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"e13dc66aafabd5ec958aba21f323a039aebfe144","unresolved":true,"context_lines":[{"line_number":777,"context_line":"            \u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":778,"context_line":"        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {"},{"line_number":779,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":780,"context_line":"            \u0027Message\u0027: (\u0027The crc32c you specified for part 1 did not match \u0027"},{"line_number":781,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":782,"context_line":"            # but... we *didn\u0027t* specify the crc32c..."},{"line_number":783,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":30,"id":"d16b18ab_f59299ed","line":780,"in_reply_to":"7bf51042_5e619871","updated":"2024-10-24 20:57:08.000000000","message":"@tburke@nvidia.com thoughts ?","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"dd3b8409c0341a9a54f238097653aa4445b98128","unresolved":false,"context_lines":[{"line_number":777,"context_line":"            \u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":778,"context_line":"        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {"},{"line_number":779,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":780,"context_line":"            \u0027Message\u0027: (\u0027The crc32c you specified for part 1 did not match \u0027"},{"line_number":781,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":782,"context_line":"            # but... we *didn\u0027t* specify the crc32c..."},{"line_number":783,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":30,"id":"f3daac03_464ed0dd","line":780,"in_reply_to":"9085594e_46cbc1fe","updated":"2024-12-02 22:02:11.000000000","message":"Done","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b562b700adf4727454d633f4c6eb8ecc7527d7e5","unresolved":true,"context_lines":[{"line_number":777,"context_line":"            \u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":778,"context_line":"        self.assertEqual(bad_complete_resp[\u0027Error\u0027], {"},{"line_number":779,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":780,"context_line":"            \u0027Message\u0027: (\u0027The crc32c you specified for part 1 did not match \u0027"},{"line_number":781,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":782,"context_line":"            # but... we *didn\u0027t* specify the crc32c..."},{"line_number":783,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":30,"id":"9085594e_46cbc1fe","line":780,"in_reply_to":"d16b18ab_f59299ed","updated":"2024-11-15 20:05:05.000000000","message":"Fixed, though the test re-org doesn\u0027t make it simple to see. I think I must\u0027ve lost track of which window was running against s3api vs AWS and tried to assert on our behavior instead of AWS\u0027s.","commit_id":"2edfce072ac98fdcfb29c01d689a67f70206a252"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b562b700adf4727454d633f4c6eb8ecc7527d7e5","unresolved":true,"context_lines":[{"line_number":1020,"context_line":"                         resp)"},{"line_number":1021,"context_line":"        self.assertEqual(resp[\u0027Error\u0027], {"},{"line_number":1022,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":1023,"context_line":"            \u0027Message\u0027: (\u0027The crc32 you specified for part 2 did not match \u0027"},{"line_number":1024,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":1025,"context_line":"            # But it was valid! The server just wasn\u0027t tracking it"},{"line_number":1026,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":34,"id":"cfe12656_5a279b0e","line":1023,"updated":"2024-11-15 20:05:05.000000000","message":"Here\u0027s the mixed-checksum message.","commit_id":"e8748821a97402f18759df967d03df2863548cea"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"dd3b8409c0341a9a54f238097653aa4445b98128","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"                         resp)"},{"line_number":1021,"context_line":"        self.assertEqual(resp[\u0027Error\u0027], {"},{"line_number":1022,"context_line":"            \u0027Code\u0027: \u0027BadDigest\u0027,"},{"line_number":1023,"context_line":"            \u0027Message\u0027: (\u0027The crc32 you specified for part 2 did not match \u0027"},{"line_number":1024,"context_line":"                        \u0027what we received.\u0027),"},{"line_number":1025,"context_line":"            # But it was valid! The server just wasn\u0027t tracking it"},{"line_number":1026,"context_line":"        })"}],"source_content_type":"text/x-python","patch_set":34,"id":"5a5b02b8_dcc0bcb0","line":1023,"in_reply_to":"cfe12656_5a279b0e","updated":"2024-12-02 22:02:11.000000000","message":"Done","commit_id":"e8748821a97402f18759df967d03df2863548cea"}]}
