More S3 client test (#255)

Author: TingDaoK

.builder/actions/mock_server_setup.py

@@ -22,7 +22,7 @@ def run(self, env):
         python_path = sys.executable
         # install dependency for mock server
         self.env.shell.exec(python_path,
-                            '-m', 'pip', 'install', 'h11', 'trio', check=True)
+                            '-m', 'pip', 'install', 'h11', 'trio', 'proxy.py', check=True)
         # check the deps can be import correctly
         self.env.shell.exec(python_path,
                             '-c', 'import h11, trio', check=True)
@@ -34,8 +34,16 @@ def run(self, env):
         base_dir = os.path.dirname(os.path.realpath(__file__))
         dir = os.path.join(base_dir, "..", "..", "tests", "mock_s3_server")
 
-        p = subprocess.Popen([python_path, "mock_s3_server.py"], cwd=dir)
+        p1 = subprocess.Popen([python_path, "mock_s3_server.py"], cwd=dir)
+        try:
+            p2 = subprocess.Popen("proxy", cwd=dir)
+        except Exception as e:
+            # Okay for proxy to fail starting up as it may not be in the path
+            print(e)
+            p2 = None
 
         @atexit.register
         def close_mock_server():
-            p.terminate()
+            p1.terminate()
+            if p2 != None:
+                p2.terminate()

include/aws/s3/private/s3_request_messages.h

@@ -40,7 +40,7 @@ struct aws_http_message *aws_s3_message_util_copy_http_message_no_body_filter_he
 /* Copy headers from one message to the other and exclude specific headers.
  * exclude_x_amz_meta controls whether S3 user metadata headers (prefixed with "x-amz-meta) are excluded.*/
 AWS_S3_API
