diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index 1fb28f1ca..086cf2296 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -24,20 +24,55 @@ struct aws_s3_request; struct aws_s3_meta_request; struct aws_s3_meta_request_result; +/** + * A Meta Request represents a group of generated requests that are being done on behalf of the + * original request. For example, one large GetObject request can be transformed into a series + * of ranged GetObject requests that are executed in parallel to improve throughput. + * + * The aws_s3_meta_request_type is a hint of transformation to be applied. + */ enum aws_s3_meta_request_type { + + /** + * The Default meta request type sends any request to S3 as-is (with no transformation). For example, + * it can be used to pass a CreateBucket request. + */ AWS_S3_META_REQUEST_TYPE_DEFAULT, + + /** + * The GetObject request will be split into a series of ranged GetObject requests that are + * executed in parallel to improve throughput, when possible. + */ AWS_S3_META_REQUEST_TYPE_GET_OBJECT, + + /** + * The PutObject request will be split into MultiPart uploads that are executed in parallel + * to improve throughput, when possible. + */ AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, AWS_S3_META_REQUEST_TYPE_MAX, }; +/** + * Invoked to provide response headers received during execution of the meta request, both for + * success and error HTTP status codes. + * + * Return AWS_OP_SUCCESS to continue processing the request. + * Return AWS_OP_ERR to indicate failure and cancel the request. + */ typedef int(aws_s3_meta_request_headers_callback_fn)( struct aws_s3_meta_request *meta_request, const struct aws_http_headers *headers, int response_status, void *user_data); +/** + * Invoked to provide the request body as it is received. + * + * Return AWS_OP_SUCCESS to continue processing the request. + * Return AWS_OP_ERR to indicate failure and cancel the request. + */ typedef int(aws_s3_meta_request_receive_body_callback_fn)( /* The meta request that the callback is being issued for. */ struct aws_s3_meta_request *meta_request, @@ -52,6 +87,9 @@ typedef int(aws_s3_meta_request_receive_body_callback_fn)( /* User data specified by aws_s3_meta_request_options.*/ void *user_data); +/** + * Invoked when the entire meta request execution is complete. + */ typedef void(aws_s3_meta_request_finish_fn)( struct aws_s3_meta_request *meta_request, const struct aws_s3_meta_request_result *meta_request_result, @@ -139,13 +177,23 @@ struct aws_s3_meta_request_options { /* User data for all callbacks. */ void *user_data; - /* Callback for receiving incoming headers. */ + /** + * Optional. + * Invoked to provide response headers received during execution of the meta request. + * See `aws_s3_meta_request_headers_callback_fn`. + */ aws_s3_meta_request_headers_callback_fn *headers_callback; - /* Callback for incoming body data. */ + /** + * Invoked to provide the request body as it is received. + * See `aws_s3_meta_request_receive_body_callback_fn`. + */ aws_s3_meta_request_receive_body_callback_fn *body_callback; - /* Callback for when the meta request is completely finished. */ + /** + * Invoked when the entire meta request execution is complete. + * See `aws_s3_meta_request_finish_fn`. + */ aws_s3_meta_request_finish_fn *finish_callback; /* Callback for when the meta request has completely cleaned up. */ diff --git a/source/s3_auto_ranged_get.c b/source/s3_auto_ranged_get.c index 390a83a15..cca62ebee 100644 --- a/source/s3_auto_ranged_get.c +++ b/source/s3_auto_ranged_get.c @@ -569,6 +569,7 @@ static void s_s3_auto_ranged_get_request_finished( error_code = aws_last_error_or_unknown(); } + meta_request->headers_callback = NULL; aws_http_headers_release(response_headers); } diff --git a/source/s3_auto_ranged_put.c b/source/s3_auto_ranged_put.c index d53befc92..82bdad749 100644 --- a/source/s3_auto_ranged_put.c +++ b/source/s3_auto_ranged_put.c @@ -612,6 +612,7 @@ static void s_s3_auto_ranged_put_request_finished( error_code = aws_last_error_or_unknown(); } + meta_request->headers_callback = NULL; aws_http_headers_release(final_response_headers); } diff --git a/source/s3_client.c b/source/s3_client.c index 8fe5a41ab..a9ab6b5c0 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -1397,6 +1397,10 @@ static void s_s3_client_acquired_retry_token( AWS_ASSERT(client->vtable->acquire_http_connection); + /* client needs to be kept alive until s_s3_client_on_acquire_http_connection completes */ + /* TODO: not a blocker, consider managing the life time of aws_s3_client from aws_s3_endpoint to simplify usage */ + aws_s3_client_acquire(client); + client->vtable->acquire_http_connection( endpoint->http_connection_manager, s_s3_client_on_acquire_http_connection, connection); @@ -1444,16 +1448,19 @@ static void s_s3_client_on_acquire_http_connection( connection->http_connection = incoming_http_connection; aws_s3_meta_request_send_request(meta_request, connection); + aws_s3_client_release(client); /* kept since this callback was registered */ return; error_retry: aws_s3_client_notify_connection_finished(client, connection, error_code, AWS_S3_CONNECTION_FINISH_CODE_RETRY); + aws_s3_client_release(client); /* kept since this callback was registered */ return; error_fail: aws_s3_client_notify_connection_finished(client, connection, error_code, AWS_S3_CONNECTION_FINISH_CODE_FAILED); + aws_s3_client_release(client); /* kept since this callback was registered */ } /* Called by aws_s3_meta_request when it has finished using this connection for a single request. */ diff --git a/source/s3_default_meta_request.c b/source/s3_default_meta_request.c index 6cf13cd3f..9289a1a24 100644 --- a/source/s3_default_meta_request.c +++ b/source/s3_default_meta_request.c @@ -255,13 +255,17 @@ static void s_s3_meta_request_default_request_finished( AWS_PRECONDITION(meta_request_default); if (error_code == AWS_ERROR_SUCCESS && meta_request->headers_callback != NULL && - meta_request->headers_callback( - meta_request, - request->send_data.response_headers, - request->send_data.response_status, - meta_request->user_data)) { + request->send_data.response_headers != NULL) { + + if (meta_request->headers_callback( + meta_request, + request->send_data.response_headers, + request->send_data.response_status, + meta_request->user_data)) { + error_code = aws_last_error_or_unknown(); + } - error_code = aws_last_error_or_unknown(); + meta_request->headers_callback = NULL; } aws_s3_meta_request_lock_synced_data(meta_request); diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 10cbc896a..73d541c5b 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -1145,6 +1145,17 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request aws_s3_request_release(release_request); } + if (meta_request->headers_callback && finish_result.error_response_headers) { + if (meta_request->headers_callback( + meta_request, + finish_result.error_response_headers, + finish_result.response_status, + meta_request->user_data)) { + finish_result.error_code = aws_last_error_or_unknown(); + } + meta_request->headers_callback = NULL; + } + AWS_LOGF_DEBUG( AWS_LS_S3_META_REQUEST, "id=%p Meta request finished with error code %d (%s)", diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6237778b4..e7c1502eb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -98,6 +98,11 @@ add_net_test_case(test_s3_put_object_fail_body_callback) add_net_test_case(test_s3_get_object_fail_headers_callback) add_net_test_case(test_s3_get_object_fail_body_callback) add_net_test_case(test_s3_default_fail_headers_callback) +add_net_test_case(test_s3_default_invoke_headers_callback_on_error) +add_net_test_case(test_s3_default_invoke_headers_callback_cancels_on_error) +add_net_test_case(test_s3_get_object_invoke_headers_callback_on_error) +add_net_test_case(test_s3_put_object_invoke_headers_callback_on_error) +add_net_test_case(test_s3_put_object_invoke_headers_callback_on_error_with_user_cancellation) add_net_test_case(test_s3_default_fail_body_callback) add_net_test_case(test_s3_error_missing_file) add_net_test_case(test_s3_existing_host_entry) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 313b49627..27f7a5746 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -2354,6 +2354,7 @@ static int s_test_s3_bad_endpoint(struct aws_allocator *allocator, void *ctx) { aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); aws_http_message_release(message); + aws_s3_client_release(client); client = NULL; @@ -2519,6 +2520,206 @@ static int s_test_s3_default_fail_headers_callback(struct aws_allocator *allocat return 0; } +static struct aws_atomic_var s_test_headers_callback_invoked; + +static int s_s3_test_headers_callback_check_returns_success( + struct aws_s3_meta_request *meta_request, + const struct aws_http_headers *headers, + int response_status, + void *user_data) { + (void)meta_request; + (void)headers; + (void)response_status; + (void)user_data; + + /* increments counter to check if callback was invoked exactly once */ + aws_atomic_fetch_add(&s_test_headers_callback_invoked, 1); + + return AWS_OP_SUCCESS; +} + +static int s_s3_test_headers_callback_check_returns_error( + struct aws_s3_meta_request *meta_request, + const struct aws_http_headers *headers, + int response_status, + void *user_data) { + (void)meta_request; + (void)headers; + (void)response_status; + (void)user_data; + + /* increments counter to check if callback was invoked exactly once */ + aws_atomic_fetch_add(&s_test_headers_callback_invoked, 1); + + aws_raise_error(AWS_ERROR_UNKNOWN); + return AWS_OP_ERR; +} + +AWS_TEST_CASE(test_s3_default_invoke_headers_callback_on_error, s_test_s3_default_invoke_headers_callback_on_error) +static int s_test_s3_default_invoke_headers_callback_on_error(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + aws_atomic_init_int(&s_test_headers_callback_invoked, 0); + + struct aws_byte_cursor invalid_path = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("___INVALID_PATH___"); + + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_DEFAULT, + .headers_callback = s_s3_test_headers_callback_check_returns_success, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .default_type_options = + { + .mode = AWS_S3_TESTER_DEFAULT_TYPE_MODE_GET, + }, + .get_options = + { + .object_path = invalid_path, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results)); + ASSERT_INT_EQUALS(aws_atomic_load_int(&s_test_headers_callback_invoked), 1); + ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_S3_INVALID_RESPONSE_STATUS); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + + return 0; +} + +AWS_TEST_CASE( + test_s3_default_invoke_headers_callback_cancels_on_error, + s_test_s3_default_invoke_headers_callback_cancels_on_error) +static int s_test_s3_default_invoke_headers_callback_cancels_on_error(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + aws_atomic_init_int(&s_test_headers_callback_invoked, 0); + + struct aws_byte_cursor invalid_path = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("___INVALID_PATH___"); + + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_DEFAULT, + .headers_callback = s_s3_test_headers_callback_check_returns_error, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .default_type_options = + { + .mode = AWS_S3_TESTER_DEFAULT_TYPE_MODE_GET, + }, + .get_options = + { + .object_path = invalid_path, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results)); + ASSERT_INT_EQUALS(aws_atomic_load_int(&s_test_headers_callback_invoked), 1); + ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_UNKNOWN); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + + return 0; +} + +AWS_TEST_CASE( + test_s3_get_object_invoke_headers_callback_on_error, + s_test_s3_get_object_invoke_headers_callback_on_error) +static int s_test_s3_get_object_invoke_headers_callback_on_error(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + aws_atomic_init_int(&s_test_headers_callback_invoked, 0); + + struct aws_byte_cursor invalid_path = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("___INVALID_PATH___"); + + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT, + .headers_callback = s_s3_test_headers_callback_check_returns_success, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .get_options = + { + .object_path = invalid_path, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results)); + ASSERT_INT_EQUALS(aws_atomic_load_int(&s_test_headers_callback_invoked), 1); + ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_S3_INVALID_RESPONSE_STATUS); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + + return 0; +} + +AWS_TEST_CASE( + test_s3_put_object_invoke_headers_callback_on_error, + s_test_s3_put_object_invoke_headers_callback_on_error) +static int s_test_s3_put_object_invoke_headers_callback_on_error(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + aws_atomic_init_int(&s_test_headers_callback_invoked, 0); + + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .headers_callback = s_s3_test_headers_callback_check_returns_success, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .put_options = + { + .object_size_mb = 10, + .invalid_request = true, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results)); + ASSERT_INT_EQUALS(aws_atomic_load_int(&s_test_headers_callback_invoked), 1); + ASSERT_UINT_EQUALS(meta_request_test_results.finished_error_code, AWS_ERROR_S3_INVALID_RESPONSE_STATUS); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + + return 0; +} + +AWS_TEST_CASE( + test_s3_put_object_invoke_headers_callback_on_error_with_user_cancellation, + s_test_s3_put_object_invoke_headers_callback_on_error_with_user_cancellation) +static int s_test_s3_put_object_invoke_headers_callback_on_error_with_user_cancellation( + struct aws_allocator *allocator, + void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + aws_atomic_init_int(&s_test_headers_callback_invoked, 0); + + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .headers_callback = s_s3_test_headers_callback_check_returns_error, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .put_options = + { + .ensure_multipart = true, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results)); + ASSERT_INT_EQUALS(aws_atomic_load_int(&s_test_headers_callback_invoked), 1); + ASSERT_UINT_EQUALS(meta_request_test_results.finished_error_code, AWS_ERROR_UNKNOWN); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + + return 0; +} + AWS_TEST_CASE(test_s3_default_fail_body_callback, s_test_s3_default_fail_body_callback) static int s_test_s3_default_fail_body_callback(struct aws_allocator *allocator, void *ctx) { (void)ctx; @@ -2943,7 +3144,9 @@ static int s_range_requests_headers_callback( struct aws_s3_meta_request_test_results *test_results = user_data; struct range_requests_test_user_data *test_user_data = test_results->tester->user_data; - copy_http_headers(headers, test_user_data->headers); + if (test_user_data != NULL) { + copy_http_headers(headers, test_user_data->headers); + } return AWS_OP_SUCCESS; }