From b02098ecf8ebaa83353579d873ae2fcd54f4b635 Mon Sep 17 00:00:00 2001 From: Dengke Date: Fri, 8 Nov 2024 09:48:06 -0800 Subject: [PATCH 1/6] will it break? --- include/aws/s3/private/s3_client_impl.h | 4 ++ include/aws/s3/private/s3_request.h | 3 ++ source/s3_client.c | 1 + source/s3_meta_request.c | 50 ++++++++++++++----------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index d90dad112..597f8add8 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -169,6 +169,10 @@ struct aws_s3_client_vtable { struct aws_parallel_input_stream *( *parallel_input_stream_new_from_file)(struct aws_allocator *allocator, struct aws_byte_cursor file_name); + + struct aws_http_stream *(*http_connection_make_request)( + struct aws_http_connection *client_connection, + const struct aws_http_make_request_options *options); }; struct aws_s3_upload_part_timeout_stats { diff --git a/include/aws/s3/private/s3_request.h b/include/aws/s3/private/s3_request.h index c132246fc..dab05323d 100644 --- a/include/aws/s3/private/s3_request.h +++ b/include/aws/s3/private/s3_request.h @@ -212,6 +212,9 @@ struct aws_s3_request { /* The metrics for the request telemetry */ struct aws_s3_request_metrics *metrics; + + /* The request is required to have the unsigned payload */ + uint32_t require_streaming_unsigned_payload_header : 1; } send_data; /* When true, response headers from the request will be stored in the request's response_headers variable. */ diff --git a/source/s3_client.c b/source/s3_client.c index c1f2fcf75..9caac24d5 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -141,6 +141,7 @@ static struct aws_s3_client_vtable s_s3_client_default_vtable = { .endpoint_shutdown_callback = s_s3_client_endpoint_shutdown_callback, .finish_destroy = s_s3_client_finish_destroy_default, .parallel_input_stream_new_from_file = aws_parallel_input_stream_new_from_file, + .http_connection_make_request = aws_http_connection_make_request, }; void aws_s3_set_dns_ttl(size_t ttl) { diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index a78034c4c..9360dbbfd 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -864,7 +864,7 @@ static void s_get_original_credentials_callback(struct aws_credentials *credenti } } -static int s_meta_request_resolve_signing_config( +static void s_meta_request_resolve_signing_config( struct aws_signing_config_aws *out_signing_config, struct aws_s3_request *request, struct aws_s3_meta_request *meta_request) { @@ -897,7 +897,12 @@ static int s_meta_request_resolve_signing_config( &out_signing_config->signed_body_value, &g_aws_signed_body_value_streaming_unsigned_payload_trailer)) { out_signing_config->signed_body_value = g_aws_signed_body_value_unsigned_payload; } - return AWS_OP_SUCCESS; + + /** + * In case of the signing was skipped for anonymous credentials, or presigned URL. + */ + request->send_data.require_streaming_unsigned_payload_header = aws_byte_cursor_eq( + &out_signing_config->signed_body_value, &g_aws_signed_body_value_streaming_unsigned_payload_trailer); } void aws_s3_meta_request_sign_request_default_impl( @@ -915,19 +920,19 @@ void aws_s3_meta_request_sign_request_default_impl( struct aws_signing_config_aws signing_config; - if (s_meta_request_resolve_signing_config(&signing_config, request, meta_request)) { - AWS_LOGF_DEBUG( + s_meta_request_resolve_signing_config(&signing_config, request, meta_request); + + request->send_data.signable = aws_signable_new_http_request(meta_request->allocator, request->send_data.message); + if (request->send_data.signable == NULL) { + AWS_LOGF_ERROR( AWS_LS_S3_META_REQUEST, - "id=%p: No signing config present. Not signing request %p.", + "id=%p: Could not allocate signable for request %p", (void *)meta_request, (void *)request); - on_signing_complete(NULL, AWS_ERROR_SUCCESS, user_data); + on_signing_complete(NULL, aws_last_error_or_unknown(), user_data); return; } - - request->send_data.signable = aws_signable_new_http_request(meta_request->allocator, request->send_data.message); - AWS_LOGF_TRACE( AWS_LS_S3_META_REQUEST, "id=%p Created signable %p for request %p with message %p", @@ -936,17 +941,6 @@ void aws_s3_meta_request_sign_request_default_impl( (void *)request, (void *)request->send_data.message); - if (request->send_data.signable == NULL) { - AWS_LOGF_ERROR( - AWS_LS_S3_META_REQUEST, - "id=%p: Could not allocate signable for request %p", - (void *)meta_request, - (void *)request); - - on_signing_complete(NULL, aws_last_error_or_unknown(), user_data); - return; - } - if (signing_config.algorithm == AWS_SIGNING_ALGORITHM_V4_S3EXPRESS && !disable_s3_express_signing) { /* Fetch credentials from S3 Express provider. */ struct aws_get_s3express_credentials_user_data *context = @@ -1074,6 +1068,17 @@ static void s_s3_meta_request_request_on_signed( aws_error_str(error_code)); } + /** + * Add the x-amz-content-sha256 header to support trailing checksum. + */ + if (request->send_data.require_streaming_unsigned_payload_header) { + struct aws_http_header trailer_content_sha256_header = { + .name = aws_byte_cursor_from_c_str("x-amz-content-sha256"), + .value = g_aws_signed_body_value_streaming_unsigned_payload_trailer, + }; + aws_http_message_add_header(request->send_data.message, trailer_content_sha256_header); + } + s_s3_prepare_request_payload_callback_and_destroy(payload, error_code); } @@ -1082,6 +1087,8 @@ void aws_s3_meta_request_send_request(struct aws_s3_meta_request *meta_request, AWS_PRECONDITION(connection); AWS_PRECONDITION(connection->http_connection); + struct aws_s3_client *client = meta_request->client; + AWS_PRECONDITION(client); struct aws_s3_request *request = connection->request; AWS_PRECONDITION(request); @@ -1104,7 +1111,8 @@ void aws_s3_meta_request_send_request(struct aws_s3_meta_request *meta_request, request->upload_timeout_ms = (size_t)options.response_first_byte_timeout_ms; } - struct aws_http_stream *stream = aws_http_connection_make_request(connection->http_connection, &options); + struct aws_http_stream *stream = + client->vtable->http_connection_make_request(connection->http_connection, &options); if (stream == NULL) { AWS_LOGF_ERROR( From 56e80f1191b4e5e46077cad1ac125087c555e875 Mon Sep 17 00:00:00 2001 From: Dengke Date: Fri, 8 Nov 2024 10:31:52 -0800 Subject: [PATCH 2/6] don't add, use set --- source/s3_meta_request.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 9360dbbfd..286ff7705 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -1058,6 +1058,17 @@ static void s_s3_meta_request_request_on_signed( finish: + /** + * Add the x-amz-content-sha256 header to support trailing checksum. + */ + if (error_code == AWS_ERROR_SUCCESS && request->send_data.require_streaming_unsigned_payload_header) { + struct aws_http_headers *headers = aws_http_message_get_headers(request->send_data.message); + error_code = aws_http_headers_set( + headers, + aws_byte_cursor_from_c_str("x-amz-content-sha256"), + g_aws_signed_body_value_streaming_unsigned_payload_trailer); + } + if (error_code != AWS_ERROR_SUCCESS) { AWS_LOGF_ERROR( @@ -1068,17 +1079,6 @@ static void s_s3_meta_request_request_on_signed( aws_error_str(error_code)); } - /** - * Add the x-amz-content-sha256 header to support trailing checksum. - */ - if (request->send_data.require_streaming_unsigned_payload_header) { - struct aws_http_header trailer_content_sha256_header = { - .name = aws_byte_cursor_from_c_str("x-amz-content-sha256"), - .value = g_aws_signed_body_value_streaming_unsigned_payload_trailer, - }; - aws_http_message_add_header(request->send_data.message, trailer_content_sha256_header); - } - s_s3_prepare_request_payload_callback_and_destroy(payload, error_code); } From 0122e74e8f5524636d08a4d0e69df71170715607 Mon Sep 17 00:00:00 2001 From: Dengke Date: Fri, 8 Nov 2024 13:55:35 -0800 Subject: [PATCH 3/6] all the complicated part is to add test --- include/aws/s3/private/s3_client_impl.h | 3 + source/s3_client.c | 4 +- tests/CMakeLists.txt | 2 + tests/s3_mock_server_tests.c | 200 ++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 2 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 597f8add8..2afc9f760 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -528,6 +528,9 @@ void aws_s3_client_update_upload_part_timeout( struct aws_s3_request *finished_upload_part_request, int finished_error_code); +AWS_S3_API +extern struct aws_s3_client_vtable g_s3_client_default_vtable; + AWS_EXTERN_C_END #endif /* AWS_S3_CLIENT_IMPL_H */ diff --git a/source/s3_client.c b/source/s3_client.c index 9caac24d5..54c875bb6 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -132,7 +132,7 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( struct aws_s3_client *client, const struct aws_s3_meta_request_options *options); -static struct aws_s3_client_vtable s_s3_client_default_vtable = { +struct aws_s3_client_vtable g_s3_client_default_vtable = { .meta_request_factory = s_s3_client_meta_request_factory_default, .acquire_http_connection = aws_http_connection_manager_acquire_connection, .get_host_address_count = aws_host_resolver_get_host_address_count, @@ -361,7 +361,7 @@ struct aws_s3_client *aws_s3_client_new( goto on_error; } - client->vtable = &s_s3_client_default_vtable; + client->vtable = &g_s3_client_default_vtable; aws_ref_count_init(&client->ref_count, client, (aws_simple_completion_callback *)s_s3_client_start_destroy); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b3466fb93..29eb67ddd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -312,6 +312,8 @@ add_net_test_case(test_s3_list_bucket_valid) # Tests against local mock server if(ENABLE_MOCK_SERVER_TESTS) add_net_test_case(multipart_upload_mock_server) + add_net_test_case(multipart_upload_unsigned_with_trailer_checksum_mock_server) + add_net_test_case(single_upload_unsigned_with_trailer_checksum_mock_server) add_net_test_case(multipart_upload_with_network_interface_names_mock_server) add_net_test_case(multipart_upload_checksum_with_retry_mock_server) add_net_test_case(multipart_download_checksum_with_retry_mock_server) diff --git a/tests/s3_mock_server_tests.c b/tests/s3_mock_server_tests.c index 4ac61a315..380b07be4 100644 --- a/tests/s3_mock_server_tests.c +++ b/tests/s3_mock_server_tests.c @@ -167,6 +167,206 @@ TEST_CASE(multipart_upload_mock_server) { return AWS_OP_SUCCESS; } +/* Singleton used by tests in this file */ +static struct get_requests_header_tester { + struct aws_allocator *alloc; + + /* Store the requests headers in the array. Array of struct aws_http_headers * */ + struct aws_array_list headers_array; + struct aws_mutex lock; +} s_get_requests_header_tester; + +static int s_get_requests_header_tester_init(struct aws_allocator *alloc) { + ASSERT_SUCCESS(aws_array_list_init_dynamic( + &s_get_requests_header_tester.headers_array, alloc, 1, sizeof(struct aws_http_headers *))); + ASSERT_SUCCESS(aws_mutex_init(&s_get_requests_header_tester.lock)); + return AWS_OP_SUCCESS; +} + +static void s_get_requests_header_tester_clean_up(void) { + /* iterate thought the headers array to clean up the headers */ + for (size_t i = 0; i < aws_array_list_length(&s_get_requests_header_tester.headers_array); ++i) { + struct aws_http_headers *headers = NULL; + aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, i); + aws_http_headers_release(headers); + } + aws_mutex_clean_up(&s_get_requests_header_tester.lock); + aws_array_list_clean_up(&s_get_requests_header_tester.headers_array); +} + +struct aws_http_stream *s_get_requests_header_make_request( + struct aws_http_connection *client_connection, + const struct aws_http_make_request_options *options) { + /** + * Record the headers in the array. + */ + aws_mutex_lock(&s_get_requests_header_tester.lock); + struct aws_http_headers *headers = aws_http_message_get_headers(options->request); + /* Keep the headers alive until we clean up the tester. */ + aws_http_headers_acquire(headers); + aws_array_list_push_back(&s_get_requests_header_tester.headers_array, &headers); + aws_mutex_unlock(&s_get_requests_header_tester.lock); + + struct aws_http_stream *stream = aws_http_connection_make_request(client_connection, options); + return stream; +} + +TEST_CASE(multipart_upload_unsigned_with_trailer_checksum_mock_server) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + ASSERT_SUCCESS(s_get_requests_header_tester_init(allocator)); + + struct aws_s3_client_config client_config = { + .tls_mode = AWS_MR_TLS_DISABLED, + }; + + ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION)); + + struct aws_s3_client_vtable s3_client_get_requests_header_vtable = g_s3_client_default_vtable; + s3_client_get_requests_header_vtable.http_connection_make_request = s_get_requests_header_make_request; + struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config); + ASSERT_NOT_NULL(client); + client->vtable = &s3_client_get_requests_header_vtable; + + struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/default"); + + 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, + .validate_get_response_checksum = false, + .put_options = + { + .object_size_mb = 10, + .object_path_override = object_path, + }, + .mock_server = true, + }; + struct aws_s3_meta_request_test_results out_results; + aws_s3_meta_request_test_results_init(&out_results, allocator); + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &out_results)); + ASSERT_SUCCESS(s_validate_mpu_mock_server_metrics(&out_results.synced_data.metrics)); + + /** + * Check the recorded headers. + * 4 requests should be made: + * - Create MPU + * - 2 Upload Part + * - Complete MPU + */ + ASSERT_UINT_EQUALS(4, aws_array_list_length(&s_get_requests_header_tester.headers_array)); + struct aws_byte_cursor content_sha256_header = aws_byte_cursor_from_c_str("x-amz-content-sha256"); + struct aws_byte_cursor content_sha256_header_val; + AWS_ZERO_STRUCT(content_sha256_header_val); + struct aws_byte_cursor authorization_header = aws_byte_cursor_from_c_str("Authorization"); + struct aws_byte_cursor authorization_header_val; + AWS_ZERO_STRUCT(authorization_header_val); + /* The first request should be Create MPU, and it should not have x-amz-content-sha256 header. */ + struct aws_http_headers *headers = NULL; + ASSERT_SUCCESS(aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, 0)); + /* No x-amz-content-sha256 header should be found. */ + ASSERT_FAILS(aws_http_headers_get(headers, content_sha256_header, &content_sha256_header_val)); + /* The second and third requests should be Upload Part, and it should have x-amz-content-sha256 header with + * STREAMING-UNSIGNED-PAYLOAD-TRAILER. */ + ASSERT_SUCCESS(aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, 1)); + /* x-amz-content-sha256 header should be found. */ + ASSERT_SUCCESS(aws_http_headers_get(headers, content_sha256_header, &content_sha256_header_val)); + ASSERT_TRUE( + aws_byte_cursor_eq(&content_sha256_header_val, &g_aws_signed_body_value_streaming_unsigned_payload_trailer)); + /* But the Authorization header should not be found, since we are not signing the request. */ + ASSERT_FAILS(aws_http_headers_get(headers, authorization_header, &authorization_header_val)); + ASSERT_SUCCESS(aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, 2)); + /* x-amz-content-sha256 header should be found. */ + ASSERT_SUCCESS(aws_http_headers_get(headers, content_sha256_header, &content_sha256_header_val)); + ASSERT_TRUE( + aws_byte_cursor_eq(&content_sha256_header_val, &g_aws_signed_body_value_streaming_unsigned_payload_trailer)); + /* But the Authorization header should not be found, since we are not signing the request. */ + ASSERT_FAILS(aws_http_headers_get(headers, authorization_header, &authorization_header_val)); + /* The last request should be Complete MPU, and it should not have x-amz-content-sha256 header. */ + ASSERT_SUCCESS(aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, 3)); + /* No x-amz-content-sha256 header should be found. */ + ASSERT_FAILS(aws_http_headers_get(headers, content_sha256_header, &content_sha256_header_val)); + + aws_s3_meta_request_test_results_clean_up(&out_results); + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + s_get_requests_header_tester_clean_up(); + + return AWS_OP_SUCCESS; +} + +TEST_CASE(single_upload_unsigned_with_trailer_checksum_mock_server) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + ASSERT_SUCCESS(s_get_requests_header_tester_init(allocator)); + + struct aws_s3_client_config client_config = { + .tls_mode = AWS_MR_TLS_DISABLED, + .part_size = 20 * 1024 * 1024, + }; + + ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION)); + + struct aws_s3_client_vtable s3_client_get_requests_header_vtable = g_s3_client_default_vtable; + s3_client_get_requests_header_vtable.http_connection_make_request = s_get_requests_header_make_request; + struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config); + ASSERT_NOT_NULL(client); + client->vtable = &s3_client_get_requests_header_vtable; + + struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/default"); + + 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, + .validate_get_response_checksum = false, + .put_options = + { + .object_size_mb = 10, + .object_path_override = object_path, + }, + .mock_server = true, + }; + struct aws_s3_meta_request_test_results out_results; + aws_s3_meta_request_test_results_init(&out_results, allocator); + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &out_results)); + + /** + * Check the recorded headers. + * 1 request should be made: + * - Put Object + */ + ASSERT_UINT_EQUALS(1, aws_array_list_length(&s_get_requests_header_tester.headers_array)); + struct aws_byte_cursor content_sha256_header = aws_byte_cursor_from_c_str("x-amz-content-sha256"); + struct aws_byte_cursor content_sha256_header_val; + AWS_ZERO_STRUCT(content_sha256_header_val); + struct aws_byte_cursor authorization_header = aws_byte_cursor_from_c_str("Authorization"); + struct aws_byte_cursor authorization_header_val; + AWS_ZERO_STRUCT(authorization_header_val); + /* The request should be Put Object, and it should have x-amz-content-sha256 header and not Authorization header. */ + struct aws_http_headers *headers = NULL; + ASSERT_SUCCESS(aws_array_list_get_at(&s_get_requests_header_tester.headers_array, &headers, 0)); + /* x-amz-content-sha256 header should be found. */ + ASSERT_SUCCESS(aws_http_headers_get(headers, content_sha256_header, &content_sha256_header_val)); + ASSERT_TRUE( + aws_byte_cursor_eq(&content_sha256_header_val, &g_aws_signed_body_value_streaming_unsigned_payload_trailer)); + /* But the Authorization header should not be found, since we are not signing the request. */ + ASSERT_FAILS(aws_http_headers_get(headers, authorization_header, &authorization_header_val)); + + aws_s3_meta_request_test_results_clean_up(&out_results); + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + s_get_requests_header_tester_clean_up(); + + return AWS_OP_SUCCESS; +} + TEST_CASE(multipart_upload_with_network_interface_names_mock_server) { (void)ctx; #if defined(AWS_OS_WINDOWS) From 945dbeff8d99537007b9aac5ba01af1a40568b59 Mon Sep 17 00:00:00 2001 From: Dengke Date: Tue, 19 Nov 2024 10:25:38 -0800 Subject: [PATCH 4/6] address comments --- include/aws/s3/private/s3_client_impl.h | 3 --- source/s3_client.c | 4 ++-- source/s3_meta_request.c | 8 ++++---- tests/s3_mock_server_tests.c | 19 +++++++++++-------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h index 2afc9f760..597f8add8 100644 --- a/include/aws/s3/private/s3_client_impl.h +++ b/include/aws/s3/private/s3_client_impl.h @@ -528,9 +528,6 @@ void aws_s3_client_update_upload_part_timeout( struct aws_s3_request *finished_upload_part_request, int finished_error_code); -AWS_S3_API -extern struct aws_s3_client_vtable g_s3_client_default_vtable; - AWS_EXTERN_C_END #endif /* AWS_S3_CLIENT_IMPL_H */ diff --git a/source/s3_client.c b/source/s3_client.c index f22d05511..f775427ae 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -132,7 +132,7 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default( struct aws_s3_client *client, const struct aws_s3_meta_request_options *options); -struct aws_s3_client_vtable g_s3_client_default_vtable = { +static struct aws_s3_client_vtable s_s3_client_default_vtable = { .meta_request_factory = s_s3_client_meta_request_factory_default, .acquire_http_connection = aws_http_connection_manager_acquire_connection, .get_host_address_count = aws_host_resolver_get_host_address_count, @@ -361,7 +361,7 @@ struct aws_s3_client *aws_s3_client_new( goto on_error; } - client->vtable = &g_s3_client_default_vtable; + client->vtable = &s_s3_client_default_vtable; aws_ref_count_init(&client->ref_count, client, (aws_simple_completion_callback *)s_s3_client_start_destroy); diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 286ff7705..74f37a519 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -1056,12 +1056,10 @@ static void s_s3_meta_request_request_on_signed( metric->time_metrics.sign_end_timestamp_ns - metric->time_metrics.sign_start_timestamp_ns; } -finish: - /** - * Add the x-amz-content-sha256 header to support trailing checksum. + * Add "x-amz-content-sha256: STREAMING-UNSIGNED-PAYLOAD-TRAILER" header to support trailing checksum. */ - if (error_code == AWS_ERROR_SUCCESS && request->send_data.require_streaming_unsigned_payload_header) { + if (request->send_data.require_streaming_unsigned_payload_header) { struct aws_http_headers *headers = aws_http_message_get_headers(request->send_data.message); error_code = aws_http_headers_set( headers, @@ -1069,6 +1067,8 @@ static void s_s3_meta_request_request_on_signed( g_aws_signed_body_value_streaming_unsigned_payload_trailer); } +finish: + if (error_code != AWS_ERROR_SUCCESS) { AWS_LOGF_ERROR( diff --git a/tests/s3_mock_server_tests.c b/tests/s3_mock_server_tests.c index 380b07be4..025f06216 100644 --- a/tests/s3_mock_server_tests.c +++ b/tests/s3_mock_server_tests.c @@ -211,6 +211,9 @@ struct aws_http_stream *s_get_requests_header_make_request( return stream; } +/** + * Note: currently (Nov, 2024), S3 don't support create multipart upload anonymously. + */ TEST_CASE(multipart_upload_unsigned_with_trailer_checksum_mock_server) { (void)ctx; @@ -223,12 +226,12 @@ TEST_CASE(multipart_upload_unsigned_with_trailer_checksum_mock_server) { }; ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION)); - - struct aws_s3_client_vtable s3_client_get_requests_header_vtable = g_s3_client_default_vtable; - s3_client_get_requests_header_vtable.http_connection_make_request = s_get_requests_header_make_request; struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config); ASSERT_NOT_NULL(client); - client->vtable = &s3_client_get_requests_header_vtable; + + /* Patch the client vtable to record the request header */ + struct aws_s3_client_vtable *s3_client_get_requests_header_vtable = client->vtable; + s3_client_get_requests_header_vtable->http_connection_make_request = s_get_requests_header_make_request; struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/default"); @@ -311,12 +314,12 @@ TEST_CASE(single_upload_unsigned_with_trailer_checksum_mock_server) { }; ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION)); - - struct aws_s3_client_vtable s3_client_get_requests_header_vtable = g_s3_client_default_vtable; - s3_client_get_requests_header_vtable.http_connection_make_request = s_get_requests_header_make_request; struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config); ASSERT_NOT_NULL(client); - client->vtable = &s3_client_get_requests_header_vtable; + + /* Patch the client vtable to record the request header */ + struct aws_s3_client_vtable *s3_client_get_requests_header_vtable = client->vtable; + s3_client_get_requests_header_vtable->http_connection_make_request = s_get_requests_header_make_request; struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/default"); From 1f1a844c306335eadcaf245777369159fc9844f1 Mon Sep 17 00:00:00 2001 From: Dengke Date: Tue, 26 Nov 2024 16:32:08 -0800 Subject: [PATCH 5/6] address comments --- source/s3_meta_request.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 74f37a519..13c7113c5 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -1048,6 +1048,20 @@ static void s_s3_meta_request_request_on_signed( goto finish; } + /** + * Add "x-amz-content-sha256: STREAMING-UNSIGNED-PAYLOAD-TRAILER" header to support trailing checksum. + */ + if (request->send_data.require_streaming_unsigned_payload_header) { + struct aws_http_headers *headers = aws_http_message_get_headers(request->send_data.message); + if (aws_http_headers_set( + headers, + aws_byte_cursor_from_c_str("x-amz-content-sha256"), + g_aws_signed_body_value_streaming_unsigned_payload_trailer)) { + error_code = aws_last_error_or_unknown(); + goto finish; + } + } + if (request->send_data.metrics) { struct aws_s3_request_metrics *metric = request->send_data.metrics; aws_high_res_clock_get_ticks((uint64_t *)&metric->time_metrics.sign_end_timestamp_ns); @@ -1056,17 +1070,6 @@ static void s_s3_meta_request_request_on_signed( metric->time_metrics.sign_end_timestamp_ns - metric->time_metrics.sign_start_timestamp_ns; } - /** - * Add "x-amz-content-sha256: STREAMING-UNSIGNED-PAYLOAD-TRAILER" header to support trailing checksum. - */ - if (request->send_data.require_streaming_unsigned_payload_header) { - struct aws_http_headers *headers = aws_http_message_get_headers(request->send_data.message); - error_code = aws_http_headers_set( - headers, - aws_byte_cursor_from_c_str("x-amz-content-sha256"), - g_aws_signed_body_value_streaming_unsigned_payload_trailer); - } - finish: if (error_code != AWS_ERROR_SUCCESS) { From dd0f49e09402abefaa8103b343866ad924fcb057 Mon Sep 17 00:00:00 2001 From: Dengke Date: Tue, 26 Nov 2024 16:33:46 -0800 Subject: [PATCH 6/6] add an assertion --- source/s3_meta_request.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 13c7113c5..2982bb40d 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -1053,6 +1053,7 @@ static void s_s3_meta_request_request_on_signed( */ if (request->send_data.require_streaming_unsigned_payload_header) { struct aws_http_headers *headers = aws_http_message_get_headers(request->send_data.message); + AWS_ASSERT(headers != NULL); if (aws_http_headers_set( headers, aws_byte_cursor_from_c_str("x-amz-content-sha256"),