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,23 @@ 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);
+
+/**
+ * Return operation name for aws_s3_request_type as static-lifetime aws_string*
+ * or NULL if the type doesn't map to an actual operation.
+ * For example:
+ * AWS_S3_REQUEST_TYPE_HEAD_OBJECT -> static aws_string "HeadObject"
+ * AWS_S3_REQUEST_TYPE_UNKNOWN -> NULL
+ * AWS_S3_REQUEST_TYPE_MAX -> NULL
+ */
+AWS_S3_API
+struct aws_string *aws_s3_request_type_to_operation_name_static_string(enum aws_s3_request_type type);
+
 /**
  * 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,23 @@ 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.
+     * The canonical operation names are listed here:
+     * https://docs.aws.amazon.com/AmazonS3/latest/API/API_Operations_Amazon_Simple_Storage_Service.html
+     *
      * This name is used to fill out details in metrics and error reports.
+     * It also drives some operation-specific behavior.
+     * If you pass the wrong name, you risk getting the wrong behavior.
+     *
+     * For example, every operation except "GetObject" has its response checked
+     * for error, even if the HTTP status-code was 200 OK
+     * (see https://repost.aws/knowledge-center/s3-resolve-200-internalerror).
+     * If you used AWS_S3_META_REQUEST_TYPE_DEFAULT to do GetObject, but mis-named
+     * it "Download", and the object looked like XML with an error code,
+     * then the meta-request would fail. You may log the full response body,
+     * and leak sensitive data.
      */
     struct aws_byte_cursor operation_name;
 
@@ -780,7 +793,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

@@ -4,7 +4,9 @@
  */
 
 #include <aws/s3/private/s3_platform_info.h>
+#include <aws/s3/private/s3_util.h>
 #include <aws/s3/s3.h>
+#include <aws/s3/s3_client.h>
 
 #include <aws/auth/auth.h>
 #include <aws/common/error.h>
