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;
 };
 
 /**

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_checksum_context.c

@@ -48,6 +48,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 = checksum_config->full_object_checksum_callback;
 
     /* Convert to base64 encoded size */
     size_t encoded_size = 0;
@@ -115,7 +116,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);
+        }
 
         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

@@ -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)
@@ -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);
+
+    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);
+
+    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)

tests/s3_util_tests.c

@@ -1046,3 +1046,20 @@ 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")));
+
+    aws_s3_library_clean_up();
+    return 0;
+}