Relax validation on user-specified topic aliasing (#334)

include/aws/mqtt/private/v5/mqtt5_utils.h

@@ -210,6 +210,15 @@ AWS_MQTT_API enum aws_mqtt5_client_session_behavior_type aws_mqtt5_client_sessio
 AWS_MQTT_API const char *aws_mqtt5_outbound_topic_alias_behavior_type_to_c_string(
     enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_aliasing_behavior);
 
+/**
+ * Checks an outbound aliasing behavior type value for validity
+ *
+ * @param outbound_aliasing_behavior value to check
+ * @return true if this is a valid value, false otherwise
+ */
+AWS_MQTT_API bool aws_mqtt5_outbound_topic_alias_behavior_type_validate(
+    enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_aliasing_behavior);
+
 /**
  * Converts an outbound topic aliasing behavior type value to a final non-default value.
  *
@@ -229,6 +238,15 @@ AWS_MQTT_API enum aws_mqtt5_client_outbound_topic_alias_behavior_type
 AWS_MQTT_API const char *aws_mqtt5_inbound_topic_alias_behavior_type_to_c_string(
     enum aws_mqtt5_client_inbound_topic_alias_behavior_type inbound_aliasing_behavior);
 
+/**
+ * Checks an inbound aliasing behavior type value for validity
+ *
+ * @param inbound_aliasing_behavior value to check
+ * @return true if this is a valid value, false otherwise
+ */
+AWS_MQTT_API bool aws_mqtt5_inbound_topic_alias_behavior_type_validate(
+    enum aws_mqtt5_client_inbound_topic_alias_behavior_type inbound_aliasing_behavior);
+
 /**
  * Converts an inbound topic aliasing behavior type value to a final non-default value.
  *

include/aws/mqtt/v5/mqtt5_client.h

@@ -86,16 +86,15 @@ enum aws_mqtt5_client_outbound_topic_alias_behavior_type {
      * topic alias mappings unpredictably.  The client will properly use the alias when the current connection
      * has seen the alias binding already.
      */
-    AWS_MQTT5_COTABT_USER,
+    AWS_MQTT5_COTABT_MANUAL,
 
     /**
-     * Client fails any user-specified topic aliasing and acts on the outbound alias set as an LRU cache.
+     * Client ignores any user-specified topic aliasing and acts on the outbound alias set as an LRU cache.
      */
     AWS_MQTT5_COTABT_LRU,
 
     /**
-     * Completely disable outbound topic aliasing.  Attempting to set a topic alias on a PUBLISH results in
-     * an error.
+     * Completely disable outbound topic aliasing.
      */
     AWS_MQTT5_COTABT_DISABLED
 };
@@ -162,7 +161,7 @@ struct aws_mqtt5_client_topic_alias_options {
      * disabled, this setting has no effect.
      *
      * Behaviorally, this value overrides anything present in the topic_alias_maximum field of
-     * the CONNECT packet options.  We intentionally don't bind that field to managed clients to reduce
+     * the CONNECT packet options.
      */
     uint16_t inbound_alias_cache_size;
 };

source/v5/mqtt5_encoder.c

@@ -800,7 +800,25 @@ static int s_aws_mqtt5_encoder_begin_publish(struct aws_mqtt5_encoder *encoder,
 
         local_publish_view.topic = outbound_topic;
         if (outbound_topic_alias != 0) {
+            AWS_LOGF_DEBUG(
+                AWS_LS_MQTT5_GENERAL,
+                "(%p) mqtt5 client encoder - PUBLISH packet using topic alias value %" PRIu16,
+                (void *)encoder->config.client,
+                outbound_topic_alias);
+            if (outbound_topic.len == 0) {
+                AWS_LOGF_DEBUG(
+                    AWS_LS_MQTT5_GENERAL,
+                    "(%p) mqtt5 client encoder - PUBLISH packet dropping topic field for previously established alias",
+                    (void *)encoder->config.client);
+            }
             local_publish_view.topic_alias = &outbound_topic_alias;
+        } else {
+            AWS_FATAL_ASSERT(local_publish_view.topic.len > 0);
+            AWS_LOGF_DEBUG(
+                AWS_LS_MQTT5_GENERAL,
+                "(%p) mqtt5 client encoder - PUBLISH packet not using a topic alias",
+                (void *)encoder->config.client);
+            local_publish_view.topic_alias = NULL;
         }
     }
 

source/v5/mqtt5_options_storage.c

