fix unknown checksum handling

include/aws/s3/private/s3_checksum_context.h

@@ -40,6 +40,8 @@ struct aws_s3_upload_request_checksum_context {
 
     /* Validation */
     size_t encoded_checksum_size;
+
+    bool has_review_callback;
 };
 
 /**
@@ -54,7 +56,8 @@ struct aws_s3_upload_request_checksum_context {
 AWS_S3_API
 struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_context_new(
     struct aws_allocator *allocator,
-    const struct aws_s3_meta_request_checksum_config_storage *checksum_config);
+    const struct aws_s3_meta_request_checksum_config_storage *checksum_config,
+    bool has_review_callback);
 
 /**
  * Create a new upload request checksum context with an existing base64 encoded checksum value.
@@ -70,7 +73,8 @@ AWS_S3_API
 struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(
     struct aws_allocator *allocator,
     const struct aws_s3_meta_request_checksum_config_storage *checksum_config,
-    struct aws_byte_cursor existing_base64_checksum);
+    struct aws_byte_cursor existing_base64_checksum,
+    bool has_review_callback);
 
 /**
  * Acquire a reference to the upload request checksum context.

include/aws/s3/private/s3_util.h

@@ -385,6 +385,14 @@ int aws_s3_calculate_request_optimal_range_size(
 AWS_S3_API
 int aws_s3_extract_parts_from_etag(struct aws_byte_cursor etag_header_value, uint32_t *out_num_parts);
 
+/**
+ * Helper to figure out if given header name is one of the checksum value headers.
+ * ex. returns true for x-amz-checksum-crc32 or x-amz-checksum-sha256, but false for x-amz-checksum-type.
+ * Note: relies on hardcoded list of non-checksum headers, which needs to be updated if s3 expands list of those.
+ */
+AWS_S3_API
+bool aws_s3_is_checksum_value_header_name(struct aws_byte_cursor header_name);
+
 AWS_EXTERN_C_END
 
 #endif /* AWS_S3_UTIL_H */

source/s3_auto_ranged_put.c

