Skip to content

Commit 5defc6a

Browse files
xiazhveraxiazhvera
committed
update CR
1 parent f05ff9e commit 5defc6a

4 files changed

Lines changed: 104 additions & 95 deletions

File tree

‎include/aws/mqtt/mqtt.h‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -123,26 +123,6 @@ struct aws_mqtt_metadata_entry {
123123
* IoT SDK metrics configuration structure
124124
*/
125125
struct aws_mqtt_iot_sdk_metrics {
126-
/**
127-
* TODO: This is a place holder for metadata entries.
128-
* The metadata_entries is not added to username yet.
129-
*
130-
* DO NOT USE.
131-
*
132-
* Array of metadata entries
133-
*/
134-
// struct aws_mqtt_metadata_entry *metadata_entries;
135-
136-
/**
137-
* TODO: This is a place holder for metadata entries.
138-
* The metadata_entries is not added to username yet.
139-
*
140-
* DO NOT USE.
141-
*
142-
* Number of metadata entries
143-
*/
144-
// size_t metadata_count;
145-
146126
/**
147127
* Library name string (SDK attribute)
148128
*/

‎include/aws/mqtt/private/mqtt_iot_sdk_metrics.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#ifndef AWS_MQTT_IOT_SDK_METRICS_H
77
#define AWS_MQTT_IOT_SDK_METRICS_H
88

9+
#include <aws/mqtt/mqtt.h>
10+
911
/* Storage for `aws_mqtt_iot_sdk_metrics`. */
1012
struct aws_mqtt_iot_sdk_metrics_storage {
1113
struct aws_allocator *allocator;
@@ -26,13 +28,14 @@ AWS_MQTT_API struct aws_mqtt_iot_sdk_metrics_storage *aws_mqtt_iot_sdk_metrics_s
2628
AWS_MQTT_API void aws_mqtt_iot_sdk_metrics_storage_destroy(struct aws_mqtt_iot_sdk_metrics_storage *metrics_storage);
2729

2830
/**
29-
* Appends SDK metrics to the username
31+
* Builds a new username by appending SDK metrics to the original username.
3032
*
33+
* @param allocator The allocator to use for memory allocation
3134
* @param original_username The original username
3235
* @param metrics The metrics configuration
33-
* @param output_username Buffer to store the modified username. If the function succeed, caller is responsible to
34-
* release the memory for output_username.
35-
* @param out_full_username_size If not NULL, will be set to the full size of the username with metrics appended
36+
* @param output_username Buffer that will be initialized and populated with the new final username. The function
37+
* expects this buffer to be uninitialized. On success, the caller is responsible for cleaning up the buffer.
38+
* @param out_full_username_size If not NULL, will be set to the full size of the final username
3639
*
3740
* @return AWS_OP_SUCCESS on success, AWS_OP_ERR on failure
3841
*/

‎source/client.c‎

Lines changed: 68 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ static void s_mqtt_client_init(
505505

506506
struct aws_mqtt_client_connection_311_impl *connection = user_data;
507507
struct aws_byte_buf username_with_metrics_buf;
508-
aws_byte_buf_init(&username_with_metrics_buf, connection->allocator, 0);
508+
AWS_ZERO_STRUCT(username_with_metrics_buf);
509509

510510
if (error_code != AWS_OP_SUCCESS) {
511511
/* client shutdown already handles this case, so just call that. */
@@ -634,16 +634,20 @@ static void s_mqtt_client_init(
634634
}
635635

636636
/* Apply metrics to username if configured */
637-
if (aws_mqtt_append_sdk_metrics_to_username(
638-
connection->allocator,
639-
&username_cur,
640-
connection->metrics_storage ? &connection->metrics_storage->storage_view : NULL,
641-
&username_with_metrics_buf,
642-
NULL) == AWS_OP_SUCCESS) {
643-
username_cur = aws_byte_cursor_from_buf(&username_with_metrics_buf);
644-
} else {
645-
AWS_LOGF_WARN(
646-
AWS_LS_MQTT_CLIENT, "id=%p: Failed to apply metrics to username, using original", (void *)connection);
637+
if (connection->metrics_storage) {
638+
if (aws_mqtt_append_sdk_metrics_to_username(
639+
connection->allocator,
640+
&username_cur,
641+
&connection->metrics_storage->storage_view,
642+
&username_with_metrics_buf,
643+
NULL) == AWS_OP_SUCCESS) {
644+
username_cur = aws_byte_cursor_from_buf(&username_with_metrics_buf);
645+
} else {
646+
AWS_LOGF_WARN(
647+
AWS_LS_MQTT_CLIENT,
648+
"id=%p: Failed to apply metrics to username, using original",
649+
(void *)connection);
650+
}
647651
}
648652

649653
if (aws_byte_cursor_is_valid(&username_cur)) {
@@ -653,16 +657,20 @@ static void s_mqtt_client_init(
653657
(void *)connection,
654658
AWS_BYTE_CURSOR_PRI(username_cur));
655659

656-
struct aws_byte_cursor password_cur = {
657-
.ptr = NULL,
658-
.len = 0,
659-
};
660+
struct aws_byte_cursor password_cur;
661+
AWS_ZERO_STRUCT(password_cur);
660662

661663
if (connection->password) {
662664
password_cur = aws_byte_cursor_from_string(connection->password);
663665
}
664666

665667
aws_mqtt_packet_connect_add_credentials(&connect, username_cur, password_cur);
668+
} else {
669+
AWS_LOGF_WARN(
670+
AWS_LS_MQTT_CLIENT,
671+
"id=%p: Failed to set username and password. Most likely there is an issue in metrics. (e.x.: username "
672+
"is empty and metrics string exceed the size limit). ",
673+
(void *)connection);
666674
}
667675
}
668676

@@ -685,20 +693,15 @@ static void s_mqtt_client_init(
685693
goto handle_error;
686694
}
687695

688-
if (aws_byte_buf_is_valid(&username_with_metrics_buf)) {
689-
aws_byte_buf_clean_up(&username_with_metrics_buf);
690-
}
691-
696+
aws_byte_buf_clean_up(&username_with_metrics_buf);
692697
return;
693698

694699
handle_error:
695700
MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_connection_complete, aws_last_error(), 0, false);
696701
MQTT_CLIENT_CALL_CALLBACK_ARGS(connection, on_connection_failure, aws_last_error());
697702
aws_channel_shutdown(channel, aws_last_error());
698703

699-
if (aws_byte_buf_is_valid(&username_with_metrics_buf)) {
700-
aws_byte_buf_clean_up(&username_with_metrics_buf);
701-
}
704+
aws_byte_buf_clean_up(&username_with_metrics_buf);
702705

703706
if (message) {
704707
aws_mem_release(message->allocator, message);
@@ -1881,7 +1884,7 @@ static enum aws_mqtt_client_request_state s_subscribe_send(uint16_t packet_id, b
18811884
return AWS_MQTT_CLIENT_REQUEST_ERROR;
18821885
}
18831886

1884-
AWS_VARIABLE_LENGTH_ARRAY(uint8_t, transaction_buf, num_topics * aws_mqtt_topic_tree_action_size);
1887+
AWS_VARIABLE_LENGTH_ARRAY(uint8_t, transaction_buf, num_topics *aws_mqtt_topic_tree_action_size);
18851888
struct aws_array_list transaction;
18861889
aws_array_list_init_static(&transaction, transaction_buf, num_topics, aws_mqtt_topic_tree_action_size);
18871890

@@ -2674,7 +2677,7 @@ static enum aws_mqtt_client_request_state s_unsubscribe_send(
26742677

26752678
static const size_t num_topics = 1;
26762679

2677-
AWS_VARIABLE_LENGTH_ARRAY(uint8_t, transaction_buf, num_topics * aws_mqtt_topic_tree_action_size);
2680+
AWS_VARIABLE_LENGTH_ARRAY(uint8_t, transaction_buf, num_topics *aws_mqtt_topic_tree_action_size);
26782681
struct aws_array_list transaction;
26792682
aws_array_list_init_static(&transaction, transaction_buf, num_topics, aws_mqtt_topic_tree_action_size);
26802683

@@ -3380,33 +3383,17 @@ int aws_mqtt_client_connection_set_on_operation_statistics_handler(
33803383
static int s_aws_mqtt_client_connection_311_set_metrics(void *impl, const struct aws_mqtt_iot_sdk_metrics *metrics) {
33813384

33823385
struct aws_mqtt_client_connection_311_impl *connection = impl;
3383-
33843386
AWS_PRECONDITION(connection);
3385-
if (s_check_connection_state_for_configuration(connection)) {
3386-
return aws_raise_error(AWS_ERROR_INVALID_STATE);
3387-
}
3388-
3389-
if (metrics != NULL && aws_mqtt_validate_iot_sdk_metrics(metrics) == AWS_OP_ERR) {
3390-
AWS_LOGF_ERROR(
3391-
AWS_LS_MQTT_CLIENT, "id=%p: Invalid utf8 or forbidden codepoints in metrics.", (void *)connection);
3392-
return aws_raise_error(AWS_ERROR_INVALID_UTF8);
3393-
}
3394-
3395-
// TODO: Validate metadata value, when we had metadata in place.
3396-
3397-
AWS_LOGF_TRACE(AWS_LS_MQTT_CLIENT, "id=%p: Setting IoT SDK metrics", (void *)connection);
3398-
3399-
/* Clean up existing metrics if any */
3400-
if (connection->metrics_storage) {
3401-
aws_mqtt_iot_sdk_metrics_storage_destroy(connection->metrics_storage);
3402-
connection->metrics_storage = NULL;
3403-
}
34043387

3388+
struct aws_mqtt_iot_sdk_metrics_storage *metrics_storage = NULL;
34053389
if (metrics) {
3406-
/* Initialize metrics storage */
3407-
connection->metrics_storage = aws_mqtt_iot_sdk_metrics_storage_new(connection->allocator, metrics);
3390+
if (aws_mqtt_validate_iot_sdk_metrics(metrics) == AWS_OP_ERR) {
3391+
AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Invalid metrics.", (void *)connection);
3392+
return aws_raise_error(AWS_ERROR_INVALID_UTF8);
3393+
}
34083394

3409-
if (!connection->metrics_storage) {
3395+
metrics_storage = aws_mqtt_iot_sdk_metrics_storage_new(connection->allocator, metrics);
3396+
if (!metrics_storage) {
34103397
int error = aws_last_error();
34113398
AWS_LOGF_ERROR(
34123399
AWS_LS_MQTT_CLIENT,
@@ -3418,7 +3405,40 @@ static int s_aws_mqtt_client_connection_311_set_metrics(void *impl, const struct
34183405
}
34193406
}
34203407

3421-
return AWS_OP_SUCCESS;
3408+
int result = AWS_OP_ERR;
3409+
3410+
/* BEGIN CRITICAL SECTION */
3411+
mqtt_connection_lock_synced_data(connection);
3412+
3413+
if (connection->synced_data.state != AWS_MQTT_CLIENT_STATE_DISCONNECTED &&
3414+
connection->synced_data.state != AWS_MQTT_CLIENT_STATE_CONNECTED) {
3415+
AWS_LOGF_ERROR(
3416+
AWS_LS_MQTT_CLIENT,
3417+
"id=%p: Connection is currently pending connect/disconnect. Unable to make configuration changes until "
3418+
"pending operation completes.",
3419+
(void *)connection);
3420+
aws_raise_error(AWS_ERROR_INVALID_STATE);
3421+
goto done;
3422+
}
3423+
3424+
/* Clean up existing metrics if any */
3425+
if (connection->metrics_storage) {
3426+
aws_mqtt_iot_sdk_metrics_storage_destroy(connection->metrics_storage);
3427+
}
3428+
connection->metrics_storage = metrics_storage;
3429+
AWS_LOGF_TRACE(AWS_LS_MQTT_CLIENT, "id=%p: Setting IoT SDK metrics", (void *)connection);
3430+
metrics_storage = NULL;
3431+
result = AWS_OP_SUCCESS;
3432+
3433+
done:
3434+
mqtt_connection_unlock_synced_data(connection);
3435+
/* END CRITICAL SECTION */
3436+
3437+
if (metrics_storage) {
3438+
aws_mqtt_iot_sdk_metrics_storage_destroy(metrics_storage);
3439+
}
3440+
3441+
return result;
34223442
}
34233443

34243444
static struct aws_mqtt_client_connection *s_aws_mqtt_client_connection_311_acquire(void *impl) {

‎source/mqtt_iot_sdk_metrics.c‎

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ int s_build_username_query(
2929
AWS_ASSERT(params_list);
3030

3131
if (output_username) {
32-
aws_byte_buf_write(output_username, base_username->ptr, base_username_length);
32+
if (!aws_byte_buf_write(output_username, base_username->ptr, base_username_length)) {
33+
return AWS_OP_ERR;
34+
}
3335
}
3436

3537
if (out_full_username_size) {
@@ -46,26 +48,36 @@ int s_build_username_query(
4648
AWS_ZERO_STRUCT(param);
4749
aws_array_list_get_at(params_list, &param, i);
4850

49-
if (i == 0 && output_username) {
50-
aws_byte_buf_append(output_username, &query_delim);
51-
} else if (i > 0 && output_username) {
52-
aws_byte_buf_append(output_username, &query_param_amp);
51+
if (output_username) {
52+
if (i == 0) {
53+
aws_byte_buf_append(output_username, &query_delim);
54+
} else {
55+
aws_byte_buf_append(output_username, &query_param_amp);
56+
}
5357
}
5458

5559
if (out_full_username_size) {
5660
*out_full_username_size += 1;
5761
}
5862

5963
if (output_username) {
60-
if (param.key.len > 0 && (aws_byte_buf_append(output_username, &param.key))) {
61-
return AWS_OP_ERR;
64+
if (param.key.len > 0) {
65+
// append key if key exists
66+
if (aws_byte_buf_append(output_username, &param.key)) {
67+
return AWS_OP_ERR;
68+
}
6269
}
6370

64-
if (param.value.len > 0 && ((aws_byte_buf_append(output_username, &key_value_delim)) ||
65-
aws_byte_buf_append(output_username, &param.value))) {
66-
return AWS_OP_ERR;
67-
}
71+
// append value if value exists
72+
if (param.value.len > 0)
73+
// Note: If value exists without a key, append "=" and value (e.g., "?=abc").
74+
// Please note server treats "a=", "a", and "=a" equivalently.
75+
if ((aws_byte_buf_append(output_username, &key_value_delim)) ||
76+
aws_byte_buf_append(output_username, &param.value)) {
77+
return AWS_OP_ERR;
78+
}
6879
}
80+
6981
if (out_full_username_size) {
7082
*out_full_username_size += param.key.len + (param.value.len > 0 ? 1 : 0) + param.value.len;
7183
}
@@ -101,7 +113,7 @@ int aws_mqtt_append_sdk_metrics_to_username(
101113
}
102114

103115
if (aws_mqtt_validate_iot_sdk_metrics(metrics)) {
104-
return AWS_OP_ERR;
116+
return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
105117
}
106118

107119
int result = AWS_OP_ERR;
@@ -173,6 +185,9 @@ int aws_mqtt_append_sdk_metrics_to_username(
173185
s_build_username_query(&local_original_username, base_username_length, &params_list, NULL, &total_size);
174186

175187
if (total_size > AWS_IOT_MAX_USERNAME_SIZE) {
188+
AWS_LOGF_ERROR(
189+
AWS_LL_DEBUG, "Failed to append SDK metrics to username: resulting username exceeds max username size.");
190+
aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
176191
goto cleanup;
177192
}
178193

@@ -189,11 +204,9 @@ int aws_mqtt_append_sdk_metrics_to_username(
189204
result = AWS_OP_SUCCESS;
190205

191206
cleanup:
192-
if (aws_array_list_is_valid(&params_list)) {
193-
aws_array_list_clean_up(&params_list);
194-
}
207+
aws_array_list_clean_up(&params_list);
195208

196-
if (result == AWS_OP_ERR && aws_byte_buf_is_valid(output_username)) {
209+
if (result == AWS_OP_ERR) {
197210
aws_byte_buf_clean_up(output_username);
198211
}
199212
return result;
@@ -209,15 +222,8 @@ size_t aws_mqtt_iot_sdk_metrics_compute_storage_size(const struct aws_mqtt_iot_s
209222
}
210223

211224
size_t storage_size = 0;
212-
213225
storage_size += metrics->library_name.len;
214226

215-
// TODO: add metadata entries when enabled
216-
// for (size_t i = 0; i < metrics->metadata_count; ++i) {
217-
// storage_size += metrics->metadata_entries[i].key.len;
218-
// storage_size += metrics->metadata_entries[i].value.len;
219-
// }
220-
221227
return storage_size;
222228
}
223229

0 commit comments

Comments
 (0)