@@ -75,6 +77,102 @@ 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_info {
+    enum aws_s3_request_type type;
+    struct aws_string *name_string;
+    struct aws_byte_cursor name_cursor;
+};
+
+static struct aws_s3_request_type_info s_s3_request_type_info_array[AWS_S3_REQUEST_TYPE_MAX];
+
+/* Hash-table for case-insensitive lookup, from operation-name -> request-type.
+ * key is operation-name (stored as `struct aws_byte_cursor*`, pointing into array above).
+ * value is request-type (stored as `enum aws_s3_request_type *`, pointing into array above). */
+static struct aws_hash_table s_s3_operation_name_to_request_type_table;
+
+static void s_s3_request_type_register(enum aws_s3_request_type type, const struct aws_string *name) {
+
+    AWS_PRECONDITION(type >= 0 && type < AWS_ARRAY_SIZE(s_s3_request_type_info_array));
+    AWS_PRECONDITION(AWS_IS_ZEROED(s_s3_request_type_info_array[type]));
+
+    struct aws_s3_request_type_info *info = &s_s3_request_type_info_array[type];
+    info->type = type;
+    info->name_string = (struct aws_string *)name;
+    info->name_cursor = aws_byte_cursor_from_string(name);
+
+    int err = aws_hash_table_put(&s_s3_operation_name_to_request_type_table, &info->name_cursor, &info->type, NULL);
+    AWS_FATAL_ASSERT(!err);
+}
+
+AWS_STATIC_STRING_FROM_LITERAL(s_HeadObject_str, "HeadObject");
+AWS_STATIC_STRING_FROM_LITERAL(s_GetObject_str, "GetObject");
+AWS_STATIC_STRING_FROM_LITERAL(s_ListParts_str, "ListParts");
+AWS_STATIC_STRING_FROM_LITERAL(s_CreateMultipartUpload_str, "CreateMultipartUpload");
+AWS_STATIC_STRING_FROM_LITERAL(s_UploadPart_str, "UploadPart");
+AWS_STATIC_STRING_FROM_LITERAL(s_AbortMultipartUpload_str, "AbortMultipartUpload");
+AWS_STATIC_STRING_FROM_LITERAL(s_CompleteMultipartUpload_str, "CompleteMultipartUpload");
+AWS_STATIC_STRING_FROM_LITERAL(s_UploadPartCopy_str, "UploadPartCopy");
+AWS_STATIC_STRING_FROM_LITERAL(s_CopyObject_str, "CopyObject");
+AWS_STATIC_STRING_FROM_LITERAL(s_PutObject_str, "PutObject");
+AWS_STATIC_STRING_FROM_LITERAL(s_CreateSession_str, "CreateSession");
+
+static void s_s3_request_type_info_init(struct aws_allocator *allocator) {
+    int err = aws_hash_table_init(
+        &s_s3_operation_name_to_request_type_table,
+        allocator,
+        AWS_ARRAY_SIZE(s_s3_request_type_info_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);
+
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_HEAD_OBJECT, s_HeadObject_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_GET_OBJECT, s_GetObject_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_LIST_PARTS, s_ListParts_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD, s_CreateMultipartUpload_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_UPLOAD_PART, s_UploadPart_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD, s_AbortMultipartUpload_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD, s_CompleteMultipartUpload_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY, s_UploadPartCopy_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_COPY_OBJECT, s_CopyObject_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_PUT_OBJECT, s_PutObject_str);
+    s_s3_request_type_register(AWS_S3_REQUEST_TYPE_CREATE_SESSION, s_CreateSession_str);
+}
+
+static void s_s3_request_type_info_clean_up(void) {
+    aws_hash_table_clean_up(&s_s3_operation_name_to_request_type_table);
+}
+
+struct aws_string *aws_s3_request_type_to_operation_name_static_string(enum aws_s3_request_type type) {
+
+    if (type >= 0 && type < AWS_ARRAY_SIZE(s_s3_request_type_info_array)) {
+        struct aws_s3_request_type_info *info = &s_s3_request_type_info_array[type];
+        return info->name_string;
+    }
+
+    return NULL;
+}
+
+const char *aws_s3_request_type_operation_name(enum aws_s3_request_type type) {
+    struct aws_string *name_string = aws_s3_request_type_to_operation_name_static_string(type);
+    if (name_string != NULL) {
+        return aws_string_c_str(name_string);
+    }
+
+    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 +195,8 @@ 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);
+    s_s3_request_type_info_init(allocator);
+
     s_library_initialized = true;
 }
 
@@ -118,9 +218,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();
 
+    s_s3_request_type_info_clean_up();
+    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,28 @@ 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 is unknown, look it up from operation name */
+    if (request_type != AWS_S3_REQUEST_TYPE_UNKNOWN) {
+        meta_request_default->request_type = request_type;
+    } else {
+        meta_request_default->request_type = aws_s3_request_type_from_operation_name(options->operation_name);
+    }
+
+    /* If we have a static string for this operation name, use that.
+     * Otherwise, copy the operation_name passed in by user. */
+    struct aws_string *static_operation_name = aws_s3_request_type_to_operation_name_static_string(request_type);
+    if (static_operation_name != NULL) {
+        meta_request_default->operation_name = static_operation_name;
+    } else {
         meta_request_default->operation_name = aws_string_new_from_cursor(allocator, &options->operation_name);
     }
 
     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 +174,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

@@ -1496,9 +1496,11 @@ static void s_s3_meta_request_send_request_finish(
 /* Return whether the response to this request might contain an error, even though we got 200 OK.
  * see: https://repost.aws/knowledge-center/s3-resolve-200-internalerror */
 static bool s_should_check_for_error_despite_200_OK(const struct aws_s3_request *request) {
-    /* We handle async error for every request BUT get object. */
-    struct aws_s3_meta_request *meta_request = request->meta_request;
-    if (meta_request->type == AWS_S3_META_REQUEST_TYPE_GET_OBJECT) {
+    /* We handle async error for every thing EXCEPT GetObject.
+     *
+     * Note that we check the aws_s3_request_type (not the aws_s3_meta_request_type),
+     * in case someone is using a DEFAULT meta-request to send GetObject */
+    if (request->request_type == AWS_S3_REQUEST_TYPE_GET_OBJECT) {
         return false;
     }
     return true;
@@ -2176,10 +2178,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;