@@ -200,10 +200,15 @@ static int s_process_part_info_synced(const struct aws_s3_part_info *info, void
     if ((checksum_cur != NULL) && (checksum_cur->len > 0)) {
         /* Create checksum context with pre-calculated checksum */
         part->checksum_context = aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(
-            auto_ranged_put->base.allocator, &auto_ranged_put->base.checksum_config, *checksum_cur);
+            auto_ranged_put->base.allocator,
+            &auto_ranged_put->base.checksum_config,
+            *checksum_cur,
+            meta_request->upload_review_callback != NULL);
     } else {
         part->checksum_context = aws_s3_upload_request_checksum_context_new(
-            auto_ranged_put->base.allocator, &auto_ranged_put->base.checksum_config);
+            auto_ranged_put->base.allocator,
+            &auto_ranged_put->base.checksum_config,
+            meta_request->upload_review_callback != NULL);
     }
     if (part->checksum_context == NULL) {
         aws_mem_release(meta_request->allocator, part);
@@ -1072,8 +1077,8 @@ static int s_s3_new_upload_part_info_after_body(
         /* Add part to array-list */
         struct aws_s3_mpu_part_info *part =
             aws_mem_calloc(meta_request->allocator, 1, sizeof(struct aws_s3_mpu_part_info));
-        part->checksum_context =
-            aws_s3_upload_request_checksum_context_new(meta_request->allocator, &meta_request->checksum_config);
+        part->checksum_context = aws_s3_upload_request_checksum_context_new(
+            meta_request->allocator, &meta_request->checksum_config, meta_request->upload_review_callback != NULL);
         part->size = request->request_body.len;
         aws_array_list_set_at(&auto_ranged_put->synced_data.part_list, &part, request->part_number - 1);
     }

source/s3_checksum_context.c

@@ -25,7 +25,8 @@ static void s_aws_s3_upload_request_checksum_context_destroy(void *context) {
 
 static struct aws_s3_upload_request_checksum_context *s_s3_upload_request_checksum_context_new_base(
     struct aws_allocator *allocator,
-    const struct aws_s3_meta_request_checksum_config_storage *checksum_config) {
+    const struct aws_s3_meta_request_checksum_config_storage *checksum_config,
+    bool has_review_callback) {
     AWS_PRECONDITION(allocator);
 
     struct aws_s3_upload_request_checksum_context *context =
@@ -48,6 +49,7 @@ static struct aws_s3_upload_request_checksum_context *s_s3_upload_request_checks
     context->algorithm = checksum_config->checksum_algorithm;
     context->location = checksum_config->location;
     context->encoded_checksum_size = aws_get_digest_size_from_checksum_algorithm(context->algorithm);
+    context->has_review_callback = has_review_callback;
 
     /* Convert to base64 encoded size */
     size_t encoded_size = 0;
@@ -62,9 +64,10 @@ static struct aws_s3_upload_request_checksum_context *s_s3_upload_request_checks
 
 struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_context_new(
     struct aws_allocator *allocator,
-    const struct aws_s3_meta_request_checksum_config_storage *checksum_config) {
+    const struct aws_s3_meta_request_checksum_config_storage *checksum_config,
+    bool has_review_callback) {
     struct aws_s3_upload_request_checksum_context *context =
-        s_s3_upload_request_checksum_context_new_base(allocator, checksum_config);
+        s_s3_upload_request_checksum_context_new_base(allocator, checksum_config, has_review_callback);
     if (context && context->encoded_checksum_size > 0) {
         /* Initial the buffer for checksum */
         aws_byte_buf_init(&context->synced_data.base64_checksum, allocator, context->encoded_checksum_size);
@@ -75,9 +78,10 @@ struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_co
 struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(
     struct aws_allocator *allocator,
     const struct aws_s3_meta_request_checksum_config_storage *checksum_config,
-    struct aws_byte_cursor existing_base64_checksum) {
+    struct aws_byte_cursor existing_base64_checksum,
+    bool has_review_callback) {
     struct aws_s3_upload_request_checksum_context *context =
-        s_s3_upload_request_checksum_context_new_base(allocator, checksum_config);
+        s_s3_upload_request_checksum_context_new_base(allocator, checksum_config, has_review_callback);
     if (context) {
         /* Initial the buffer for checksum from the exist checksum */
         if (context->encoded_checksum_size != existing_base64_checksum.len) {
@@ -115,7 +119,8 @@ struct aws_s3_upload_request_checksum_context *aws_s3_upload_request_checksum_co
 }
 
 bool aws_s3_upload_request_checksum_context_should_calculate(struct aws_s3_upload_request_checksum_context *context) {
-    if (!context || context->algorithm == AWS_SCA_NONE) {
+    if (!context || context->algorithm == AWS_SCA_NONE || context->algorithm == AWS_SCA_UNKNOWN ||
+        (context->location == AWS_SCL_NONE && !context->has_review_callback)) {
         return false;
     }
 

source/s3_checksums.c

@@ -453,8 +453,6 @@ int aws_checksum_compute(
     }
 }
 
-static const struct aws_byte_cursor s_checksum_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-");
-
 static void s_byte_buf_to_upper(struct aws_byte_buf *buf) {
     AWS_PRECONDITION(buf);
 
@@ -489,7 +487,7 @@ static int s_init_and_verify_checksum_config_from_headers(
             return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
         }
 
-        if (aws_byte_cursor_starts_with_ignore_case(&header.name, &s_checksum_prefix)) {
+        if (aws_s3_is_checksum_value_header_name(header.name)) {
             checksum_header_name = header.name;
             has_checksum_value_header = true;
             break;
@@ -551,7 +549,7 @@ static int s_init_and_verify_checksum_config_from_headers(
     checksum_config->location = AWS_SCL_NONE;
 
     if (header_algo == AWS_SCA_UNKNOWN) {
-        aws_byte_cursor_advance(&checksum_header_name, s_checksum_prefix.len);
+        aws_byte_cursor_advance(&checksum_header_name, sizeof("x-amz-checksum-") - 1);
         aws_byte_buf_init_copy_from_cursor(
             &checksum_config->unknown_checksum_algo, checksum_config->allocator, checksum_header_name);
         s_byte_buf_to_upper(&checksum_config->unknown_checksum_algo);

source/s3_default_meta_request.c

@@ -347,7 +347,7 @@ static void s_s3_default_prepare_request_finish(
     struct aws_http_message *message = aws_s3_message_util_copy_http_message_no_body_all_headers(
         meta_request->allocator, meta_request->initial_request_message);
 
-    bool flexible_checksum = meta_request->checksum_config.location != AWS_SCL_NONE;
+    bool flexible_checksum = meta_request->checksum_config.checksum_algorithm != AWS_SCA_NONE;
     if (!flexible_checksum && meta_request->should_compute_content_md5) {
         /* If flexible checksum used, client MUST skip Content-MD5 header computation */
         aws_s3_message_util_add_content_md5_header(meta_request->allocator, &request->request_body, message);
@@ -362,8 +362,17 @@ static void s_s3_default_prepare_request_finish(
         /* Only PUT Object and Upload part support trailing checksum, that needs the special encoding even if the body
          * has 0 length. */
         /* Create checksum context from config if needed */
-        struct aws_s3_upload_request_checksum_context *checksum_context =
-            aws_s3_upload_request_checksum_context_new(meta_request->allocator, &meta_request->checksum_config);
+        struct aws_s3_upload_request_checksum_context *checksum_context = NULL;
+
+        /**
+         * Note: CompleteMPU is unique in the sence that checksum on the object level is the full object checksum for
+         * all parts and not checksum of the body. So avoid any additional checksum handling if default req is
+         * completeMPU.
+         */
+        if (meta_request_default->request_type != AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD) {
+            checksum_context = aws_s3_upload_request_checksum_context_new(
+                meta_request->allocator, &meta_request->checksum_config, meta_request->upload_review_callback != NULL);
+        }
 
         aws_s3_message_util_assign_body(
             meta_request->allocator, &request->request_body, NULL, message, checksum_context);

source/s3_request_messages.c

@@ -239,7 +239,6 @@ const struct aws_byte_cursor g_s3_create_session_allowed_headers[] = {
 const size_t g_s3_create_session_allowed_headers_count = AWS_ARRAY_SIZE(g_s3_create_session_allowed_headers);
 
 static const struct aws_byte_cursor s_x_amz_meta_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-meta-");
-static const struct aws_byte_cursor s_x_amz_checksum_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-");
 
 static const struct aws_byte_cursor s_checksum_type_header =
     AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-type");
@@ -1234,7 +1233,7 @@ void aws_s3_message_util_copy_headers(
         }
 
         if (exclude_x_amz_checksum) {
-            if (aws_byte_cursor_starts_with_ignore_case(&header.name, &s_x_amz_checksum_prefix)) {
+            if (aws_s3_is_checksum_value_header_name(header.name)) {
                 continue;
             }
         }

source/s3_util.c

@@ -1019,3 +1019,12 @@ int aws_s3_extract_parts_from_etag(struct aws_byte_cursor etag_header_value, uin
 
     return AWS_OP_SUCCESS;
 }
+
+static const struct aws_byte_cursor s_checksum_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-");
+
+bool aws_s3_is_checksum_value_header_name(struct aws_byte_cursor header_name) {
+    return aws_byte_cursor_starts_with_ignore_case(&header_name, &s_checksum_prefix) &&
+           !aws_byte_cursor_eq_c_str_ignore_case(&header_name, "x-amz-checksum-type") &&
+           !aws_byte_cursor_eq_c_str_ignore_case(&header_name, "x-amz-checksum-mode") &&
+           !aws_byte_cursor_eq_c_str_ignore_case(&header_name, "x-amz-checksum-algorithm");
+}

tests/CMakeLists.txt

@@ -261,6 +261,7 @@ add_test_case(test_s3_calculate_client_optimal_range_size)
 add_test_case(test_s3_calculate_request_optimal_range_size)
 add_test_case(test_s3_extract_parts_from_etag)
 add_test_case(test_add_user_agent_header)
+add_test_case(test_s3_checksum_header)
 
 add_test_case(test_get_existing_platform_info)
 add_test_case(test_get_nonexistent_platform_info)

tests/s3_checksum_context_test.c

@@ -21,7 +21,7 @@ static int s_test_upload_request_checksum_context_get_checksum_cursor(struct aws
 
     /* Test get checksum cursor with context that has no calculated checksum */
     struct aws_s3_upload_request_checksum_context *context =
-        aws_s3_upload_request_checksum_context_new(allocator, &config);
+        aws_s3_upload_request_checksum_context_new(allocator, &config, false);
     ASSERT_NOT_NULL(context);
 
     struct aws_byte_cursor cursor = aws_s3_upload_request_checksum_context_get_checksum_cursor(context);
@@ -32,8 +32,8 @@ static int s_test_upload_request_checksum_context_get_checksum_cursor(struct aws
 
     /* Test get checksum cursor with context that has calculated checksum */
     struct aws_byte_cursor existing_checksum = aws_byte_cursor_from_c_str("dGVzdA==");
-    context =
-        aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(allocator, &config, existing_checksum);
+    context = aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(
+        allocator, &config, existing_checksum, false);
     ASSERT_NOT_NULL(context);
 
     cursor = aws_s3_upload_request_checksum_context_get_checksum_cursor(context);
@@ -49,6 +49,7 @@ static int s_test_upload_request_checksum_context_get_checksum_cursor(struct aws
 
     return AWS_OP_SUCCESS;
 }
+
 AWS_TEST_CASE(
     test_upload_request_checksum_context_get_checksum_cursor,
     s_test_upload_request_checksum_context_get_checksum_cursor)
@@ -68,7 +69,7 @@ static int s_test_upload_request_checksum_context_error_cases(struct aws_allocat
     struct aws_byte_cursor wrong_size_checksum = aws_byte_cursor_from_c_str("short");
     struct aws_s3_upload_request_checksum_context *context =
         aws_s3_upload_request_checksum_context_new_with_existing_base64_checksum(
-            allocator, &config, wrong_size_checksum);
+            allocator, &config, wrong_size_checksum, false);
     ASSERT_NULL(context);
 
     /* Test helper functions with NULL context */
@@ -80,6 +81,35 @@ static int s_test_upload_request_checksum_context_error_cases(struct aws_allocat
     ASSERT_NULL(aws_s3_upload_request_checksum_context_acquire(NULL));
     ASSERT_NULL(aws_s3_upload_request_checksum_context_release(NULL));
 
+    /* unknown algo */
+    struct aws_s3_meta_request_checksum_config_storage config2 = {
+        .allocator = allocator,
+        .checksum_algorithm = AWS_SCA_UNKNOWN,
+        .location = AWS_SCL_NONE,
+        .has_full_object_checksum = false,
+    };
+    struct aws_s3_upload_request_checksum_context *context2 =
+        aws_s3_upload_request_checksum_context_new(allocator, &config2, false);
+
+    ASSERT_NOT_NULL(context2);
+    ASSERT_FALSE(aws_s3_upload_request_checksum_context_should_calculate(context2));
+
+    /* unknown algo */
+    struct aws_s3_meta_request_checksum_config_storage config3 = {
+        .allocator = allocator,
+        .checksum_algorithm = AWS_SCA_CRC32,
+        .location = AWS_SCL_NONE,
+        .has_full_object_checksum = false,
+    };
+    struct aws_s3_upload_request_checksum_context *context3 =
+        aws_s3_upload_request_checksum_context_new(allocator, &config3, false);
+
+    ASSERT_NOT_NULL(context3);
+    ASSERT_FALSE(aws_s3_upload_request_checksum_context_should_calculate(context3));
+
+    aws_s3_upload_request_checksum_context_release(context2);
+    aws_s3_upload_request_checksum_context_release(context3);
+
     return AWS_OP_SUCCESS;
 }
 AWS_TEST_CASE(test_upload_request_checksum_context_error_cases, s_test_upload_request_checksum_context_error_cases)
@@ -101,7 +131,7 @@ static int s_test_upload_request_checksum_context_different_algorithms(struct aw
         AWS_ZERO_STRUCT(config.full_object_checksum);
 
         struct aws_s3_upload_request_checksum_context *context =
-            aws_s3_upload_request_checksum_context_new(allocator, &config);
+            aws_s3_upload_request_checksum_context_new(allocator, &config, false);
         ASSERT_NOT_NULL(context);
         ASSERT_INT_EQUALS(algorithms[i], context->algorithm);
         ASSERT_INT_EQUALS(AWS_SCL_HEADER, context->location);

tests/s3_checksum_stream_test.c

@@ -124,7 +124,7 @@ static int s_stream_chunk(
 
     /* Create checksum context */
     struct aws_s3_upload_request_checksum_context *context =
-        aws_s3_upload_request_checksum_context_new(allocator, &config);
+        aws_s3_upload_request_checksum_context_new(allocator, &config, false);
     if (!context) {
         return AWS_OP_ERR;
     }

tests/s3_util_tests.c

@@ -1046,3 +1046,22 @@ static int s_test_s3_extract_parts_from_etag(struct aws_allocator *allocator, vo
     aws_s3_library_clean_up();
     return 0;
 }
+
+AWS_TEST_CASE(test_s3_checksum_header, s_test_s3_checksum_header)
+static int s_test_s3_checksum_header(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+    aws_s3_library_init(allocator);
+
+    ASSERT_TRUE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-crc32")));
+    ASSERT_TRUE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-foo")));
+    ASSERT_TRUE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-md5")));
+
+    ASSERT_FALSE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-type")));
+    ASSERT_FALSE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-mode")));
+    ASSERT_FALSE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-checksum-algorithm")));
+    ASSERT_FALSE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("x-amz-cHeCKsuM-tYpE")));
+    ASSERT_FALSE(aws_s3_is_checksum_value_header_name(aws_byte_cursor_from_c_str("X-AMZ-CHECKSUM-ALGORITHM")));
+
+    aws_s3_library_clean_up();
+    return 0;
+}