fix the checksum array-list

awslabs · Jun 19, 2023 · c106b0b · c106b0b
1 parent cabcef7
commit c106b0b
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 22 deletions.
diff --git a/include/aws/s3/private/s3_auto_ranged_put.h b/include/aws/s3/private/s3_auto_ranged_put.h
@@ -65,8 +65,11 @@ struct aws_s3_auto_ranged_put {
         /* Array list of `struct aws_string *`. */
         struct aws_array_list etag_list;
 
-        /* Very similar to the etag_list used in complete_multipart_upload to create the XML payload. Each part will set
-         * the corresponding index to its checksum result. */
+        /**
+         * Array list of `struct aws_byte_buf *`.
+         * Very similar to the etag_list used in complete_multipart_upload to create the XML payload. Each part will set
+         * the corresponding index to its checksum result.
+         **/
         struct aws_array_list encoded_checksum_list;
 
         struct aws_s3_paginated_operation *list_parts_operation;

diff --git a/source/s3_auto_ranged_put.c b/source/s3_auto_ranged_put.c
@@ -151,7 +151,7 @@ static bool s_process_part_info(const struct aws_s3_part_info *info, void *user_
      * element make sure to init any new elements to zeroed values. */
     size_t current_num_parts = aws_array_list_length(&auto_ranged_put->synced_data.etag_list);
     if (info->part_number > current_num_parts) {
-        struct aws_byte_buf empty_buf = {0};
+        struct aws_byte_buf *empty_buf = NULL;
         struct aws_string *null_etag = NULL;
 
         /* Note: using 1 based part nums here to avoid dealing with underflow of
@@ -163,8 +163,9 @@ static bool s_process_part_info(const struct aws_s3_part_info *info, void *user_
     }
 
     if (checksum_cur) {
-        struct aws_byte_buf checksum_buf;
-        aws_byte_buf_init_copy_from_cursor(&checksum_buf, auto_ranged_put->base.allocator, *checksum_cur);
+        struct aws_byte_buf *checksum_buf =
+            aws_mem_acquire(auto_ranged_put->base.allocator, sizeof(struct aws_byte_buf));
+        aws_byte_buf_init_copy_from_cursor(checksum_buf, auto_ranged_put->base.allocator, *checksum_cur);
         aws_array_list_set_at(
             &auto_ranged_put->synced_data.encoded_checksum_list, &checksum_buf, info->part_number - 1);
     }
@@ -387,7 +388,10 @@ struct aws_s3_meta_request *aws_s3_meta_request_auto_ranged_put_new(
     aws_array_list_init_dynamic(
         &auto_ranged_put->synced_data.etag_list, allocator, initial_num_parts, sizeof(struct aws_string *));
     aws_array_list_init_dynamic(
-        &auto_ranged_put->synced_data.encoded_checksum_list, allocator, initial_num_parts, sizeof(struct aws_byte_buf));
+        &auto_ranged_put->synced_data.encoded_checksum_list,
+        allocator,
+        initial_num_parts,
+        sizeof(struct aws_byte_buf *));
 
     if (s_try_init_resume_state_from_persisted_data(allocator, auto_ranged_put, options->resume_token)) {
         goto error_clean_up;
@@ -431,10 +435,11 @@ static void s_s3_meta_request_auto_ranged_put_destroy(struct aws_s3_meta_request
          checksum_index < aws_array_list_length(&auto_ranged_put->synced_data.encoded_checksum_list);
          ++checksum_index) {
 
-        struct aws_byte_buf checksum_buf;
+        struct aws_byte_buf *checksum_buf;
         aws_array_list_get_at(&auto_ranged_put->synced_data.encoded_checksum_list, &checksum_buf, checksum_index);
 
-        aws_byte_buf_clean_up(&checksum_buf);
+        aws_byte_buf_clean_up(checksum_buf);
+        aws_mem_release(meta_request->allocator, checksum_buf);
     }
     aws_array_list_clean_up(&auto_ranged_put->synced_data.etag_list);
     aws_array_list_clean_up(&auto_ranged_put->synced_data.encoded_checksum_list);
@@ -748,7 +753,7 @@ static int s_verify_part_matches_checksum(
     struct aws_allocator *allocator,
     struct aws_byte_buf part_body,
     enum aws_s3_checksum_algorithm algorithm,
-    struct aws_byte_buf part_checksum) {
+    struct aws_byte_buf *part_checksum) {
     AWS_PRECONDITION(allocator);
 
     if (algorithm == AWS_SCA_NONE) {
@@ -794,7 +799,7 @@ static int s_verify_part_matches_checksum(
         goto on_done;
     }
 
-    if (!aws_byte_buf_eq(&encoded_checksum, &part_checksum)) {
+    if (!aws_byte_buf_eq(&encoded_checksum, part_checksum)) {
         AWS_LOGF_ERROR(
             AWS_LS_S3_META_REQUEST, "Failed to resume upload. Checksum for previously uploaded part does not match");
         return_status = aws_raise_error(AWS_ERROR_S3_RESUMED_PART_CHECKSUM_MISMATCH);
@@ -936,15 +941,15 @@ static void s_skip_parts_from_stream_loop(void *user_data) {
             goto on_done;
         }
 
-        struct aws_byte_buf checksum_buf;
+        struct aws_byte_buf *checksum_buf;
         aws_array_list_get_at(&auto_ranged_put->synced_data.encoded_checksum_list, &checksum_buf, skip_job->part_index);
 
         // compare skipped checksum to previously uploaded checksum
-        if (checksum_buf.len > 0 && s_verify_part_matches_checksum(
-                                        meta_request->allocator,
-                                        *temp_body_buf,
-                                        meta_request->checksum_config.checksum_algorithm,
-                                        checksum_buf)) {
+        if (checksum_buf->len > 0 && s_verify_part_matches_checksum(
+                                         meta_request->allocator,
+                                         *temp_body_buf,
+                                         meta_request->checksum_config.checksum_algorithm,
+                                         checksum_buf)) {
             error_code = aws_last_error_or_unknown();
             goto on_done;
         }
@@ -1185,7 +1190,7 @@ static void s_s3_prepare_upload_part_on_read_done(void *user_data) {
              * Note: During resume flow this might cause the values to be
              * reset twice (if we are preparing part in between
              * previously completed parts). */
-            struct aws_byte_buf checksum_buf = {0};
+            struct aws_byte_buf *checksum_buf = aws_mem_calloc(meta_request->allocator, 1, sizeof(struct aws_byte_buf));
             aws_array_list_set_at(
                 &auto_ranged_put->synced_data.encoded_checksum_list, &checksum_buf, request->part_number - 1);
 
@@ -1231,8 +1236,8 @@ static void s_s3_prepare_upload_part_finish(struct aws_s3_prepare_upload_part_jo
             aws_string_c_str(auto_ranged_put->upload_id));
 
     } else {
-        aws_array_list_get_at_ptr(
-            &auto_ranged_put->synced_data.encoded_checksum_list, (void **)&checksum_buf, request->part_number - 1);
+        aws_array_list_get_at(
+            &auto_ranged_put->synced_data.encoded_checksum_list, &checksum_buf, request->part_number - 1);
         /* Clean up the buffer in case of it's initialized before and retry happens. */
         aws_byte_buf_clean_up(checksum_buf);
 

diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c
@@ -643,11 +643,11 @@ struct aws_http_message *aws_s3_complete_multipart_message_new(
             }
 
             if (mpu_algorithm_checksum_name) {
-                struct aws_byte_buf checksum_buf;
+                struct aws_byte_buf *checksum_buf;
 
                 aws_array_list_get_at(checksums, &checksum_buf, etag_index);
 
-                struct aws_byte_cursor checksum = aws_byte_cursor_from_buf(&checksum_buf);
+                struct aws_byte_cursor checksum = aws_byte_cursor_from_buf(checksum_buf);
 
                 if (aws_byte_buf_append_dynamic(body_buffer, &s_open_start_bracket)) {
                     goto error_clean_up;

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -108,6 +108,7 @@ add_net_test_case(test_s3_create_multipart_upload_message_with_content_md5)
 add_net_test_case(test_s3_complete_multipart_message_with_content_md5)
 add_net_test_case(test_s3_put_object_double_slashes)
 add_net_test_case(test_s3_put_object_no_content_length)
+add_net_test_case(test_s3_put_large_object_no_content_length_with_checksum)
 add_net_test_case(test_s3_put_object_single_part_no_content_length)
 add_net_test_case(test_s3_put_object_zero_size_no_content_length)
 add_net_test_case(test_s3_put_object_async_singlepart)

diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c
@@ -2153,6 +2153,59 @@ static int s_test_s3_put_object_no_content_length(struct aws_allocator *allocato
     return 0;
 }
 
+AWS_TEST_CASE(
+    test_s3_put_large_object_no_content_length_with_checksum,
+    s_test_s3_put_large_object_no_content_length_with_checksum)
+static int s_test_s3_put_large_object_no_content_length_with_checksum(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+    /* Use Debug level log for large object by default. */
+    struct aws_logger_standard_options logger_options = {
+        .level = AWS_LOG_LEVEL_DEBUG,
+        .file = stderr,
+    };
+
+    aws_logger_init_standard(&s_logger, allocator, &logger_options);
+    aws_logger_set(&s_logger);
+
+    struct aws_s3_client_config client_config = {
+        .part_size = MB_TO_BYTES(8),
+    };
+
+    ASSERT_SUCCESS(aws_s3_tester_bind_client(
+        &tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION | AWS_S3_TESTER_BIND_CLIENT_SIGNING));
+
+    struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config);
+
+    ASSERT_TRUE(client != NULL);
+
+    struct aws_s3_tester_meta_request_options put_options = {
+        .allocator = allocator,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
+        .client = client,
+        .checksum_algorithm = AWS_SCA_CRC32,
+        .put_options =
+            {
+                .object_size_mb = 1280,
+                .skip_content_length = true,
+            },
+    };
+    struct aws_s3_meta_request_test_results meta_request_test_results;
+    aws_s3_meta_request_test_results_init(&meta_request_test_results, allocator);
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &meta_request_test_results));
+    aws_s3_meta_request_test_results_clean_up(&meta_request_test_results);
+
+    aws_s3_client_release(client);
+
+    aws_s3_tester_clean_up(&tester);
+    aws_logger_clean_up(&s_logger);
+
+    return 0;
+}
+
 AWS_TEST_CASE(test_s3_put_object_single_part_no_content_length, s_test_s3_put_object_single_part_no_content_length)
 static int s_test_s3_put_object_single_part_no_content_length(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;

diff --git a/tests/s3_tester.c b/tests/s3_tester.c
@@ -1392,7 +1392,7 @@ int aws_s3_tester_send_meta_request_with_options(
             meta_request_options.checksum_config = &checksum_config;
 
             uint32_t object_size_mb = options->put_options.object_size_mb;
-            size_t object_size_bytes = (size_t)object_size_mb * 1024ULL * 1024ULL;
+            size_t object_size_bytes = (size_t)object_size_mb * 1000ULL * 1000ULL;
 
             /* This doesn't do what we think it should because
              * g_min_upload_part_size overrides client->part_size */

diff --git a/tests/s3_tester.h b/tests/s3_tester.h
@@ -26,6 +26,9 @@ struct aws_event_loop_group;
 struct aws_host_resolver;
 struct aws_input_stream;
 
+/* To override the default logger incase of the trace log is just too long to read. */
+static struct aws_logger s_logger;
+
 enum AWS_S3_TESTER_BIND_CLIENT_FLAGS {
     AWS_S3_TESTER_BIND_CLIENT_REGION = 0x00000001,
     AWS_S3_TESTER_BIND_CLIENT_SIGNING = 0x00000002,