-int aws_s3_message_util_copy_headers(
+void aws_s3_message_util_copy_headers(
     struct aws_http_message *source_message,
     struct aws_http_message *dest_message,
     const struct aws_byte_cursor *excluded_headers_arrays,

source/s3_client.c

@@ -668,13 +668,7 @@ int s_apply_endpoint_override(
     }
 
     struct aws_byte_cursor host_value;
-    if (aws_http_headers_get(message_headers, g_host_header_name, &host_value)) {
-        AWS_LOGF_ERROR(
-            AWS_LS_S3_CLIENT,
-            "id=%p Cannot create meta s3 request; message provided in options does not have a 'Host' header.",
-            (void *)client);
-        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
-    }
+    AWS_FATAL_ASSERT(aws_http_headers_get(message_headers, g_host_header_name, &host_value) == AWS_OP_SUCCESS);
 
     if (endpoint_authority != NULL && !aws_byte_cursor_eq(&host_value, endpoint_authority)) {
         AWS_LOGF_ERROR(

source/s3_request_messages.c

@@ -266,7 +266,7 @@ struct aws_http_message *aws_s3_create_multipart_upload_message_new(
         false /*exclude_x_amz_meta*/);
 
     if (message == NULL) {
-        goto error_clean_up;
+        return NULL;
     }
 
     if (aws_s3_message_util_set_multipart_request_path(allocator, NULL, 0, true, message)) {
@@ -299,12 +299,7 @@ struct aws_http_message *aws_s3_create_multipart_upload_message_new(
     return message;
 
 error_clean_up:
-
-    if (message != NULL) {
-        aws_http_message_release(message);
-        message = NULL;
-    }
-
+    aws_http_message_release(message);
     return NULL;
 }
 
@@ -322,6 +317,7 @@ struct aws_http_message *aws_s3_upload_part_message_new(
     AWS_PRECONDITION(allocator);
     AWS_PRECONDITION(base_message);
     AWS_PRECONDITION(part_number > 0);
+    AWS_PRECONDITION(buffer);
 
     struct aws_http_message *message = aws_s3_message_util_copy_http_message_no_body_filter_headers(
         allocator,
@@ -331,40 +327,30 @@ struct aws_http_message *aws_s3_upload_part_message_new(
         true /*exclude_x_amz_meta*/);
 
     if (message == NULL) {
-        goto error_clean_up;
+        return NULL;
     }
 
     if (aws_s3_message_util_set_multipart_request_path(allocator, upload_id, part_number, false, message)) {
         goto error_clean_up;
     }
 
-    if (buffer != NULL) {
+    if (aws_s3_message_util_assign_body(allocator, buffer, message, checksum_config, encoded_checksum_output) == NULL) {
+        goto error_clean_up;
+    }
 
-        if (aws_s3_message_util_assign_body(allocator, buffer, message, checksum_config, encoded_checksum_output) ==
-            NULL) {
-            goto error_clean_up;
-        }
-        if (should_compute_content_md5) {
-            if (!checksum_config || checksum_config->location == AWS_SCL_NONE) {
-                /* MD5 will be skiped if flexible checksum used */
-                if (aws_s3_message_util_add_content_md5_header(allocator, buffer, message)) {
-                    goto error_clean_up;
-                }
+    if (should_compute_content_md5) {
+        if (!checksum_config || checksum_config->location == AWS_SCL_NONE) {
+            /* MD5 will be skiped if flexible checksum used */
+            if (aws_s3_message_util_add_content_md5_header(allocator, buffer, message)) {
+                goto error_clean_up;
             }
         }
-    } else {
-        goto error_clean_up;
     }
 
     return message;
 
 error_clean_up:
-
-    if (message != NULL) {
-        aws_http_message_release(message);
-        message = NULL;
-    }
-
+    aws_http_message_release(message);
     return NULL;
 }
 
@@ -900,10 +886,7 @@ struct aws_http_message *aws_s3_message_util_copy_http_message_no_body_filter_he
     AWS_PRECONDITION(base_message);
 
     struct aws_http_message *message = aws_http_message_new_request(allocator);
-
-    if (message == NULL) {
-        return NULL;
-    }
+    AWS_ASSERT(message);
 
     struct aws_byte_cursor request_method;
     if (aws_http_message_get_request_method(base_message, &request_method)) {
@@ -925,24 +908,17 @@ struct aws_http_message *aws_s3_message_util_copy_http_message_no_body_filter_he
         goto error_clean_up;
     }
 
-    if (aws_s3_message_util_copy_headers(
-            base_message, message, excluded_header_array, excluded_header_array_size, exclude_x_amz_meta)) {
-        goto error_clean_up;
-    }
+    aws_s3_message_util_copy_headers(
+        base_message, message, excluded_header_array, excluded_header_array_size, exclude_x_amz_meta);
 
     return message;
 
 error_clean_up:
-
-    if (message != NULL) {
-        aws_http_message_release(message);
-        message = NULL;
-    }
-
+    aws_http_message_release(message);
     return NULL;
 }
 
-int aws_s3_message_util_copy_headers(
+void aws_s3_message_util_copy_headers(
     struct aws_http_message *source_message,
     struct aws_http_message *dest_message,
     const struct aws_byte_cursor *excluded_header_array,
@@ -954,9 +930,7 @@ int aws_s3_message_util_copy_headers(
     for (size_t header_index = 0; header_index < num_headers; ++header_index) {
         struct aws_http_header header;
 
-        if (aws_http_message_get_header(source_message, &header, header_index)) {
-            return AWS_OP_ERR;
-        }
+        int error = aws_http_message_get_header(source_message, &header, header_index);
 
         if (excluded_header_array && excluded_header_array_size > 0) {
             bool exclude_header = false;
@@ -979,12 +953,10 @@ int aws_s3_message_util_copy_headers(
             }
         }
 
-        if (aws_http_message_add_header(dest_message, header)) {
-            return AWS_OP_ERR;
-        }
+        error |= aws_http_message_add_header(dest_message, header);
+        (void)error;
+        AWS_ASSERT(!error);
     }
-
-    return AWS_OP_SUCCESS;
 }
 
 /* Add a range header.*/

tests/CMakeLists.txt

@@ -20,10 +20,12 @@ add_test_case(test_s3_ranged_get_object_message_new)
 add_test_case(test_s3_set_multipart_request_path)
 add_test_case(test_s3_create_multipart_upload_message_new)
 add_test_case(test_s3_upload_part_message_new)
+add_test_case(test_s3_upload_part_message_fail)
 add_test_case(test_s3_complete_multipart_message_new)
 add_test_case(test_s3_abort_multipart_upload_message_new)
 
 add_net_test_case(test_s3_client_create_destroy)
+add_net_test_case(test_s3_client_create_error)
 add_net_test_case(test_s3_client_monitoring_options_override)
 add_net_test_case(test_s3_client_proxy_ev_settings_override)
 add_net_test_case(test_s3_client_tcp_keep_alive_options_override)
@@ -235,11 +237,14 @@ if(ENABLE_MOCK_SERVER_TESTS)
     add_net_test_case(resume_mutli_page_list_parts_mock_server)
     add_net_test_case(resume_list_parts_failed_mock_server)
     add_net_test_case(resume_after_finished_mock_server)
+    add_net_test_case(multipart_upload_proxy_mock_server)
+    add_net_test_case(endpoint_override_mock_server)
 endif()
 
 add_net_test_case(meta_request_auto_ranged_get_new_error_handling)
 add_net_test_case(meta_request_auto_ranged_put_new_error_handling)
 add_net_test_case(bad_request_error_handling)
+add_net_test_case(make_meta_request_error_handling)
 
 set(TEST_BINARY_NAME ${PROJECT_NAME}-tests)
 generate_test_driver(${TEST_BINARY_NAME})

tests/s3_data_plane_tests.c

@@ -50,6 +50,31 @@ static int s_test_s3_client_create_destroy(struct aws_allocator *allocator, void
     return 0;
 }
 
+AWS_TEST_CASE(test_s3_client_create_error, s_test_s3_client_create_error)
+static int s_test_s3_client_create_error(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_tester tester;
+    AWS_ZERO_STRUCT(tester);
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+
+    struct aws_s3_client_config client_config;
+    AWS_ZERO_STRUCT(client_config);
+    struct aws_http_proxy_options proxy_options = {
+        .connection_type = AWS_HPCT_HTTP_LEGACY,
+        .host = aws_byte_cursor_from_c_str("localhost"),
+        .port = 8899,
+    };
+    client_config.proxy_options = &proxy_options;
+    struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config);
+
+    ASSERT_TRUE(client == NULL);
+
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}
+
 AWS_TEST_CASE(test_s3_client_monitoring_options_override, s_test_s3_client_monitoring_options_override)
 static int s_test_s3_client_monitoring_options_override(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
@@ -1713,13 +1738,9 @@ static int s_test_s3_put_object_multiple(struct aws_allocator *allocator, void *
         struct aws_byte_cursor test_body_cursor = aws_byte_cursor_from_buf(&input_stream_buffers[i]);
         input_streams[i] = aws_input_stream_new_from_cursor(allocator, &test_body_cursor);
         struct aws_byte_cursor test_object_path = aws_byte_cursor_from_c_str(object_path_buffer);
+        struct aws_byte_cursor host_cur = aws_byte_cursor_from_string(host_name);
         messages[i] = aws_s3_test_put_object_request_new(
-            allocator,
-            aws_byte_cursor_from_string(host_name),
-            test_object_path,
-            g_test_body_content_type,
-            input_streams[i],
-            0);
+            allocator, &host_cur, test_object_path, g_test_body_content_type, input_streams[i], 0);
         struct aws_s3_meta_request_options options;
         AWS_ZERO_STRUCT(options);
         options.type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT;
@@ -2293,7 +2314,7 @@ static int s_test_s3_upload_part_message_helper(struct aws_allocator *allocator,
 
     /* Put together a simple S3 Put Object request. */
     struct aws_http_message *base_message = aws_s3_test_put_object_request_new(
-        allocator, host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
+        allocator, &host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
 
     uint32_t part_number = 1;
     struct aws_string *upload_id = aws_string_new_from_c_str(allocator, "dummy_upload_id");
@@ -2366,7 +2387,7 @@ static int s_test_s3_create_multipart_upload_message_with_content_md5(struct aws
 
     /* Put together a simple S3 Put Object request. */
     struct aws_http_message *base_message = aws_s3_test_put_object_request_new(
-        allocator, host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
+        allocator, &host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
 
     struct aws_http_header content_md5_header = {
         .name = g_content_md5_header_name,
@@ -2415,7 +2436,7 @@ static int s_test_s3_complete_multipart_message_with_content_md5(struct aws_allo
 
     /* Put together a simple S3 Put Object request. */
     struct aws_http_message *base_message = aws_s3_test_put_object_request_new(
-        allocator, host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
+        allocator, &host_name, test_object_path, g_test_body_content_type, input_stream, AWS_S3_TESTER_SSE_NONE);
 
     struct aws_http_header content_md5_header = {
         .name = g_content_md5_header_name,

tests/s3_meta_request_test.c

@@ -153,3 +153,59 @@ TEST_CASE(bad_request_error_handling) {
 
     return 0;
 }
+
+TEST_CASE(make_meta_request_error_handling) {
+    /* The original request without method and path. */
+    (void)ctx;
+    struct aws_http_message *message = aws_http_message_new_request(allocator);
+    ASSERT_SUCCESS(aws_http_message_set_request_method(message, aws_http_method_get));
+    ASSERT_SUCCESS(aws_http_message_set_request_path(message, aws_byte_cursor_from_c_str("/")));
+    struct aws_s3_tester tester;
+    AWS_ZERO_STRUCT(tester);
+    ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
+    struct aws_s3_client *client = NULL;
+    struct aws_s3_tester_client_options client_options = {
+        .part_size = 5 * 1024 * 1024,
+    };
+
+    ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));
+
+    /* 1. Bad options type */
+    struct aws_s3_meta_request_options options;
+    AWS_ZERO_STRUCT(options);
+    options.type = AWS_S3_META_REQUEST_TYPE_MAX;
+
+    struct aws_s3_meta_request *meta_request = aws_s3_client_make_meta_request(client, &options);
+    ASSERT_NULL(meta_request);
+    /* 2. No message */
+    options.type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT;
+
+    meta_request = aws_s3_client_make_meta_request(client, &options);
+    ASSERT_NULL(meta_request);
+
+    /* 3. No message header */
+    options.type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT;
+    options.message = message;
+
+    meta_request = aws_s3_client_make_meta_request(client, &options);
+    ASSERT_NULL(meta_request);
+
+    /* 4. Bad host name */
+    struct aws_http_header host_header = {
+        .name = g_host_header_name,
+        .value = aws_byte_cursor_from_c_str("invalid:/s3.us-east-1.amazonaws.com"),
+    };
+    ASSERT_SUCCESS(aws_http_message_add_header(message, host_header));
+
+    options.type = AWS_S3_META_REQUEST_TYPE_GET_OBJECT;
+    options.message = message;
+
+    meta_request = aws_s3_client_make_meta_request(client, &options);
+    ASSERT_NULL(meta_request);
+
+    aws_http_message_release(message);
+    aws_s3_client_release(client);
+    aws_s3_tester_clean_up(&tester);
+
+    return 0;
+}