BREAKING CHANGE: operation_name must be set for DEFAULT meta-requests

include/aws/s3/private/s3_default_meta_request.h

@@ -18,7 +18,7 @@ struct aws_s3_meta_request_default {
     /* Actual type for the single request (may be AWS_S3_REQUEST_TYPE_UNKNOWN) */
     enum aws_s3_request_type request_type;
 
-    /* S3 operation name for the single request (NULL if unknown) */
+    /* S3 operation name for the single request */
     struct aws_string *operation_name;
 
     /* Members to only be used when the mutex in the base type is locked. */

include/aws/s3/private/s3_paginator.h

@@ -82,6 +82,11 @@ struct aws_s3_paginator_params {
  * Parameters for initiating paginated operation. All values are copied out or re-seated and reference counted.
  */
 struct aws_s3_paginated_operation_params {
+    /**
+     * The S3 operation name (e.g. "ListParts"). Must not be empty.
+     */
+    struct aws_byte_cursor operation_name;
+
     /**
      * Name of the top level result node. Must not be empty.
      */

include/aws/s3/private/s3_request.h

@@ -85,7 +85,7 @@ struct aws_s3_request_metrics {
         struct aws_string *host_address;
         /* The the request ID header value. */
         struct aws_string *request_id;
-        /* S3 operation name for the request (NULL if unknown) */
+        /* S3 operation name for the request */
         struct aws_string *operation_name;
         /* The type of request made */
         enum aws_s3_request_type request_type;
@@ -185,7 +185,7 @@ struct aws_s3_request {
     /* Actual S3 type for the single request (may be AWS_S3_REQUEST_TYPE_UNKNOWN) */
     enum aws_s3_request_type request_type;
 
-    /* S3 operation name for the single request (e.g. "CompleteMultipartUpload") (NULL if unknown) */
+    /* S3 operation name for the single request (e.g. "CompleteMultipartUpload") */
     struct aws_string *operation_name;
 
     /* Members of this structure will be repopulated each time the request is sent. If the request fails, and needs to

include/aws/s3/private/s3_util.h

@@ -10,7 +10,7 @@
 
 #include <aws/auth/signing_config.h>
 #include <aws/common/byte_buf.h>
-#include <aws/s3/s3.h>
+#include <aws/s3/s3_client.h>
 
 #if ASSERT_LOCK_HELD
 #    define ASSERT_SYNCED_DATA_LOCK_HELD(object)                                                                       \
@@ -175,6 +175,12 @@ extern const struct aws_byte_cursor g_delete_method;
 AWS_S3_API
 extern const uint32_t g_s3_max_num_upload_parts;
 
+/**
+ * Returns AWS_S3_REQUEST_TYPE_UNKNOWN if name doesn't map to an enum value.
+ */
+AWS_S3_API
+enum aws_s3_request_type aws_s3_request_type_from_operation_name(struct aws_byte_cursor name);
+
 /**
  * Cache and initial the signing config based on the client.
  *

include/aws/s3/s3_client.h

@@ -116,6 +116,7 @@ enum aws_s3_request_type {
     AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY,
     AWS_S3_REQUEST_TYPE_COPY_OBJECT,
     AWS_S3_REQUEST_TYPE_PUT_OBJECT,
+    AWS_S3_REQUEST_TYPE_CREATE_SESSION,
 
     /* Max enum value */
     AWS_S3_REQUEST_TYPE_MAX,
@@ -574,11 +575,12 @@ struct aws_s3_meta_request_options {
     enum aws_s3_meta_request_type type;
 
     /**
-     * Optional.
      * The S3 operation name (e.g. "CreateBucket").
-     * This will only be used when type is AWS_S3_META_REQUEST_TYPE_DEFAULT;
+     * This must be set if type is AWS_S3_META_REQUEST_TYPE_DEFAULT;
      * it is automatically populated for other meta-request types.
      * This name is used to fill out details in metrics and error reports.
+     * It's also used to check for niche behavior specific to some operations
+     * (e.g. "CompleteMultipartUpload" may return an error, even though the status-code was 200 OK).
      */
     struct aws_byte_cursor operation_name;
 
@@ -780,7 +782,7 @@ struct aws_s3_meta_request_result {
      * "PutObject, "CreateMultipartUpload", "UploadPart", "CompleteMultipartUpload", or others.
      * For AWS_S3_META_REQUEST_TYPE_DEFAULT, this is the same value passed to
      * aws_s3_meta_request_options.operation_name.
-     * NULL if the meta request failed for another reason, or the operation name is not known. */
+     * NULL if the meta request failed for another reason. */
     struct aws_string *error_response_operation_name;
 
     /* Response status of the failed request or of the entire meta request. */

source/s3.c

@@ -5,6 +5,7 @@
 
 #include <aws/s3/private/s3_platform_info.h>
 #include <aws/s3/s3.h>
+#include <aws/s3/s3_client.h>
 
 #include <aws/auth/auth.h>
 #include <aws/common/error.h>
@@ -75,6 +76,56 @@ static struct aws_log_subject_info_list s_s3_log_subject_list = {
     .count = AWS_ARRAY_SIZE(s_s3_log_subject_infos),
 };
 
+struct aws_s3_request_type_operation_name_pair {
+    enum aws_s3_request_type type;
+    struct aws_byte_cursor name;
+};
+
+#define DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(TYPE, NAME)                                                            \
+    [TYPE] = {.type = TYPE, .name = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(NAME)}
+
+static struct aws_s3_request_type_operation_name_pair s_s3_request_type_operation_name_array[] = {
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_HEAD_OBJECT, "HeadObject"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_GET_OBJECT, "GetObject"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_LIST_PARTS, "ListParts"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD, "CreateMultipartUpload"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_UPLOAD_PART, "UploadPart"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD, "AbortMultipartUpload"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD, "CompleteMultipartUpload"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY, "UploadPartCopy"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_COPY_OBJECT, "CopyObject"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_PUT_OBJECT, "PutObject"),
+    DEFINE_REQUEST_TYPE_OPERATION_NAME_PAIR(AWS_S3_REQUEST_TYPE_CREATE_SESSION, "CreateSession"),
+};
+
+/* Hash-table for case-insensitive lookup from operation-name -> request-type.
+ * key is `struct aws_byte_cursor *`, pointing into array above.
+ * value is `enum aws_s3_request_type *`, pointing into array above. */
+static struct aws_hash_table s_s3_operation_name_to_request_type_table;
+
+const char *aws_s3_request_type_operation_name(enum aws_s3_request_type type) {
+
+    if (type >= 0 && type < AWS_ARRAY_SIZE(s_s3_request_type_operation_name_array)) {
+        const struct aws_s3_request_type_operation_name_pair *pair = &s_s3_request_type_operation_name_array[type];
+        /* The array may have gaps in it */
+        if (pair->name.len > 0) {
+            return (const char *)pair->name.ptr;
+        }
+    }
+
+    return "";
+}
+
+enum aws_s3_request_type aws_s3_request_type_from_operation_name(struct aws_byte_cursor name) {
+    struct aws_hash_element *elem = NULL;
+    aws_hash_table_find(&s_s3_operation_name_to_request_type_table, &name, &elem);
+    if (elem != NULL) {
+        enum aws_s3_request_type *type = elem->value;
+        return *type;
+    }
+    return AWS_S3_REQUEST_TYPE_UNKNOWN;
+}
+
 static bool s_library_initialized = false;
 static struct aws_allocator *s_library_allocator = NULL;
 static struct aws_s3_platform_info_loader *s_loader;
@@ -97,6 +148,26 @@ void aws_s3_library_init(struct aws_allocator *allocator) {
     aws_register_log_subject_info_list(&s_s3_log_subject_list);
     s_loader = aws_s3_platform_info_loader_new(allocator);
     AWS_FATAL_ASSERT(s_loader);
+
+    /* Initialize s_s3_operation_name_to_request_type_table */
+    int err = aws_hash_table_init(
+        &s_s3_operation_name_to_request_type_table,
+        allocator,
+        AWS_ARRAY_SIZE(s_s3_request_type_operation_name_array) /*initial_size*/,
+        aws_hash_byte_cursor_ptr_ignore_case,
+        (aws_hash_callback_eq_fn *)aws_byte_cursor_eq_ignore_case,
+        NULL /*destroy_key*/,
+        NULL /*destroy_value*/);
+    AWS_FATAL_ASSERT(!err);
+    for (size_t i = 0; i < AWS_ARRAY_SIZE(s_s3_request_type_operation_name_array); ++i) {
+        struct aws_s3_request_type_operation_name_pair *pair = &s_s3_request_type_operation_name_array[i];
+        /* The array may have gaps in it */
+        if (pair->name.len != 0) {
+            err = aws_hash_table_put(&s_s3_operation_name_to_request_type_table, &pair->name, &pair->type, NULL);
+            AWS_FATAL_ASSERT(!err);
+        }
+    }
+
     s_library_initialized = true;
 }
 
@@ -118,9 +189,10 @@ void aws_s3_library_clean_up(void) {
     }
 
     s_library_initialized = false;
-    s_loader = aws_s3_platform_info_loader_release(s_loader);
     aws_thread_join_all_managed();
 
+    aws_hash_table_clean_up(&s_s3_operation_name_to_request_type_table);
+    s_loader = aws_s3_platform_info_loader_release(s_loader);
     aws_unregister_log_subject_info_list(&s_s3_log_subject_list);
     aws_unregister_error_info(&s_error_list);
     aws_http_library_clean_up();

source/s3_client.c

@@ -1319,6 +1319,13 @@ static struct aws_s3_meta_request *s_s3_client_meta_request_factory_default(
             return aws_s3_meta_request_copy_object_new(client->allocator, client, options);
         }
         case AWS_S3_META_REQUEST_TYPE_DEFAULT:
+            if (options->operation_name.len == 0) {
+                AWS_LOGF_ERROR(
+                    AWS_LS_S3_META_REQUEST, "Could not create Default Meta Request; operation name is required");
+                aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+                return NULL;
+            }
+
             return aws_s3_meta_request_default_new(
                 client->allocator,
                 client,
@@ -1733,7 +1740,7 @@ static bool s_s3_client_should_update_meta_request(
     /* CreateSession has high priority to bypass the checks. */
     if (meta_request->type == AWS_S3_META_REQUEST_TYPE_DEFAULT) {
         struct aws_s3_meta_request_default *meta_request_default = meta_request->impl;
-        if (aws_string_eq_c_str(meta_request_default->operation_name, "CreateSession")) {
+        if (meta_request_default->request_type == AWS_S3_REQUEST_TYPE_CREATE_SESSION) {
             return true;
         }
     }

source/s3_default_meta_request.c

@@ -60,6 +60,7 @@ struct aws_s3_meta_request *aws_s3_meta_request_default_new(
     AWS_PRECONDITION(client);
     AWS_PRECONDITION(options);
     AWS_PRECONDITION(options->message);
+    AWS_PRECONDITION(request_type != AWS_S3_REQUEST_TYPE_UNKNOWN || options->operation_name.len != 0);
 
     struct aws_byte_cursor request_method;
     if (aws_http_message_get_request_method(options->message, &request_method)) {
@@ -105,25 +106,24 @@ struct aws_s3_meta_request *aws_s3_meta_request_default_new(
     }
 
     meta_request_default->content_length = (size_t)content_length;
-    meta_request_default->request_type = request_type;
-
-    /* Try to get operation name.
-     * When internal aws-c-s3 code creates a default meta-request,
-     * a valid request_type is always passed in, and we can get its operation name.
-     * When external users create a default meta-request, they may have provided
-     * operation name in the options. */
-    const char *operation_name = aws_s3_request_type_operation_name(request_type);
-    if (operation_name[0] != '\0') {
-        meta_request_default->operation_name = aws_string_new_from_c_str(allocator, operation_name);
-    } else if (options->operation_name.len != 0) {
+
+    if (request_type == AWS_S3_REQUEST_TYPE_UNKNOWN) {
+        /* use name to get type */
         meta_request_default->operation_name = aws_string_new_from_cursor(allocator, &options->operation_name);
+        meta_request_default->request_type = aws_s3_request_type_from_operation_name(options->operation_name);
+    } else {
+        /* use type to get name */
+        meta_request_default->request_type = request_type;
+        meta_request_default->operation_name =
+            aws_string_new_from_c_str(allocator, aws_s3_request_type_operation_name(request_type));
+        AWS_ASSERT(meta_request_default->operation_name->len != 0);
     }
 
     AWS_LOGF_DEBUG(
         AWS_LS_S3_META_REQUEST,
         "id=%p Created new Default Meta Request. operation=%s",
         (void *)meta_request_default,
-        meta_request_default->operation_name ? aws_string_c_str(meta_request_default->operation_name) : "?");
+        aws_string_c_str(meta_request_default->operation_name));
 
     return &meta_request_default->base;
 }
@@ -170,9 +170,8 @@ static bool s_s3_meta_request_default_update(
                     1 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
-                /* Default meta-request might know operation name, despite not knowing valid request_type.
-                 * If so, pass the name along. */
-                if (request->operation_name == NULL && meta_request_default->operation_name != NULL) {
+                /* If request_type didn't map to a name, copy over the name passed in by user */
+                if (request->operation_name == NULL) {
                     request->operation_name =
                         aws_string_new_from_string(meta_request->allocator, meta_request_default->operation_name);
                 }

source/s3_list_objects.c

@@ -230,6 +230,7 @@ struct aws_s3_paginator *aws_s3_initiate_list_objects(
     aws_ref_count_init(&operation_data->ref_count, operation_data, s_ref_count_zero_callback);
 
     struct aws_s3_paginated_operation_params operation_params = {
+        .operation_name = aws_byte_cursor_from_c_str("ListObjectsV2"),
         .next_message = s_construct_next_request_http_message,
         .on_result_node_encountered_fn = s_on_list_bucket_result_node_encountered,
         .on_paginated_operation_cleanup = s_on_paginator_cleanup,

source/s3_list_parts.c

@@ -220,6 +220,7 @@ struct aws_s3_paginated_operation *aws_s3_list_parts_operation_new(
     aws_ref_count_init(&operation_data->ref_count, operation_data, s_ref_count_zero_callback);
 
     struct aws_s3_paginated_operation_params operation_params = {
+        .operation_name = aws_byte_cursor_from_c_str("ListParts"),
         .next_message = s_construct_next_request_http_message,
         .on_result_node_encountered_fn = s_xml_on_ListPartsResult_child,
         .on_paginated_operation_cleanup = s_on_paginator_cleanup,

source/s3_meta_request.c

@@ -2176,10 +2176,8 @@ void aws_s3_meta_request_result_setup(
                 result->error_response_body, meta_request->allocator, &failed_request->send_data.response_body);
         }
 
-        if (failed_request->operation_name != NULL) {
-            result->error_response_operation_name =
-                aws_string_new_from_string(meta_request->allocator, failed_request->operation_name);
-        }
+        result->error_response_operation_name =
+            aws_string_new_from_string(meta_request->allocator, failed_request->operation_name);
     }
 
     result->response_status = response_status;

source/s3_paginator.c

@@ -27,6 +27,7 @@ enum operation_state {
 struct aws_s3_paginated_operation {
     struct aws_allocator *allocator;
 
+    struct aws_string *operation_name;
     struct aws_string *result_xml_node_name;
     struct aws_string *continuation_xml_node_name;
 
@@ -73,6 +74,10 @@ static void s_operation_ref_count_zero_callback(void *arg) {
         operation->on_paginated_operation_cleanup(operation->user_data);
     }
 
+    if (operation->operation_name) {
+        aws_string_destroy(operation->operation_name);
+    }
+
     if (operation->result_xml_node_name) {
         aws_string_destroy(operation->result_xml_node_name);
     }
@@ -158,6 +163,7 @@ struct aws_s3_paginated_operation *aws_s3_paginated_operation_new(
         aws_mem_calloc(allocator, 1, sizeof(struct aws_s3_paginated_operation));
     operation->allocator = allocator;
 
+    operation->operation_name = aws_string_new_from_cursor(allocator, &params->operation_name);
     operation->result_xml_node_name = aws_string_new_from_cursor(allocator, &params->result_xml_node_name);
     operation->continuation_xml_node_name =
         aws_string_new_from_cursor(allocator, &params->continuation_token_node_name);
@@ -433,6 +439,7 @@ int aws_s3_paginator_continue(struct aws_s3_paginator *paginator, const struct a
         .user_data = paginator,
         .signing_config = (struct aws_signing_config_aws *)signing_config,
         .type = AWS_S3_META_REQUEST_TYPE_DEFAULT,
+        .operation_name = aws_byte_cursor_from_string(paginator->operation->operation_name),
         .body_callback = s_receive_body_callback,
         .finish_callback = s_on_request_finished,
         .message = paginated_request_message,

source/s3_request.c

@@ -180,10 +180,7 @@ struct aws_s3_request_metrics *aws_s3_request_metrics_new(
     AWS_ASSERT(metrics->req_resp_info_metrics.host_address != NULL);
 
     metrics->req_resp_info_metrics.request_type = request->request_type;
-
-    if (request->operation_name != NULL) {
-        metrics->req_resp_info_metrics.operation_name = aws_string_new_from_string(allocator, request->operation_name);
-    }
+    metrics->req_resp_info_metrics.operation_name = aws_string_new_from_string(allocator, request->operation_name);
 
     metrics->time_metrics.start_timestamp_ns = -1;
     metrics->time_metrics.end_timestamp_ns = -1;

source/s3_util.c

@@ -73,33 +73,6 @@ const struct aws_byte_cursor g_user_agent_header_unknown = AWS_BYTE_CUR_INIT_FRO
 const uint32_t g_s3_max_num_upload_parts = 10000;
 const size_t g_s3_min_upload_part_size = MB_TO_BYTES(5);
 
-const char *aws_s3_request_type_operation_name(enum aws_s3_request_type type) {
-    switch (type) {
-        case AWS_S3_REQUEST_TYPE_HEAD_OBJECT:
-            return "HeadObject";
-        case AWS_S3_REQUEST_TYPE_GET_OBJECT:
-            return "GetObject";
-        case AWS_S3_REQUEST_TYPE_LIST_PARTS:
-            return "ListParts";
-        case AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD:
-            return "CreateMultipartUpload";
-        case AWS_S3_REQUEST_TYPE_UPLOAD_PART:
-            return "UploadPart";
-        case AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD:
-            return "AbortMultipartUpload";
-        case AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD:
-            return "CompleteMultipartUpload";
-        case AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY:
-            return "UploadPartCopy";
-        case AWS_S3_REQUEST_TYPE_COPY_OBJECT:
-            return "CopyObject";
-        case AWS_S3_REQUEST_TYPE_PUT_OBJECT:
-            return "PutObject";
-        default:
-            return "";
-    }
-}
-
 void copy_http_headers(const struct aws_http_headers *src, struct aws_http_headers *dest) {
     AWS_PRECONDITION(src);
     AWS_PRECONDITION(dest);

tests/s3_data_plane_tests.c

@@ -4345,6 +4345,7 @@ static int s_test_s3_meta_request_default(struct aws_allocator *allocator, void
 
     /* Pass the request through as a default request so that it goes through as-is. */
     options.type = AWS_S3_META_REQUEST_TYPE_DEFAULT;
+    options.operation_name = aws_byte_cursor_from_c_str("GetObject");
     options.message = message;
 
     struct aws_s3_meta_request_test_results meta_request_test_results;