@@ -1743,30 +1743,6 @@ static int s_aws_mqtt5_packet_publish_view_validate_vs_connection_settings(
             return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
         }
 
-        if (publish_view->topic_alias != NULL) {
-            const struct aws_mqtt5_client_options_storage *client_options = client->config;
-            if (client_options->topic_aliasing_options.outbound_topic_alias_behavior != AWS_MQTT5_COTABT_USER) {
-                AWS_LOGF_ERROR(
-                    AWS_LS_MQTT5_GENERAL,
-                    "id=%p: aws_mqtt5_packet_publish_view - topic alias set but outbound topic alias behavior has not "
-                    "been set to user controlled",
-                    (void *)publish_view);
-                return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
-            }
-
-            if (*publish_view->topic_alias > settings->topic_alias_maximum_to_server) {
-                AWS_LOGF_ERROR(
-                    AWS_LS_MQTT5_GENERAL,
-                    "id=%p: aws_mqtt5_packet_publish_view - outbound topic alias (%d) exceeds server's topic alias "
-                    "maximum "
-                    "(%d)",
-                    (void *)publish_view,
-                    (int)(*publish_view->topic_alias),
-                    (int)settings->topic_alias_maximum_to_server);
-                return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
-            }
-        }
-
         if (publish_view->retain && settings->retain_available == false) {
             AWS_LOGF_ERROR(
                 AWS_LS_MQTT5_GENERAL,
@@ -3416,6 +3392,20 @@ int aws_mqtt5_client_options_validate(const struct aws_mqtt5_client_options *opt
         return aws_raise_error(AWS_ERROR_MQTT5_CLIENT_OPTIONS_VALIDATION);
     }
 
+    if (options->topic_aliasing_options != NULL) {
+        if (!aws_mqtt5_outbound_topic_alias_behavior_type_validate(
+                options->topic_aliasing_options->outbound_topic_alias_behavior)) {
+            AWS_LOGF_ERROR(AWS_LS_MQTT5_GENERAL, "invalid outbound topic alias behavior type value");
+            return aws_raise_error(AWS_ERROR_MQTT5_CLIENT_OPTIONS_VALIDATION);
+        }
+
+        if (!aws_mqtt5_inbound_topic_alias_behavior_type_validate(
+                options->topic_aliasing_options->inbound_topic_alias_behavior)) {
+            AWS_LOGF_ERROR(AWS_LS_MQTT5_GENERAL, "invalid inbound topic alias behavior type value");
+            return aws_raise_error(AWS_ERROR_MQTT5_CLIENT_OPTIONS_VALIDATION);
+        }
+    }
+
     return AWS_OP_SUCCESS;
 }
 

source/v5/mqtt5_topic_alias.c

@@ -132,16 +132,16 @@ static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topi
     struct aws_allocator *allocator);
 static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topic_alias_resolver_lru_new(
     struct aws_allocator *allocator);
