Respect checksum header over settings from options (#474)

TingDaoK · web-flow · commit 33b8cd07c530 · 2024-12-11T14:27:55.000-08:00
diff --git a/include/aws/s3/private/s3_checksums.h b/include/aws/s3/private/s3_checksums.h
@@ -166,10 +166,12 @@ AWS_S3_API
 int aws_checksum_finalize(struct aws_s3_checksum *checksum, struct aws_byte_buf *output);
 
 AWS_S3_API
-void aws_checksum_config_storage_init(
+int aws_checksum_config_storage_init(
     struct aws_allocator *allocator,
     struct checksum_config_storage *internal_config,
-    const struct aws_s3_checksum_config *config);
+    const struct aws_s3_checksum_config *config,
+    const struct aws_http_message *message,
+    const void *log_id);
 
 AWS_S3_API
 void aws_checksum_config_storage_cleanup(struct checksum_config_storage *internal_config);
diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h
@@ -756,6 +756,9 @@ struct aws_s3_meta_request_options {
     /**
      * Optional.
      * if set, the flexible checksum will be performed by client based on the config.
+     *
+     * Notes: checksum can also be added through the http message provided.
+     *      The checksum in http header will override corresponding the checksum config.
      */
     const struct aws_s3_checksum_config *checksum_config;
 
diff --git a/source/s3_auto_ranged_put.c b/source/s3_auto_ranged_put.c
@@ -313,78 +313,6 @@ static struct aws_s3_meta_request_vtable s_s3_auto_ranged_put_vtable = {
     .pause = s_s3_auto_ranged_put_pause,
 };
 
-static int s_init_and_verify_checksum_config_from_headers(
-    struct checksum_config_storage *checksum_config,
-    const struct aws_http_message *message,
-    const void *log_id) {
-    /* Check if the checksum header was set from the message */
-    struct aws_http_headers *headers = aws_http_message_get_headers(message);
-    enum aws_s3_checksum_algorithm header_algo = AWS_SCA_NONE;
-    struct aws_byte_cursor header_value;
-    AWS_ZERO_STRUCT(header_value);
-
-    for (size_t i = 0; i < AWS_ARRAY_SIZE(s_checksum_algo_priority_list); i++) {
-        enum aws_s3_checksum_algorithm algorithm = s_checksum_algo_priority_list[i];
-        const struct aws_byte_cursor algorithm_header_name =
-            aws_get_http_header_name_from_checksum_algorithm(algorithm);
-        if (aws_http_headers_get(headers, algorithm_header_name, &header_value) == AWS_OP_SUCCESS) {
-            if (header_algo == AWS_SCA_NONE) {
-                header_algo = algorithm;
-            } else {
-                /* If there are multiple checksum headers set, it's malformed request */
-                AWS_LOGF_ERROR(
-                    AWS_LS_S3_META_REQUEST,
-                    "id=%p Could not create auto-ranged-put meta request; multiple checksum headers has been set",
-                    log_id);
-                return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-            }
-        }
-    }
-    if (header_algo == AWS_SCA_NONE) {
-        /* No checksum header found, done */
-        return AWS_OP_SUCCESS;
-    }
-
-    /* Found the full object checksum from the header, check if it matches the explicit setting from config */
-    if (checksum_config->checksum_algorithm != AWS_SCA_NONE && checksum_config->checksum_algorithm != header_algo) {
-        AWS_LOGF_ERROR(
-            AWS_LS_S3_META_REQUEST,
-            "id=%p: Could not create auto-ranged-put meta request; checksum config mismatch the checksum from header.",
-            log_id);
-        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-    }
-    if (checksum_config->has_full_object_checksum) {
-        /* If the full object checksum has been set, it's malformed request */
-        AWS_LOGF_ERROR(
-            AWS_LS_S3_META_REQUEST,
-            "id=%p: Could not create auto-ranged-put meta request; full object checksum is set from multiple ways.",
-            log_id);
-        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-    }
-
-    AWS_LOGF_DEBUG(
-        AWS_LS_S3_META_REQUEST,
-        "id=%p: Setting the full-object checksum from header; algorithm: " PRInSTR ", value: " PRInSTR ".",
-        log_id,
-        AWS_BYTE_CURSOR_PRI(aws_get_checksum_algorithm_name(header_algo)),
-        AWS_BYTE_CURSOR_PRI(header_value));
-    /* Set algo */
-    checksum_config->checksum_algorithm = header_algo;
-    if (checksum_config->location == AWS_SCL_NONE) {
-        /* Set the checksum location to trailer for the parts, complete MPU will still have the checksum in the header.
-         * But to keep the data integrity for the parts, we need to set the checksum location to trailer to send the
-         * parts level checksums.
-         */
-        checksum_config->location = AWS_SCL_TRAILER;
-    }
-
-    /* Set full object checksum from the header value. */
-    aws_byte_buf_init_copy_from_cursor(
-        &checksum_config->full_object_checksum, checksum_config->allocator, header_value);
-    checksum_config->has_full_object_checksum = true;
-    return AWS_OP_SUCCESS;
-}
-
 /* Allocate a new auto-ranged put meta request */
 struct aws_s3_meta_request *aws_s3_meta_request_auto_ranged_put_new(
     struct aws_allocator *allocator,
@@ -442,9 +370,10 @@ struct aws_s3_meta_request *aws_s3_meta_request_auto_ranged_put_new(
         goto error_clean_up;
     }
 
-    if (s_init_and_verify_checksum_config_from_headers(
-            &auto_ranged_put->base.checksum_config, options->message, (void *)&auto_ranged_put->base)) {
-        goto error_clean_up;
+    if (auto_ranged_put->base.checksum_config.full_object_checksum.len > 0) {
+        /* The full object checksum was set, make sure the parts level checksum will be calculated and sent via client.
+         */
+        auto_ranged_put->base.checksum_config.location = AWS_SCL_TRAILER;
     }
 
     AWS_LOGF_DEBUG(
diff --git a/source/s3_checksums.c b/source/s3_checksums.c
@@ -2,6 +2,7 @@
 #include "aws/s3/private/s3_util.h"
 #include <aws/cal/hash.h>
 #include <aws/checksums/crc.h>
+#include <aws/http/request_response.h>
 
 #define AWS_CRC32_LEN sizeof(uint32_t)
 #define AWS_CRC32C_LEN sizeof(uint32_t)
@@ -330,17 +331,109 @@ int aws_checksum_compute(
     }
 }
 
-void aws_checksum_config_storage_init(
+static int s_init_and_verify_checksum_config_from_headers(
+    struct checksum_config_storage *checksum_config,
+    const struct aws_http_message *message,
+    const void *log_id) {
+    /* Check if the checksum header was set from the message */
+    struct aws_http_headers *headers = aws_http_message_get_headers(message);
+    enum aws_s3_checksum_algorithm header_algo = AWS_SCA_NONE;
+    struct aws_byte_cursor header_value;
+    AWS_ZERO_STRUCT(header_value);
+
+    for (size_t i = 0; i < AWS_ARRAY_SIZE(s_checksum_algo_priority_list); i++) {
+        enum aws_s3_checksum_algorithm algorithm = s_checksum_algo_priority_list[i];
+        const struct aws_byte_cursor algorithm_header_name =
+            aws_get_http_header_name_from_checksum_algorithm(algorithm);
+        if (aws_http_headers_get(headers, algorithm_header_name, &header_value) == AWS_OP_SUCCESS) {
+            if (header_algo == AWS_SCA_NONE) {
+                header_algo = algorithm;
+            } else {
+                /* If there are multiple checksum headers set, it's malformed request */
+                AWS_LOGF_ERROR(
+                    AWS_LS_S3_META_REQUEST,
+                    "id=%p Could not create auto-ranged-put meta request; multiple checksum headers has been set",
+                    log_id);
+                return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+            }
+        }
+    }
+    if (header_algo == AWS_SCA_NONE) {
+        /* No checksum header found, done */
+        return AWS_OP_SUCCESS;
+    }
+
+    if (checksum_config->has_full_object_checksum) {
+        /* If the full object checksum has been set, it's malformed request */
+        AWS_LOGF_ERROR(
+            AWS_LS_S3_META_REQUEST,
+            "id=%p: Could not create auto-ranged-put meta request; full object checksum is set from multiple ways.",
+            log_id);
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+    }
+
+    AWS_LOGF_DEBUG(
+        AWS_LS_S3_META_REQUEST,
+        "id=%p Setting the full-object checksum from header; algorithm: " PRInSTR ", value: " PRInSTR ".",
+        log_id,
+        AWS_BYTE_CURSOR_PRI(aws_get_checksum_algorithm_name(header_algo)),
+        AWS_BYTE_CURSOR_PRI(header_value));
+    /* Set algo */
+    checksum_config->checksum_algorithm = header_algo;
+    /**
+     * Set the location to NONE to avoid adding extra checksums from client.
+     *
+     * Notes: The multipart upload will set the location to trailer to add parts level checksums.
+     **/
+    checksum_config->location = AWS_SCL_NONE;
+
+    /* Set full object checksum from the header value. */
+    aws_byte_buf_init_copy_from_cursor(
+        &checksum_config->full_object_checksum, checksum_config->allocator, header_value);
+    checksum_config->has_full_object_checksum = true;
+    return AWS_OP_SUCCESS;
+}
+
+int aws_checksum_config_storage_init(
     struct aws_allocator *allocator,
     struct checksum_config_storage *internal_config,
-    const struct aws_s3_checksum_config *config) {
+    const struct aws_s3_checksum_config *config,
+    const struct aws_http_message *message,
+    const void *log_id) {
     AWS_ZERO_STRUCT(*internal_config);
     /* Zero out the struct and set the allocator regardless. */
     internal_config->allocator = allocator;
 
     if (!config) {
-        return;
+        return AWS_OP_SUCCESS;
+    }
+
+    struct aws_http_headers *headers = aws_http_message_get_headers(message);
+    if (config->location == AWS_SCL_TRAILER) {
+        struct aws_byte_cursor existing_encoding;
+        AWS_ZERO_STRUCT(existing_encoding);
+        if (aws_http_headers_get(headers, g_content_encoding_header_name, &existing_encoding) == AWS_OP_SUCCESS) {
+            if (aws_byte_cursor_find_exact(&existing_encoding, &g_content_encoding_header_aws_chunked, NULL) ==
+                AWS_OP_SUCCESS) {
+                AWS_LOGF_ERROR(
+                    AWS_LS_S3_META_REQUEST,
+                    "id=%p Cannot create meta s3 request; for trailer checksum, the original request cannot be "
+                    "aws-chunked encoding. The client will encode the request instead.",
+                    (void *)log_id);
+                aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+                return AWS_OP_ERR;
+            }
+        }
     }
+    if (config->location != AWS_SCL_NONE && config->checksum_algorithm == AWS_SCA_NONE) {
+        AWS_LOGF_ERROR(
+            AWS_LS_S3_META_REQUEST,
+            "id=%p Cannot create meta s3 request; checksum location is set, but no checksum algorithm selected.",
+            (void *)log_id);
+        aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+        return AWS_OP_ERR;
+    }
+
     internal_config->checksum_algorithm = config->checksum_algorithm;
     internal_config->location = config->location;
     internal_config->validate_response_checksum = config->validate_response_checksum;
@@ -385,6 +478,14 @@ void aws_checksum_config_storage_init(
         internal_config->response_checksum_algorithms.sha1 = true;
         internal_config->response_checksum_algorithms.sha256 = true;
     }
+
+    /* After applying settings from config, check the message header to override the corresponding settings. */
+    if (s_init_and_verify_checksum_config_from_headers(internal_config, message, log_id)) {
+        return AWS_OP_ERR;
+    }
+    /* Anything fail afterward will need to cleanup the storage. */
+
+    return AWS_OP_SUCCESS;
 }
 
 void aws_checksum_config_storage_cleanup(struct checksum_config_storage *internal_config) {
diff --git a/source/s3_client.c b/source/s3_client.c
@@ -945,36 +945,6 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request(
         return NULL;
     }
 
-    if (options->checksum_config) {
-        if (options->checksum_config->location == AWS_SCL_TRAILER) {
-            struct aws_http_headers *headers = aws_http_message_get_headers(options->message);
-            struct aws_byte_cursor existing_encoding;
-            AWS_ZERO_STRUCT(existing_encoding);
-            if (aws_http_headers_get(headers, g_content_encoding_header_name, &existing_encoding) == AWS_OP_SUCCESS) {
-                if (aws_byte_cursor_find_exact(&existing_encoding, &g_content_encoding_header_aws_chunked, NULL) ==
-                    AWS_OP_SUCCESS) {
-                    AWS_LOGF_ERROR(
-                        AWS_LS_S3_CLIENT,
-                        "id=%p Cannot create meta s3 request; for trailer checksum, the original request cannot be "
-                        "aws-chunked encoding. The client will encode the request instead.",
-                        (void *)client);
-                    aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-                    return NULL;
-                }
-            }
-        }
-
-        if (options->checksum_config->location != AWS_SCL_NONE &&
-            options->checksum_config->checksum_algorithm == AWS_SCA_NONE) {
-            AWS_LOGF_ERROR(
-                AWS_LS_S3_CLIENT,
-                "id=%p Cannot create meta s3 request; checksum location is set, but no checksum algorithm selected.",
-                (void *)client);
-            aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-            return NULL;
-        }
-    }
-
     if (s_apply_endpoint_override(client, message_headers, options->endpoint)) {
         return NULL;
     }
diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c
@@ -218,7 +218,14 @@ int aws_s3_meta_request_init_base(
 
     *((size_t *)&meta_request->part_size) = part_size;
     *((bool *)&meta_request->should_compute_content_md5) = should_compute_content_md5;
-    aws_checksum_config_storage_init(meta_request->allocator, &meta_request->checksum_config, options->checksum_config);
+    if (aws_checksum_config_storage_init(
+            meta_request->allocator,
+            &meta_request->checksum_config,
+            options->checksum_config,
+            options->message,
+            (void *)meta_request)) {
+        goto error;
+    }
 
     if (options->signing_config) {
         meta_request->cached_signing_config = aws_cached_signing_config_new(client, options->signing_config);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -186,6 +186,7 @@ add_net_test_case(test_s3_round_trip_with_filepath)
 add_net_test_case(test_s3_round_trip_mpu_with_filepath)
 add_net_test_case(test_s3_round_trip_with_filepath_no_content_length)
 add_net_test_case(test_s3_round_trip_mpu_with_filepath_no_content_length)
+add_net_test_case(test_s3_round_trip_default_get_full_object_checksum_fc)
 add_net_test_case(test_s3_round_trip_mpu_multipart_get_full_object_checksum_fc)
 add_net_test_case(test_s3_round_trip_mpu_multipart_get_full_object_checksum_fc_header)
 add_net_test_case(test_s3_round_trip_mpu_multipart_get_full_object_checksum_via_callback)
diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c
@@ -3918,7 +3918,11 @@ void s_s3_test_no_validate_checksum(
 }
 
 /* TODO: maybe refactor the fc -> flexible checksum tests to be less copy/paste */
-static int s_test_s3_round_trip_default_get_fc_helper(struct aws_allocator *allocator, void *ctx, bool via_header) {
+static int s_test_s3_round_trip_default_get_fc_helper(
+    struct aws_allocator *allocator,
+    void *ctx,
+    bool via_header,
+    enum aws_s3_tester_full_object_checksum full_object_checksum) {
     (void)ctx;
 
     struct aws_s3_tester tester;
@@ -3960,6 +3964,10 @@ static int s_test_s3_round_trip_default_get_fc_helper(struct aws_allocator *allo
                     .object_path_override = object_path,
                 },
         };
+        if (algorithm != AWS_SCA_SHA1 && algorithm != AWS_SCA_SHA256) {
+            /* Full object checksums doesn't support SHA. */
+            put_options.put_options.full_object_checksum = full_object_checksum;
+        }
 
         ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL));
 
@@ -3992,12 +4000,18 @@ static int s_test_s3_round_trip_default_get_fc_helper(struct aws_allocator *allo
 
 AWS_TEST_CASE(test_s3_round_trip_default_get_fc, s_test_s3_round_trip_default_get_fc)
 static int s_test_s3_round_trip_default_get_fc(struct aws_allocator *allocator, void *ctx) {
-    return s_test_s3_round_trip_default_get_fc_helper(allocator, ctx, false);
+    return s_test_s3_round_trip_default_get_fc_helper(allocator, ctx, false, AWS_TEST_FOC_NONE);
 }
 
 AWS_TEST_CASE(test_s3_round_trip_default_get_fc_header, s_test_s3_round_trip_default_get_fc_header)
 static int s_test_s3_round_trip_default_get_fc_header(struct aws_allocator *allocator, void *ctx) {
-    return s_test_s3_round_trip_default_get_fc_helper(allocator, ctx, true);
+    return s_test_s3_round_trip_default_get_fc_helper(allocator, ctx, true, AWS_TEST_FOC_NONE);
+}
+AWS_TEST_CASE(
+    test_s3_round_trip_default_get_full_object_checksum_fc,
+    s_test_s3_round_trip_default_get_full_object_checksum_fc)
+static int s_test_s3_round_trip_default_get_full_object_checksum_fc(struct aws_allocator *allocator, void *ctx) {
+    return s_test_s3_round_trip_default_get_fc_helper(allocator, ctx, false, AWS_TEST_FOC_HEADER);
 }
 
 static int s_test_s3_round_trip_multipart_get_fc_helper(struct aws_allocator *allocator, void *ctx, bool via_header) {
@@ -4066,6 +4080,7 @@ AWS_TEST_CASE(test_s3_round_trip_multipart_get_fc, s_test_s3_round_trip_multipar
 static int s_test_s3_round_trip_multipart_get_fc(struct aws_allocator *allocator, void *ctx) {
     return s_test_s3_round_trip_multipart_get_fc_helper(allocator, ctx, false);
 }
+
 AWS_TEST_CASE(test_s3_round_trip_multipart_get_fc_header, s_test_s3_round_trip_multipart_get_fc_header)
 static int s_test_s3_round_trip_multipart_get_fc_header(struct aws_allocator *allocator, void *ctx) {
     return s_test_s3_round_trip_multipart_get_fc_helper(allocator, ctx, true);
@@ -4166,6 +4181,7 @@ AWS_TEST_CASE(
 static int s_test_s3_round_trip_mpu_multipart_get_full_object_checksum_fc(struct aws_allocator *allocator, void *ctx) {
     return s_test_s3_round_trip_mpu_multipart_get_fc_helper(allocator, ctx, false, AWS_TEST_FOC_HEADER);
 }
+
 AWS_TEST_CASE(
     test_s3_round_trip_mpu_multipart_get_full_object_checksum_fc_header,
     s_test_s3_round_trip_mpu_multipart_get_full_object_checksum_fc_header)
