Merge branch 'metrics_mqtt3' of github.com:awslabs/aws-c-mqtt into metrics_mqtt5

xiazhvera · xiazhvera · commit 93ee7be43676 · 2026-02-03T15:31:02.000-08:00
diff --git a/include/aws/mqtt/private/mqtt_iot_sdk_metrics.h b/include/aws/mqtt/private/mqtt_iot_sdk_metrics.h
@@ -48,10 +48,10 @@ int aws_mqtt_append_sdk_metrics_to_username(
     size_t *out_full_username_size);
 
 /**
- * Validates that all string fields in aws_mqtt_iot_sdk_metrics
+ * Validates all string fields in aws_mqtt_iot_sdk_metrics
  *
  * @param metrics The metrics structure to validate
- * @return AWS_OP_SUCCESS if all strings are valid, AWS_OP_ERR otherwise
+ * @return AWS_OP_SUCCESS if metrics is not null and all metrics value are valid, AWS_OP_ERR otherwise
  */
 AWS_MQTT_API
 int aws_mqtt_validate_iot_sdk_metrics(const struct aws_mqtt_iot_sdk_metrics *metrics);
diff --git a/source/client.c b/source/client.c
@@ -650,12 +650,7 @@ static void s_mqtt_client_init(
             }
         }
 