-static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topic_alias_resolver_user_new(
+static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topic_alias_resolver_manual_new(
     struct aws_allocator *allocator);
 
 struct aws_mqtt5_outbound_topic_alias_resolver *aws_mqtt5_outbound_topic_alias_resolver_new(
     struct aws_allocator *allocator,
     enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_alias_behavior) {
 
     switch (aws_mqtt5_outbound_topic_alias_behavior_type_to_non_default(outbound_alias_behavior)) {
-        case AWS_MQTT5_COTABT_USER:
-            return s_aws_mqtt5_outbound_topic_alias_resolver_user_new(allocator);
+        case AWS_MQTT5_COTABT_MANUAL:
+            return s_aws_mqtt5_outbound_topic_alias_resolver_manual_new(allocator);
 
         case AWS_MQTT5_COTABT_LRU:
             return s_aws_mqtt5_outbound_topic_alias_resolver_lru_new(allocator);
@@ -243,60 +243,60 @@ static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topi
 }
 
 /*
- * User resolver
+ * Manual resolver
  *
- * User resolution implies the user is controlling the topic alias assignments, but we still want to validate their
+ * Manual resolution implies the user is controlling the topic alias assignments, but we still want to validate their
  * actions.  In particular, we track the currently valid set of aliases (based on previous outbound publishes)
  * and only use an alias when the submitted publish is an exact match for the current assignment.
  */
 
-struct aws_mqtt5_outbound_topic_alias_resolver_user {
+struct aws_mqtt5_outbound_topic_alias_resolver_manual {
     struct aws_mqtt5_outbound_topic_alias_resolver base;
 
     struct aws_array_list aliases;
 };
 
-static void s_cleanup_user_aliases(struct aws_mqtt5_outbound_topic_alias_resolver_user *user_resolver) {
-    for (size_t i = 0; i < aws_array_list_length(&user_resolver->aliases); ++i) {
+static void s_cleanup_manual_aliases(struct aws_mqtt5_outbound_topic_alias_resolver_manual *manual_resolver) {
+    for (size_t i = 0; i < aws_array_list_length(&manual_resolver->aliases); ++i) {
         struct aws_string *alias = NULL;
-        aws_array_list_get_at(&user_resolver->aliases, &alias, i);
+        aws_array_list_get_at(&manual_resolver->aliases, &alias, i);
 
         aws_string_destroy(alias);
     }
 
-    aws_array_list_clean_up(&user_resolver->aliases);
-    AWS_ZERO_STRUCT(user_resolver->aliases);
+    aws_array_list_clean_up(&manual_resolver->aliases);
+    AWS_ZERO_STRUCT(manual_resolver->aliases);
 }
 
-static void s_aws_mqtt5_outbound_topic_alias_resolver_user_destroy(
+static void s_aws_mqtt5_outbound_topic_alias_resolver_manual_destroy(
     struct aws_mqtt5_outbound_topic_alias_resolver *resolver) {
     if (resolver == NULL) {
         return;
     }
 
-    struct aws_mqtt5_outbound_topic_alias_resolver_user *user_resolver = resolver->impl;
-    s_cleanup_user_aliases(user_resolver);
+    struct aws_mqtt5_outbound_topic_alias_resolver_manual *manual_resolver = resolver->impl;
+    s_cleanup_manual_aliases(manual_resolver);
 
-    aws_mem_release(resolver->allocator, user_resolver);
+    aws_mem_release(resolver->allocator, manual_resolver);
 }
 
-static int s_aws_mqtt5_outbound_topic_alias_resolver_user_reset(
+static int s_aws_mqtt5_outbound_topic_alias_resolver_manual_reset(
     struct aws_mqtt5_outbound_topic_alias_resolver *resolver,
     uint16_t topic_alias_maximum) {
-    struct aws_mqtt5_outbound_topic_alias_resolver_user *user_resolver = resolver->impl;
-    s_cleanup_user_aliases(user_resolver);
+    struct aws_mqtt5_outbound_topic_alias_resolver_manual *manual_resolver = resolver->impl;
+    s_cleanup_manual_aliases(manual_resolver);
 
     aws_array_list_init_dynamic(
-        &user_resolver->aliases, resolver->allocator, topic_alias_maximum, sizeof(struct aws_string *));
+        &manual_resolver->aliases, resolver->allocator, topic_alias_maximum, sizeof(struct aws_string *));
     for (size_t i = 0; i < topic_alias_maximum; ++i) {
         struct aws_string *invalid_alias = NULL;
-        aws_array_list_push_back(&user_resolver->aliases, &invalid_alias);
+        aws_array_list_push_back(&manual_resolver->aliases, &invalid_alias);
     }
 
     return AWS_OP_SUCCESS;
 }
 
-static int s_aws_mqtt5_outbound_topic_alias_resolver_user_resolve_outbound_publish_fn(
+static int s_aws_mqtt5_outbound_topic_alias_resolver_manual_resolve_outbound_publish_fn(
     struct aws_mqtt5_outbound_topic_alias_resolver *resolver,
     const struct aws_mqtt5_packet_publish_view *publish_view,
     uint16_t *topic_alias_out,
@@ -316,15 +316,15 @@ static int s_aws_mqtt5_outbound_topic_alias_resolver_user_resolve_outbound_publi
         return aws_raise_error(AWS_ERROR_MQTT5_INVALID_OUTBOUND_TOPIC_ALIAS);
     }
 
-    struct aws_mqtt5_outbound_topic_alias_resolver_user *user_resolver = resolver->impl;
+    struct aws_mqtt5_outbound_topic_alias_resolver_manual *manual_resolver = resolver->impl;
     uint16_t user_alias_index = user_alias - 1;
-    if (user_alias_index >= aws_array_list_length(&user_resolver->aliases)) {
+    if (user_alias_index >= aws_array_list_length(&manual_resolver->aliases)) {
         /* should have been caught by dynamic publish validation */
         return aws_raise_error(AWS_ERROR_MQTT5_INVALID_OUTBOUND_TOPIC_ALIAS);
     }
 
     struct aws_string *current_assignment = NULL;
-    aws_array_list_get_at(&user_resolver->aliases, &current_assignment, user_alias_index);
+    aws_array_list_get_at(&manual_resolver->aliases, &current_assignment, user_alias_index);
 
     *topic_alias_out = user_alias;
 
@@ -346,25 +346,25 @@ static int s_aws_mqtt5_outbound_topic_alias_resolver_user_resolve_outbound_publi
     if (!can_use_alias) {
         aws_string_destroy(current_assignment);
         current_assignment = aws_string_new_from_cursor(resolver->allocator, &publish_view->topic);
-        aws_array_list_set_at(&user_resolver->aliases, &current_assignment, user_alias_index);
+        aws_array_list_set_at(&manual_resolver->aliases, &current_assignment, user_alias_index);
     }
 
     return AWS_OP_SUCCESS;
 }
 
-static struct aws_mqtt5_outbound_topic_alias_resolver_vtable s_aws_mqtt5_outbound_topic_alias_resolver_user_vtable = {
-    .destroy_fn = s_aws_mqtt5_outbound_topic_alias_resolver_user_destroy,
-    .reset_fn = s_aws_mqtt5_outbound_topic_alias_resolver_user_reset,
-    .resolve_outbound_publish_fn = s_aws_mqtt5_outbound_topic_alias_resolver_user_resolve_outbound_publish_fn,
+static struct aws_mqtt5_outbound_topic_alias_resolver_vtable s_aws_mqtt5_outbound_topic_alias_resolver_manual_vtable = {
+    .destroy_fn = s_aws_mqtt5_outbound_topic_alias_resolver_manual_destroy,
+    .reset_fn = s_aws_mqtt5_outbound_topic_alias_resolver_manual_reset,
+    .resolve_outbound_publish_fn = s_aws_mqtt5_outbound_topic_alias_resolver_manual_resolve_outbound_publish_fn,
 };
 
-static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topic_alias_resolver_user_new(
+static struct aws_mqtt5_outbound_topic_alias_resolver *s_aws_mqtt5_outbound_topic_alias_resolver_manual_new(
     struct aws_allocator *allocator) {
-    struct aws_mqtt5_outbound_topic_alias_resolver_user *resolver =
-        aws_mem_calloc(allocator, 1, sizeof(struct aws_mqtt5_outbound_topic_alias_resolver_user));
+    struct aws_mqtt5_outbound_topic_alias_resolver_manual *resolver =
+        aws_mem_calloc(allocator, 1, sizeof(struct aws_mqtt5_outbound_topic_alias_resolver_manual));
 
     resolver->base.allocator = allocator;
-    resolver->base.vtable = &s_aws_mqtt5_outbound_topic_alias_resolver_user_vtable;
+    resolver->base.vtable = &s_aws_mqtt5_outbound_topic_alias_resolver_manual_vtable;
     resolver->base.impl = resolver;
 
     aws_array_list_init_dynamic(&resolver->aliases, allocator, 0, sizeof(struct aws_string *));

source/v5/mqtt5_utils.c

@@ -289,7 +289,7 @@ enum aws_mqtt5_client_session_behavior_type aws_mqtt5_client_session_behavior_ty
 const char *aws_mqtt5_outbound_topic_alias_behavior_type_to_c_string(
     enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_aliasing_behavior) {
     switch (aws_mqtt5_outbound_topic_alias_behavior_type_to_non_default(outbound_aliasing_behavior)) {
-        case AWS_MQTT5_COTABT_USER:
+        case AWS_MQTT5_COTABT_MANUAL:
             return "User-controlled outbound topic aliasing behavior";
         case AWS_MQTT5_COTABT_LRU:
             return "LRU caching outbound topic aliasing behavior";
@@ -301,6 +301,13 @@ const char *aws_mqtt5_outbound_topic_alias_behavior_type_to_c_string(
     }
 }
 
+bool aws_mqtt5_outbound_topic_alias_behavior_type_validate(
+    enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_aliasing_behavior) {
+
+    return outbound_aliasing_behavior >= AWS_MQTT5_COTABT_DEFAULT &&
+           outbound_aliasing_behavior <= AWS_MQTT5_COTABT_DISABLED;
+}
+
 enum aws_mqtt5_client_outbound_topic_alias_behavior_type aws_mqtt5_outbound_topic_alias_behavior_type_to_non_default(
     enum aws_mqtt5_client_outbound_topic_alias_behavior_type outbound_aliasing_behavior) {
     if (outbound_aliasing_behavior == AWS_MQTT5_COTABT_DEFAULT) {
@@ -322,6 +329,13 @@ const char *aws_mqtt5_inbound_topic_alias_behavior_type_to_c_string(
     }
 }
 
+bool aws_mqtt5_inbound_topic_alias_behavior_type_validate(
+    enum aws_mqtt5_client_inbound_topic_alias_behavior_type inbound_aliasing_behavior) {
+
+    return inbound_aliasing_behavior >= AWS_MQTT5_CITABT_DEFAULT &&
+           inbound_aliasing_behavior <= AWS_MQTT5_CITABT_DISABLED;
+}
+
 enum aws_mqtt5_client_inbound_topic_alias_behavior_type aws_mqtt5_inbound_topic_alias_behavior_type_to_non_default(
     enum aws_mqtt5_client_inbound_topic_alias_behavior_type inbound_aliasing_behavior) {
     if (inbound_aliasing_behavior == AWS_MQTT5_CITABT_DEFAULT) {

tests/CMakeLists.txt

@@ -131,10 +131,10 @@ add_test_case(mqtt5_inbound_topic_alias_resolve_failure)
 add_test_case(mqtt5_inbound_topic_alias_reset)
 add_test_case(mqtt5_outbound_topic_alias_disabled_resolve_success)
 add_test_case(mqtt5_outbound_topic_alias_disabled_resolve_failure)
-add_test_case(mqtt5_outbound_topic_alias_user_resolve_failure_zero_alias)
-add_test_case(mqtt5_outbound_topic_alias_user_resolve_failure_too_big_alias)
-add_test_case(mqtt5_outbound_topic_alias_user_resolve_success)
-add_test_case(mqtt5_outbound_topic_alias_user_reset)
+add_test_case(mqtt5_outbound_topic_alias_manual_resolve_failure_zero_alias)
+add_test_case(mqtt5_outbound_topic_alias_manual_resolve_failure_too_big_alias)
+add_test_case(mqtt5_outbound_topic_alias_manual_resolve_success)
+add_test_case(mqtt5_outbound_topic_alias_manual_reset)
 add_test_case(mqtt5_outbound_topic_alias_lru_zero_size)
 
 # lru topic sequence tests
@@ -247,7 +247,6 @@ add_test_case(mqtt5_client_options_validation_failure_invalid_keep_alive)
 add_test_case(mqtt5_operation_subscribe_connection_settings_validation_failure_exceeds_maximum_packet_size)
 add_test_case(mqtt5_operation_unsubscribe_connection_settings_validation_failure_exceeds_maximum_packet_size)
 add_test_case(mqtt5_operation_publish_connection_settings_validation_failure_exceeds_maximum_packet_size)
-add_test_case(mqtt5_operation_publish_connection_settings_validation_failure_exceeds_topic_alias_maximum)
 add_test_case(mqtt5_operation_publish_connection_settings_validation_failure_exceeds_maximum_qos)
 add_test_case(mqtt5_operation_publish_connection_settings_validation_failure_invalid_retain)
 add_test_case(mqtt5_operation_disconnect_connection_settings_validation_failure_exceeds_maximum_packet_size)
@@ -365,12 +364,10 @@ add_test_case(mqtt5_client_inbound_alias_failure_disabled)
 add_test_case(mqtt5_client_inbound_alias_failure_zero_id)
 add_test_case(mqtt5_client_inbound_alias_failure_too_large_id)
 add_test_case(mqtt5_client_inbound_alias_failure_unbound_id)
-add_test_case(mqtt5_client_outbound_alias_disabled_failure_alias_set)
-add_test_case(mqtt5_client_outbound_alias_user_failure_empty_topic)
-add_test_case(mqtt5_client_outbound_alias_lru_failure_alias_set)
+add_test_case(mqtt5_client_outbound_alias_manual_failure_empty_topic)
 
 # a, b, c, r imply notation as the outbound resolver unit tests above
-add_test_case(mqtt5_client_outbound_alias_user_success_a_b_ar_br)
+add_test_case(mqtt5_client_outbound_alias_manual_success_a_b_ar_br)
 add_test_case(mqtt5_client_outbound_alias_lru_success_a_b_c_br_cr_a)
 
 add_test_case(rate_limiter_token_bucket_init_invalid)