From 57c149234239643b944dcaa12189be6f7437393d Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:21:02 -0800 Subject: [PATCH 1/8] if none match --- source/s3_request_messages.c | 5 +++ tests/CMakeLists.txt | 2 + tests/s3_data_plane_tests.c | 86 ++++++++++++++++++++++++++++++++++++ tests/s3_tester.c | 7 +++ tests/s3_tester.h | 1 + 5 files changed, 101 insertions(+) diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index 4b678df2..a405ffda 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -20,6 +20,7 @@ const struct aws_byte_cursor g_s3_create_multipart_upload_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("Content-MD5"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_create_multipart_upload_excluded_headers_count = @@ -49,6 +50,7 @@ const struct aws_byte_cursor g_s3_upload_part_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-mode"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-retain-until-date"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_upload_part_excluded_headers_count = AWS_ARRAY_SIZE(g_s3_upload_part_excluded_headers); @@ -147,6 +149,7 @@ const struct aws_byte_cursor g_s3_list_parts_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_list_parts_excluded_headers_count = AWS_ARRAY_SIZE(g_s3_list_parts_excluded_headers); @@ -177,6 +180,7 @@ const struct aws_byte_cursor g_s3_list_parts_with_checksum_excluded_headers[] = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_list_parts_with_checksum_excluded_headers_count = @@ -211,6 +215,7 @@ const struct aws_byte_cursor g_s3_abort_multipart_upload_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; static const struct aws_byte_cursor s_x_amz_meta_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-meta-"); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b3466fb9..eeba5228 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -140,6 +140,8 @@ add_net_test_case(test_s3_put_object_async_no_content_length_1part) add_net_test_case(test_s3_put_object_async_no_content_length_empty_part2) add_net_test_case(test_s3_put_object_async_no_content_length_2parts) add_net_test_case(test_s3_put_object_async_fail_reading) +add_net_test_case(test_s3_put_object_if_none_match) +add_net_test_case(test_s3_put_object_mpu_if_none_match) add_net_test_case(test_s3_many_async_uploads_without_data) add_net_test_case(test_s3_download_empty_file_with_checksum) add_net_test_case(test_s3_download_empty_file_with_checksum_header) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 4154f515..1ce1271a 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -3010,6 +3010,92 @@ static int s_test_s3_put_object_async_fail_reading(struct aws_allocator *allocat return 0; } +AWS_TEST_CASE(test_s3_put_object_if_none_match, s_test_s3_put_object_if_none_match) +static int s_test_s3_put_object_if_none_match(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results test_results; + aws_s3_meta_request_test_results_init(&test_results, allocator); + struct aws_byte_cursor if_none_match_all = aws_byte_cursor_from_c_str("*"); + struct aws_s3_tester_meta_request_options put_options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .put_options = + { + /* Use pre_exist object so that the request should fail with the expected failure message. */ + .object_path_override = g_pre_existing_object_10MB, + .object_size_mb = 5, + .if_none_match_header = if_none_match_all, + }, + }; + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); + + /** + * response body should be like: + * + * PreconditionFailed + * At least one of the pre-conditions you specified did not hold + * If-None-Match + * + * + * + */ + ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results.finished_response_status); + + struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results.error_response_body); + struct aws_byte_cursor condition_string = {0}; + const char *xml_path[] = {"Error", "Condition", NULL}; + ASSERT_SUCCESS(aws_xml_get_body_at_path(allocator, xml_doc, xml_path, &condition_string)); + ASSERT_TRUE(aws_byte_cursor_eq_c_str(&condition_string, "If-None-Match")); + + aws_s3_meta_request_test_results_clean_up(&test_results); + return 0; +} + +AWS_TEST_CASE(test_s3_put_object_mpu_if_none_match, s_test_s3_put_object_mpu_if_none_match) +static int s_test_s3_put_object_mpu_if_none_match(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_meta_request_test_results test_results; + aws_s3_meta_request_test_results_init(&test_results, allocator); + struct aws_byte_cursor if_none_match_all = aws_byte_cursor_from_c_str("*"); + struct aws_s3_tester_meta_request_options put_options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .put_options = + { + /* Use pre_exist object so that the request should fail with the expected failure message. */ + .object_path_override = g_pre_existing_object_10MB, + .object_size_mb = 20, + .if_none_match_header = if_none_match_all, + }, + }; + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); + + /** + * response body should be like: + * + * PreconditionFailed + * At least one of the pre-conditions you specified did not hold + * If-None-Match + * + * + * + */ + ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results.finished_response_status); + + struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results.error_response_body); + struct aws_byte_cursor condition_string = {0}; + const char *xml_path[] = {"Error", "Condition", NULL}; + ASSERT_SUCCESS(aws_xml_get_body_at_path(allocator, xml_doc, xml_path, &condition_string)); + ASSERT_TRUE(aws_byte_cursor_eq_c_str(&condition_string, "If-None-Match")); + + aws_s3_meta_request_test_results_clean_up(&test_results); + return 0; +} + AWS_TEST_CASE(test_s3_put_object_sse_kms, s_test_s3_put_object_sse_kms) static int s_test_s3_put_object_sse_kms(struct aws_allocator *allocator, void *ctx) { (void)ctx; diff --git a/tests/s3_tester.c b/tests/s3_tester.c index d0283094..d879f385 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -1696,6 +1696,13 @@ int aws_s3_tester_send_meta_request_with_options( aws_http_message_add_header(message, content_encoding_header); } + if (options->put_options.if_none_match_header.ptr != NULL) { + struct aws_http_header if_none_match_header = { + .name = aws_byte_cursor_from_c_str("if-none-match"), + .value = options->put_options.if_none_match_header, + }; + aws_http_message_add_header(message, if_none_match_header); + } meta_request_options.message = message; aws_byte_buf_clean_up(&object_path_buffer); } diff --git a/tests/s3_tester.h b/tests/s3_tester.h index 5c1c41e8..03b1f1f6 100644 --- a/tests/s3_tester.h +++ b/tests/s3_tester.h @@ -219,6 +219,7 @@ struct aws_s3_tester_meta_request_options { size_t content_length; bool skip_content_length; struct aws_byte_cursor content_encoding; + struct aws_byte_cursor if_none_match_header; } put_options; enum aws_s3_tester_sse_type sse_type; From 9dc17d0499caea23d1120c4bf6fe0aba5cb0b02b Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:22:05 -0800 Subject: [PATCH 2/8] don't include it for the list part --- source/s3_request_messages.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index a405ffda..66330547 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -149,7 +149,6 @@ const struct aws_byte_cursor g_s3_list_parts_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), - AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_list_parts_excluded_headers_count = AWS_ARRAY_SIZE(g_s3_list_parts_excluded_headers); @@ -180,7 +179,6 @@ const struct aws_byte_cursor g_s3_list_parts_with_checksum_excluded_headers[] = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), - AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; const size_t g_s3_list_parts_with_checksum_excluded_headers_count = From fcc2617be75a68fe147aa07289dfc15d2e6cbb7f Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:29:53 -0800 Subject: [PATCH 3/8] share the code --- tests/s3_data_plane_tests.c | 67 +++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 1ce1271a..6ec80ad5 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -3010,6 +3010,35 @@ static int s_test_s3_put_object_async_fail_reading(struct aws_allocator *allocat return 0; } +static int s_test_validate_if_none_match_failure_response(struct aws_s3_meta_request_test_results *test_results) { + + /** + * response body should be like: + * + * PreconditionFailed + * At least one of the pre-conditions you specified did not hold + * If-None-Match + * + * + * + */ + ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results->finished_response_status); + + struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results->error_response_body); + struct aws_byte_cursor error_code_string = {0}; + struct aws_byte_cursor condition_string = {0}; + + const char *error_code_path[] = {"Error", "Code", NULL}; + ASSERT_SUCCESS(aws_xml_get_body_at_path(test_results->allocator, xml_doc, error_code_path, &error_code_string)); + ASSERT_TRUE(aws_byte_cursor_eq_c_str(&error_code_string, "PreconditionFailed")); + + const char *condition_path[] = {"Error", "Condition", NULL}; + ASSERT_SUCCESS(aws_xml_get_body_at_path(test_results->allocator, xml_doc, condition_path, &condition_string)); + ASSERT_TRUE(aws_byte_cursor_eq_c_str(&condition_string, "If-None-Match")); + + return AWS_OP_SUCCESS; +} + AWS_TEST_CASE(test_s3_put_object_if_none_match, s_test_s3_put_object_if_none_match) static int s_test_s3_put_object_if_none_match(struct aws_allocator *allocator, void *ctx) { (void)ctx; @@ -3030,24 +3059,7 @@ static int s_test_s3_put_object_if_none_match(struct aws_allocator *allocator, v }, }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); - - /** - * response body should be like: - * - * PreconditionFailed - * At least one of the pre-conditions you specified did not hold - * If-None-Match - * - * - * - */ - ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results.finished_response_status); - - struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results.error_response_body); - struct aws_byte_cursor condition_string = {0}; - const char *xml_path[] = {"Error", "Condition", NULL}; - ASSERT_SUCCESS(aws_xml_get_body_at_path(allocator, xml_doc, xml_path, &condition_string)); - ASSERT_TRUE(aws_byte_cursor_eq_c_str(&condition_string, "If-None-Match")); + ASSERT_SUCCESS(s_test_validate_if_none_match_failure_response(&test_results)); aws_s3_meta_request_test_results_clean_up(&test_results); return 0; @@ -3073,24 +3085,7 @@ static int s_test_s3_put_object_mpu_if_none_match(struct aws_allocator *allocato }, }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); - - /** - * response body should be like: - * - * PreconditionFailed - * At least one of the pre-conditions you specified did not hold - * If-None-Match - * - * - * - */ - ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results.finished_response_status); - - struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results.error_response_body); - struct aws_byte_cursor condition_string = {0}; - const char *xml_path[] = {"Error", "Condition", NULL}; - ASSERT_SUCCESS(aws_xml_get_body_at_path(allocator, xml_doc, xml_path, &condition_string)); - ASSERT_TRUE(aws_byte_cursor_eq_c_str(&condition_string, "If-None-Match")); + ASSERT_SUCCESS(s_test_validate_if_none_match_failure_response(&test_results)); aws_s3_meta_request_test_results_clean_up(&test_results); return 0; From 62a13ae4554771400e50a95591bb1d709b88d78a Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:30:36 -0800 Subject: [PATCH 4/8] tirivial --- tests/s3_data_plane_tests.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 6ec80ad5..67feb4ec 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -3012,6 +3012,8 @@ static int s_test_s3_put_object_async_fail_reading(struct aws_allocator *allocat static int s_test_validate_if_none_match_failure_response(struct aws_s3_meta_request_test_results *test_results) { + ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results->finished_response_status); + /** * response body should be like: * @@ -3022,7 +3024,6 @@ static int s_test_validate_if_none_match_failure_response(struct aws_s3_meta_req * * */ - ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results->finished_response_status); struct aws_byte_cursor xml_doc = aws_byte_cursor_from_buf(&test_results->error_response_body); struct aws_byte_cursor error_code_string = {0}; From 065662c704e240b2c87006db138c48eec752df83 Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:32:10 -0800 Subject: [PATCH 5/8] add the cancel older runs when change pushed --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3be2c6fd..d5f54edc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,11 @@ on: branches-ignore: - 'main' +# Allow one instance of this workflow per pull request, and cancel older runs when new changes are pushed +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: true + env: BUILDER_VERSION: v0.9.64 BUILDER_SOURCE: releases From 3eb9d234dcd2c78686d082fd667a8b830aaa473a Mon Sep 17 00:00:00 2001 From: Dengke Date: Thu, 21 Nov 2024 10:33:37 -0800 Subject: [PATCH 6/8] test-cancel From cf1d945fc67dbdb0700d6a02ff8e398fa6559738 Mon Sep 17 00:00:00 2001 From: Dengke Date: Fri, 22 Nov 2024 14:52:20 -0800 Subject: [PATCH 7/8] address comments --- .github/workflows/ci.yml | 4 ++-- tests/s3_data_plane_tests.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5f54edc..045abcc4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,9 +5,9 @@ on: branches-ignore: - 'main' -# Allow one instance of this workflow per pull request, and cancel older runs when new changes are pushed +# cancel in-progress builds after a new commit concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true env: diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 67feb4ec..0516cc2b 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -3054,8 +3054,8 @@ static int s_test_s3_put_object_if_none_match(struct aws_allocator *allocator, v .put_options = { /* Use pre_exist object so that the request should fail with the expected failure message. */ - .object_path_override = g_pre_existing_object_10MB, - .object_size_mb = 5, + .object_path_override = g_pre_existing_object_1MB, + .object_size_mb = 1, .if_none_match_header = if_none_match_all, }, }; @@ -3081,7 +3081,7 @@ static int s_test_s3_put_object_mpu_if_none_match(struct aws_allocator *allocato { /* Use pre_exist object so that the request should fail with the expected failure message. */ .object_path_override = g_pre_existing_object_10MB, - .object_size_mb = 20, + .object_size_mb = 10, .if_none_match_header = if_none_match_all, }, }; From fe4eab90e1aeb0c2e5f23f118f0d1511f136540c Mon Sep 17 00:00:00 2001 From: Dengke Date: Mon, 25 Nov 2024 16:11:47 -0800 Subject: [PATCH 8/8] tolerate 200 error --- tests/s3_data_plane_tests.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index 0516cc2b..c2ab1c62 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -3012,8 +3012,6 @@ static int s_test_s3_put_object_async_fail_reading(struct aws_allocator *allocat static int s_test_validate_if_none_match_failure_response(struct aws_s3_meta_request_test_results *test_results) { - ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results->finished_response_status); - /** * response body should be like: * @@ -3060,6 +3058,8 @@ static int s_test_s3_put_object_if_none_match(struct aws_allocator *allocator, v }, }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); + + ASSERT_UINT_EQUALS(AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED, test_results.finished_response_status); ASSERT_SUCCESS(s_test_validate_if_none_match_failure_response(&test_results)); aws_s3_meta_request_test_results_clean_up(&test_results); @@ -3086,6 +3086,11 @@ static int s_test_s3_put_object_mpu_if_none_match(struct aws_allocator *allocato }, }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &put_options, &test_results)); + + /** Complete MPU can fail with 200 error */ + ASSERT_TRUE( + AWS_HTTP_STATUS_CODE_412_PRECONDITION_FAILED == test_results.finished_response_status || + AWS_HTTP_STATUS_CODE_200_OK == test_results.finished_response_status); ASSERT_SUCCESS(s_test_validate_if_none_match_failure_response(&test_results)); aws_s3_meta_request_test_results_clean_up(&test_results);