-        if (aws_byte_cursor_is_valid(&username_cur)) {
-            AWS_LOGF_DEBUG(
-                AWS_LS_MQTT_CLIENT,
-                "id=%p: Adding username " PRInSTR " to connection",
-                (void *)connection,
-                AWS_BYTE_CURSOR_PRI(username_cur));
+        if (aws_byte_cursor_is_valid(&username_cur) && username_cur.len > 0) {
 
             struct aws_byte_cursor password_cur;
             AWS_ZERO_STRUCT(password_cur);
@@ -664,9 +659,15 @@ static void s_mqtt_client_init(
                 password_cur = aws_byte_cursor_from_string(connection->password);
             }
 
+            AWS_LOGF_DEBUG(
+                AWS_LS_MQTT_CLIENT,
+                "id=%p: Adding username " PRInSTR " to connection",
+                (void *)connection,
+                AWS_BYTE_CURSOR_PRI(username_cur));
+
             aws_mqtt_packet_connect_add_credentials(&connect, username_cur, password_cur);
         } else {
-            AWS_LOGF_WARN(
+            AWS_LOGF_INFO(
                 AWS_LS_MQTT_CLIENT,
                 "id=%p: Failed to set username and password. Most likely there is an issue in metrics. (e.x.: username "
                 "is empty and metrics string exceed the size limit). ",
@@ -3389,7 +3390,7 @@ static int s_aws_mqtt_client_connection_311_set_metrics(void *impl, const struct
     if (metrics) {
         if (aws_mqtt_validate_iot_sdk_metrics(metrics) == AWS_OP_ERR) {
             AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Invalid metrics.", (void *)connection);
-            return aws_raise_error(AWS_ERROR_INVALID_UTF8);
+            return AWS_OP_ERR;
         }
 
         metrics_storage = aws_mqtt_iot_sdk_metrics_storage_new(connection->allocator, metrics);
@@ -3421,12 +3422,10 @@ static int s_aws_mqtt_client_connection_311_set_metrics(void *impl, const struct
         goto done;
     }
 
-    /* Clean up existing metrics if any */
-    if (connection->metrics_storage) {
-        aws_mqtt_iot_sdk_metrics_storage_destroy(connection->metrics_storage);
-    }
-    connection->metrics_storage = metrics_storage;
     AWS_LOGF_TRACE(AWS_LS_MQTT_CLIENT, "id=%p: Setting IoT SDK metrics", (void *)connection);
+    aws_mqtt_iot_sdk_metrics_storage_destroy(connection->metrics_storage);
+    connection->metrics_storage = metrics_storage;
+
     metrics_storage = NULL;
     result = AWS_OP_SUCCESS;
 
diff --git a/source/mqtt_iot_sdk_metrics.c b/source/mqtt_iot_sdk_metrics.c
@@ -15,15 +15,23 @@
 const size_t AWS_IOT_MAX_USERNAME_SIZE = UINT16_MAX;
 const size_t DEFAULT_QUERY_PARAM_COUNT = 10;
 
-// Build username query string from params_list, the caller is responsible to init and clean up output_username
-// If output_username is NULL, the function will just calculate the full username size and return it in
-// out_full_username_size
+/**
+ * Builds final username with query parameters appended.
+ *
+ * @param base_username The original username cursor
+ * @param base_username_length Length of base username to use (may differ from cursor length)
+ * @param params_list List of query parameters to append
+ * @param output_username [Optional] Buffer to write result. Caller must init/cleanup the buffer.
+ * @param out_final_username_size [Optional] Outputs the final username size
+ *
+ * @return AWS_OP_SUCCESS on success, AWS_OP_ERR on failure
+ */
 int s_build_username_query(
     const struct aws_byte_cursor *base_username,
     size_t base_username_length,
     const struct aws_array_list *params_list,
     struct aws_byte_buf *output_username,
-    size_t *out_full_username_size) {
+    size_t *out_final_username_size) {
 
     AWS_ASSERT(base_username);
     AWS_ASSERT(params_list);
@@ -34,8 +42,8 @@ int s_build_username_query(
         }
     }
 
-    if (out_full_username_size) {
-        *out_full_username_size = base_username_length;
+    if (out_final_username_size) {
+        *out_final_username_size = base_username_length;
     }
 
     struct aws_byte_cursor query_delim = aws_byte_cursor_from_c_str("?");
@@ -56,8 +64,8 @@ int s_build_username_query(
             }
         }
 
-        if (out_full_username_size) {
-            *out_full_username_size += 1;
+        if (out_final_username_size) {
+            *out_final_username_size += 1;
         }
 
         if (output_username) {
@@ -78,8 +86,8 @@ int s_build_username_query(
                 }
         }
 
-        if (out_full_username_size) {
-            *out_full_username_size += param.key.len + (param.value.len > 0 ? 1 : 0) + param.value.len;
+        if (out_final_username_size) {
+            *out_final_username_size += param.key.len + (param.value.len > 0 ? 1 : 0) + param.value.len;
         }
     }
 
@@ -93,6 +101,7 @@ int aws_mqtt_append_sdk_metrics_to_username(
     const struct aws_mqtt_iot_sdk_metrics *metrics,
     struct aws_byte_buf *output_username,
     size_t *out_full_username_size) {
+    AWS_PRECONDITION(aws_byte_buf_is_valid(output_username) && output_username->buffer == NULL);
 
     if (!allocator) {
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
@@ -126,11 +135,13 @@ int aws_mqtt_append_sdk_metrics_to_username(
     struct aws_array_list params_list;
     aws_array_list_init_dynamic(&params_list, allocator, DEFAULT_QUERY_PARAM_COUNT, sizeof(struct aws_uri_param));
 
+    // Looking for any existing query in the original username
     if (local_original_username.len > 0) {
         struct aws_byte_cursor question_mark_find = local_original_username;
 
         bool found_query = false;
-        // Find last question mark
+        // Find last question mark. The IoT service will trim string after last question mark and handle it as metrics
+        // metadata
         while (AWS_OP_SUCCESS ==
                aws_byte_cursor_find_exact(&question_mark_find, &question_mark_str, &question_mark_find)) {
             // Advance cursor to skip the "?" character
@@ -147,6 +158,7 @@ int aws_mqtt_append_sdk_metrics_to_username(
         }
     }
 
+    // Verify if the username already contains "SDK" and "Platform" fields.
     bool found_sdk = false;
     bool found_platform = false;
 
@@ -180,7 +192,7 @@ int aws_mqtt_append_sdk_metrics_to_username(
     }
 
     // Rebuild metrics string from params_list
-    // First parse to calculate total size
+    // First parse to get final username size
     size_t total_size = 0;
     s_build_username_query(&local_original_username, base_username_length, &params_list, NULL, &total_size);
 
@@ -288,7 +300,7 @@ void aws_mqtt_iot_sdk_metrics_storage_destroy(struct aws_mqtt_iot_sdk_metrics_st
 
 int aws_mqtt_validate_iot_sdk_metrics(const struct aws_mqtt_iot_sdk_metrics *metrics) {
     if (metrics == NULL) {
-        return AWS_OP_SUCCESS;
+        return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
     if (aws_mqtt_validate_utf8_text(metrics->library_name)) {
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -146,6 +146,7 @@ add_test_case(mqtt_append_sdk_metrics_existing_attributes)
 add_test_case(mqtt_append_sdk_metrics_special_chars)
 add_test_case(mqtt_append_sdk_metrics_long_strings)
 add_test_case(mqtt_append_sdk_metrics_invalid_utf8)
+add_test_case(mqtt_append_sdk_metrics_multiple_question_mark)
 
 # topic aliasing
 add_test_case(mqtt5_inbound_topic_alias_register_failure)
diff --git a/tests/shared_utils_tests.c b/tests/shared_utils_tests.c
@@ -284,6 +284,38 @@ static int s_test_mqtt_append_sdk_metrics_special_chars(struct aws_allocator *al
 
 AWS_TEST_CASE(mqtt_append_sdk_metrics_special_chars, s_test_mqtt_append_sdk_metrics_special_chars)
 
+static int s_test_mqtt_append_sdk_metrics_multiple_question_mark(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_mqtt_iot_sdk_metrics metrics = {.library_name = aws_byte_cursor_from_c_str("SDK/Test-1.0")};
+
+    struct aws_byte_buf output_username;
+    AWS_ZERO_STRUCT(output_username);
+
+    struct aws_byte_cursor original_username = aws_byte_cursor_from_c_str("user?name?test?");
+
+    ASSERT_SUCCESS(
+        aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, &metrics, &output_username, NULL));
+
+    struct aws_byte_cursor output_cursor = aws_byte_cursor_from_buf(&output_username);
+
+    /* Expected string: user?name?test?SDK=SDK/Test-1.0&Platform=<OS> */
+    struct aws_byte_cursor base_username = aws_byte_cursor_from_c_str("user?name?test");
+    struct aws_byte_buf expected_buf;
+    AWS_ZERO_STRUCT(expected_buf);
+    aws_test_mqtt_build_expected_metrics(
+        allocator, &base_username, aws_byte_cursor_from_c_str("SDK/Test-1.0"), NULL, &expected_buf);
+
+    ASSERT_TRUE(aws_byte_cursor_eq_byte_buf(&output_cursor, &expected_buf));
+
+    aws_byte_buf_clean_up(&output_username);
+    aws_byte_buf_clean_up(&expected_buf);
+
+    return AWS_OP_SUCCESS;
+}
+
+AWS_TEST_CASE(mqtt_append_sdk_metrics_multiple_question_mark, s_test_mqtt_append_sdk_metrics_multiple_question_mark)
+
 static int s_test_mqtt_append_sdk_metrics_long_strings(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
 
@@ -308,12 +340,12 @@ static int s_test_mqtt_append_sdk_metrics_long_strings(struct aws_allocator *all
     // aws_mqtt_append_sdk_metrics_to_username does not valid original username length
     ASSERT_SUCCESS(
         aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, &short_metrics, &output_username, NULL));
+    aws_byte_buf_clean_up(&output_username);
+
     // aws_mqtt_append_sdk_metrics_to_username fails when the extra metrics string exceeds buffer limit
     ASSERT_FAILS(
         aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, &metrics, &output_username, NULL));
 
-    aws_byte_buf_clean_up(&output_username);
-
     return AWS_OP_SUCCESS;
 }
 
