Fix handling of content-encoding header for default request (#286)

Author: waahm7

source/s3_request_messages.c

@@ -745,6 +745,8 @@ struct aws_input_stream *aws_s3_message_util_assign_body(
     }
 
     struct aws_input_stream *input_stream = aws_input_stream_new_from_cursor(allocator, &buffer_byte_cursor);
+    struct aws_byte_buf content_encoding_header_buf;
+    AWS_ZERO_STRUCT(content_encoding_header_buf);
 
     if (input_stream == NULL) {
         goto error_clean_up;
@@ -754,11 +756,31 @@ struct aws_input_stream *aws_s3_message_util_assign_body(
         if (checksum_config->location == AWS_SCL_TRAILER) {
             /* aws-chunked encode the payload and add related headers */
 
-            /* set Content-Encoding header. TODO: the aws-chunked should be appended to the existing content encoding.
+            /* set Content-Encoding header. If the header already exists, append the exisiting value to aws-chunked
+             * We already made sure that the existing value is not 'aws_chunked' in 'aws_s3_client_make_meta_request'
+             * function.
              */
-            if (aws_http_headers_set(headers, g_content_encoding_header_name, g_content_encoding_header_aws_chunked)) {
+            struct aws_byte_cursor content_encoding_header_cursor;
+            bool has_content_encoding_header =
+                aws_http_headers_get(headers, g_content_encoding_header_name, &content_encoding_header_cursor) ==
+                AWS_OP_SUCCESS;
+            size_t content_encoding_header_buf_size =
+                has_content_encoding_header
+                    ? g_content_encoding_header_aws_chunked.len + content_encoding_header_cursor.len + 1
+                    : g_content_encoding_header_aws_chunked.len;
+            aws_byte_buf_init(&content_encoding_header_buf, allocator, content_encoding_header_buf_size);
+
+            if (has_content_encoding_header) {
+                aws_byte_buf_append_dynamic(&content_encoding_header_buf, &content_encoding_header_cursor);
+                aws_byte_buf_append_byte_dynamic(&content_encoding_header_buf, ',');
+            }
+            aws_byte_buf_append_dynamic(&content_encoding_header_buf, &g_content_encoding_header_aws_chunked);
+
+            if (aws_http_headers_set(
+                    headers, g_content_encoding_header_name, aws_byte_cursor_from_buf(&content_encoding_header_buf))) {
                 goto error_clean_up;
             }
+
             /* set x-amz-trailer header */
             if (aws_http_headers_set(
                     headers,
@@ -803,12 +825,13 @@ struct aws_input_stream *aws_s3_message_util_assign_body(
     aws_http_message_set_body_stream(out_message, input_stream);
     /* Let the message take the full ownership */
     aws_input_stream_release(input_stream);
-
+    aws_byte_buf_clean_up(&content_encoding_header_buf);
     return input_stream;
 
 error_clean_up:
     AWS_LOGF_ERROR(AWS_LS_S3_CLIENT, "Failed to assign body for s3 request http message, from body buffer .");
     aws_input_stream_release(input_stream);
+    aws_byte_buf_clean_up(&content_encoding_header_buf);
     return NULL;
 }
 

tests/CMakeLists.txt

@@ -77,6 +77,8 @@ add_net_test_case(test_s3_put_object_tls_default)
 add_net_test_case(test_s3_multipart_put_object_with_acl)
 add_net_test_case(test_s3_put_object_multiple)
 add_net_test_case(test_s3_put_object_less_than_part_size)
+add_net_test_case(test_s3_put_object_less_than_part_size_with_content_encoding)
+add_net_test_case(test_s3_put_object_mpu_with_content_encoding)
 add_net_test_case(test_s3_put_object_multipart_threshold)
 add_net_test_case(test_s3_put_object_multipart_threshold_less_than_part_size)
 add_net_test_case(test_s3_put_object_empty_object)

tests/s3_data_plane_tests.c

@@ -1825,6 +1825,157 @@ static int s_test_s3_put_object_less_than_part_size(struct aws_allocator *alloca
     return 0;
 }
 
+AWS_TEST_CASE(
+    test_s3_put_object_less_than_part_size_with_content_encoding,
+    s_test_s3_put_object_less_than_part_size_with_content_encoding)
+static int s_test_s3_put_object_less_than_part_size_with_content_encoding(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+
+    struct aws_s3_client_config client_config = {
+        .part_size = 20 * 1024 * 1024,
+    };
+
+    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_byte_cursor content_encoding_cursor = aws_byte_cursor_from_c_str("gzip");
+    uint32_t object_size_mb = 1;
+    char object_path_sprintf_buffer[128] = "";
+    snprintf(
+        object_path_sprintf_buffer,
+        sizeof(object_path_sprintf_buffer),
+        "" PRInSTR "-content-encoding-%uMB.txt",
+        AWS_BYTE_CURSOR_PRI(g_put_object_prefix),
+        object_size_mb);
+    struct aws_byte_cursor object_path_cursor = aws_byte_cursor_from_c_str(object_path_sprintf_buffer);
+
+    /*** put file with encoding ***/
+    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_SHA256,
+        .put_options =
+            {
+                .object_size_mb = object_size_mb,
+                .object_path_override = object_path_cursor,
+                .content_encoding = content_encoding_cursor,
+            },
+    };
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL));
+
+    /*** get file and validate encoding ***/
+    struct aws_s3_tester_meta_request_options get_options = {
+        .allocator = allocator,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
+        .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS,
+        .client = client,
+        .get_options =
+            {
+                .object_path = object_path_cursor,
+            },
+    };
+
+    struct aws_s3_meta_request_test_results get_object_result;
+    aws_s3_meta_request_test_results_init(&get_object_result, allocator);
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, &get_object_result));
+    struct aws_byte_cursor content_encoding_header_cursor;
+    ASSERT_SUCCESS(aws_http_headers_get(
+        get_object_result.response_headers, g_content_encoding_header_name, &content_encoding_header_cursor));
+    ASSERT_TRUE(aws_byte_cursor_eq(&content_encoding_cursor, &content_encoding_header_cursor));
+    aws_s3_meta_request_test_results_clean_up(&get_object_result);
+
+    client = aws_s3_client_release(client);
+
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}
+
+AWS_TEST_CASE(test_s3_put_object_mpu_with_content_encoding, s_test_s3_put_object_mpu_with_content_encoding)
+static int s_test_s3_put_object_mpu_with_content_encoding(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+
+    struct aws_s3_client_config client_config = {
+        .part_size = 5 * 1024 * 1024,
+    };
+
+    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_byte_cursor content_encoding_cursor = aws_byte_cursor_from_c_str("gzip");
+    uint32_t object_size_mb = 10;
+    char object_path_sprintf_buffer[128] = "";
+    snprintf(
+        object_path_sprintf_buffer,
+        sizeof(object_path_sprintf_buffer),
+        "" PRInSTR "-content-encoding-%uMB.txt",
+        AWS_BYTE_CURSOR_PRI(g_put_object_prefix),
+        object_size_mb);
+    struct aws_byte_cursor object_path_cursor = aws_byte_cursor_from_c_str(object_path_sprintf_buffer);
+
+    /*** put file with encoding ***/
+    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_SHA256,
+        .put_options =
+            {
+                .object_size_mb = object_size_mb,
+                .object_path_override = object_path_cursor,
+                .content_encoding = content_encoding_cursor,
+                .ensure_multipart = true,
+            },
+    };
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL));
+
+    /*** get file and validate encoding ***/
+    struct aws_s3_tester_meta_request_options get_options = {
+        .allocator = allocator,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT,
+        .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS,
+        .client = client,
+        .get_options =
+            {
+                .object_path = object_path_cursor,
+            },
+    };
+
+    struct aws_s3_meta_request_test_results get_object_result;
+    aws_s3_meta_request_test_results_init(&get_object_result, allocator);
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, &get_object_result));
+    struct aws_byte_cursor content_encoding_header_cursor;
+    ASSERT_SUCCESS(aws_http_headers_get(
+        get_object_result.response_headers, g_content_encoding_header_name, &content_encoding_header_cursor));
+    ASSERT_TRUE(aws_byte_cursor_eq(&content_encoding_cursor, &content_encoding_header_cursor));
+    aws_s3_meta_request_test_results_clean_up(&get_object_result);
+
+    client = aws_s3_client_release(client);
+
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}
+
 AWS_TEST_CASE(test_s3_put_object_multipart_threshold, s_test_s3_put_object_multipart_threshold)
 static int s_test_s3_put_object_multipart_threshold(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;

tests/s3_tester.c

@@ -1497,6 +1497,14 @@ int aws_s3_tester_send_meta_request_with_options(
                 aws_http_message_set_body_stream(message, NULL);
             }
 
+            if (options->put_options.content_encoding.ptr != NULL) {
+                struct aws_http_header content_encoding_header = {
+                    .name = g_content_encoding_header_name,
+                    .value = options->put_options.content_encoding,
+                };
+                aws_http_message_add_header(message, content_encoding_header);
+            }
+
             meta_request_options.message = message;
             aws_byte_buf_clean_up(&object_path_buffer);
         }

tests/s3_tester.h

@@ -187,6 +187,7 @@ struct aws_s3_tester_meta_request_options {
         struct aws_s3_meta_request_resume_token *resume_token;
         /* manually overwrite the content length for some invalid input stream */
         size_t content_length;
+        struct aws_byte_cursor content_encoding;
     } put_options;
 
     enum aws_s3_tester_sse_type sse_type;