Fix: Get object with checksum leak when retry happens (#346)

Co-authored-by: Michael Graeb <[email protected]>
awslabs · Sep 7, 2023 · 3a30ba8 · 3a30ba8
1 parent a0ee6a9
commit 3a30ba8
Show file tree

Hide file tree

Showing 6 changed files with 235 additions and 124 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -6,7 +6,7 @@ on:
       - 'main'
 
 env:
-  BUILDER_VERSION: v0.9.43
+  BUILDER_VERSION: v0.9.48
   BUILDER_SOURCE: releases
   BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
   PACKAGE_NAME: aws-c-s3
@@ -16,6 +16,7 @@ env:
   AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
   AWS_DEFAULT_REGION: ${{ secrets.AWS_DEFAULT_REGION }}
   AWS_REGION: us-east-1
+  CTEST_PARALLEL_LEVEL: 2
 
 jobs:
   linux-compat:

diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c
@@ -982,12 +982,12 @@ static void s_get_response_part_finish_checksum_helper(struct aws_s3_connection
         request->validation_algorithm = request->request_level_running_response_sum->algorithm;
         aws_byte_buf_clean_up(&response_body_sum);
         aws_byte_buf_clean_up(&encoded_response_body_sum);
-        aws_checksum_destroy(request->request_level_running_response_sum);
-        aws_byte_buf_clean_up(&request->request_level_response_header_checksum);
-        request->request_level_running_response_sum = NULL;
     } else {
         request->did_validate = false;
     }
+    aws_checksum_destroy(request->request_level_running_response_sum);
+    aws_byte_buf_clean_up(&request->request_level_response_header_checksum);
+    request->request_level_running_response_sum = NULL;
 }
 
 static int s_s3_meta_request_incoming_headers(

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -254,6 +254,7 @@ add_net_test_case(test_s3_list_bucket_valid)
 if(ENABLE_MOCK_SERVER_TESTS)
     add_net_test_case(multipart_upload_mock_server)
     add_net_test_case(multipart_upload_checksum_with_retry_mock_server)
+    add_net_test_case(multipart_download_checksum_with_retry_mock_server)
     add_net_test_case(async_internal_error_from_complete_multipart_mock_server)
     add_net_test_case(async_access_denied_from_complete_multipart_mock_server)
     add_net_test_case(get_object_modified_mock_server)

diff --git a/tests/mock_s3_server/GetObject/get_object_checksum_retry.json b/tests/mock_s3_server/GetObject/get_object_checksum_retry.json
@@ -0,0 +1,12 @@
+{
+    "status": 200,
+    "headers": {
+      "ETag": "b54357faf0632cce46e942fa68356b38",
+      "Date": "Thu, 12 Jan 2023 00:04:21 GMT",
+      "Last-Modified": "Tue, 10 Jan 2023 23:39:32 GMT",
+      "Accept-Ranges": "bytes",
+      "Content-Range": "bytes 0-65535/65536",
+      "Content-Type": "binary/octet-stream",
+      "x-amz-checksum-crc32": "q1875w=="
+    }
+}