Skip to content

Commit dcd6c32

Browse files
xiazhveraxiazhvera
committed
improve aws_mqtt_append_sdk_metrics_to_username
1 parent ef0e817 commit dcd6c32

4 files changed

Lines changed: 134 additions & 147 deletions

File tree

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ AWS_MQTT_API struct aws_event_loop *aws_mqtt_client_connection_get_event_loop(
175175
* @param metrics The metrics configuration
176176
* @param output_username Buffer to store the modified username. If the function succeed, caller is responsible to
177177
* release the memory for output_username.
178+
* @param out_full_username_size If not NULL, will be set to the full size of the username with metrics appended
178179
*
179180
* @return AWS_OP_SUCCESS on success, AWS_OP_ERR on failure
180181
*/
@@ -183,15 +184,8 @@ int aws_mqtt_append_sdk_metrics_to_username(
183184
struct aws_allocator *allocator,
184185
const struct aws_byte_cursor *original_username,
185186
const struct aws_mqtt_iot_sdk_metrics metrics,
186-
struct aws_byte_buf *output_username);
187-
188-
/**
189-
* Get final username length
190-
*/
191-
AWS_MQTT_API
192-
size_t aws_mqtt_append_sdk_metrics_to_username_size(
193-
const struct aws_byte_cursor *original_username,
194-
const struct aws_mqtt_iot_sdk_metrics metrics);
187+
struct aws_byte_buf *output_username,
188+
size_t *out_full_username_size);
195189

196190
/**
197191
* Validates that all string fields in aws_mqtt_iot_sdk_metrics are valid UTF-8

‎source/client.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,8 @@ static void s_mqtt_client_init(
640640
connection->allocator,
641641
&username_cur,
642642
connection->metrics->storage_view,
643-
&connection->username_with_metrics_buf) == AWS_OP_SUCCESS) {
643+
&connection->username_with_metrics_buf,
644+
NULL) == AWS_OP_SUCCESS) {
644645
username_cur = aws_byte_cursor_from_buf(&connection->username_with_metrics_buf);
645646
} else {
646647
AWS_LOGF_WARN(

‎source/mqtt_iot_sdk_metrics.c‎

Lines changed: 116 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,78 @@
44
*/
55

66
#include <aws/common/byte_buf.h>
7-
#include <aws/common/encoding.h>
8-
#include <aws/common/string.h>
97
#include <aws/common/system_info.h>
8+
#include <aws/common/uri.h>
109
#include <aws/mqtt/mqtt.h>
1110
#include <aws/mqtt/private/client_impl_shared.h>
1211

1312
#include <stdio.h>
1413

15-
// Maximum MQTT5 Content Type size https://docs.aws.amazon.com/general/latest/gr/iot-core.html#thing-limits
16-
const int AWS_IOT_MAX_CONTENT_SIZE = 256;
14+
// MQTT payload size https://docs.aws.amazon.com/general/latest/gr/iot-core.html#thing-limits
15+
const int AWS_IOT_MAX_CONTENT_SIZE = 128 * 1024;
16+
const size_t DEFAULT_QUERY_PARAM_COUNT = 10;
17+
18+
// Build username query string from params_list, the caller is responsible to init and clean up output_username
19+
// If output_username is NULL, the function will just calculate the full username size and return it in
20+
// out_full_username_size
21+
int s_build_username_query(
22+
const struct aws_byte_cursor *base_username,
23+
size_t base_username_length,
24+
const struct aws_array_list *params_list,
25+
struct aws_byte_buf *output_username,
26+
size_t *out_full_username_size) {
27+
28+
if (output_username) {
29+
aws_byte_buf_write(output_username, base_username->ptr, base_username_length);
30+
}
31+
32+
if (out_full_username_size) {
33+
*out_full_username_size = base_username_length;
34+
}
35+
36+
struct aws_byte_cursor query_delim = aws_byte_cursor_from_c_str("?");
37+
struct aws_byte_cursor query_param_amp = aws_byte_cursor_from_c_str("&");
38+
struct aws_byte_cursor key_value_delim = aws_byte_cursor_from_c_str("=");
39+
40+
size_t params_count = aws_array_list_length(params_list);
41+
for (size_t i = 0; i < params_count; ++i) {
42+
struct aws_uri_param param;
43+
AWS_ZERO_STRUCT(param);
44+
aws_array_list_get_at(params_list, &param, i);
45+
46+
if (i == 0 && output_username) {
47+
aws_byte_buf_append(output_username, &query_delim);
48+
} else if (i > 0 && output_username) {
49+
aws_byte_buf_append(output_username, &query_param_amp);
50+
}
51+
52+
if (out_full_username_size) {
53+
*out_full_username_size += 1;
54+
}
55+
56+
if (output_username) {
57+
aws_byte_buf_append(output_username, &param.key);
58+
aws_byte_buf_append(output_username, &key_value_delim);
59+
aws_byte_buf_append(output_username, &param.value);
60+
}
61+
62+
if (out_full_username_size) {
63+
*out_full_username_size += param.key.len + 1 + param.value.len;
64+
}
65+
}
66+
67+
return AWS_OP_SUCCESS;
68+
}
1769

70+
// TODO Future Work: we ignored the metadata field for now, will add them in future support
1871
int aws_mqtt_append_sdk_metrics_to_username(
1972
struct aws_allocator *allocator,
2073
const struct aws_byte_cursor *original_username,
2174
const struct aws_mqtt_iot_sdk_metrics metrics,
22-
struct aws_byte_buf *output_username) {
75+
struct aws_byte_buf *output_username,
76+
size_t *out_full_username_size) {
2377

24-
if (!allocator || !output_username) {
78+
if (!allocator) {
2579
return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
2680
}
2781

@@ -35,153 +89,85 @@ int aws_mqtt_append_sdk_metrics_to_username(
3589
return AWS_OP_ERR;
3690
}
3791

38-
/* Check if the attributes already exists in username */
39-
bool has_sdk = false;
40-
bool has_platform = false;
41-
bool has_query = false;
42-
43-
struct aws_byte_cursor sdk_str = aws_byte_cursor_from_c_str("SDK=");
44-
struct aws_byte_cursor platform_str = aws_byte_cursor_from_c_str("Platform=");
92+
int result = AWS_OP_ERR;
93+
// The length of the base username part not including query parameters
94+
size_t base_username_length = 0;
4595
struct aws_byte_cursor question_mark_str = aws_byte_cursor_from_c_str("?");
46-
struct aws_byte_cursor amp = aws_byte_cursor_from_c_str("&");
96+
struct aws_byte_cursor sdk_str = aws_byte_cursor_from_c_str("SDK");
97+
struct aws_byte_cursor platform_str = aws_byte_cursor_from_c_str("Platform");
4798

48-
// TODO: we ignored the metadata field for now, need to add them later
99+
struct aws_array_list params_list;
100+
aws_array_list_init_dynamic(&params_list, allocator, DEFAULT_QUERY_PARAM_COUNT, sizeof(struct aws_uri_param));
49101

50102
if (original_username && original_username->len > 0) {
51103
struct aws_byte_cursor question_mark_find;
52-
has_query =
53-
AWS_OP_SUCCESS == aws_byte_cursor_find_exact(original_username, &question_mark_str, &question_mark_find);
54-
if (has_query) {
55-
struct aws_byte_cursor temp_cursor;
56-
has_sdk = AWS_OP_SUCCESS == aws_byte_cursor_find_exact(&question_mark_find, &sdk_str, &temp_cursor);
57-
has_platform =
58-
AWS_OP_SUCCESS == aws_byte_cursor_find_exact(&question_mark_find, &platform_str, &temp_cursor);
59-
}
60-
}
61104

62-
/* Add SDK if not present */
63-
if (!has_sdk) {
64-
if (aws_byte_buf_append(&metrics_string, &sdk_str)) {
65-
goto error;
66-
}
67-
68-
struct aws_byte_cursor sdk_attr_value =
69-
metrics.library_name.len > 0 ? metrics.library_name : aws_byte_cursor_from_c_str("IoTDeviceSDK/C");
70-
if (aws_byte_buf_append(&metrics_string, &sdk_attr_value)) {
71-
goto error;
105+
if (AWS_OP_SUCCESS == aws_byte_cursor_find_exact(original_username, &question_mark_str, &question_mark_find)) {
106+
base_username_length = question_mark_find.ptr - original_username->ptr;
107+
// Advance cursor to skip the "?" character
108+
aws_byte_cursor_advance(&question_mark_find, 1);
109+
aws_byte_buf_append(&metrics_string, &question_mark_find);
110+
aws_query_string_params(question_mark_find, &params_list);
111+
} else {
112+
base_username_length = original_username->len;
72113
}
73114
}
74115

75-
/* Add Platform if not present */
76-
if (!has_platform) {
77-
struct aws_byte_cursor platform_cursor = aws_get_platform_build_os_string();
78-
if (platform_cursor.len > 0) {
79-
if (metrics_string.len > 0) {
80-
if (aws_byte_buf_append(&metrics_string, &amp)) {
81-
goto error;
82-
}
83-
}
84-
85-
if (aws_byte_buf_append(&metrics_string, &platform_str) ||
86-
aws_byte_buf_append(&metrics_string, &platform_cursor)) {
87-
goto error;
88-
}
116+
bool found_sdk = false;
117+
bool found_platform = false;
118+
119+
size_t params_count = aws_array_list_length(&params_list);
120+
for (size_t i = 0; i < params_count; ++i) {
121+
struct aws_uri_param param;
122+
AWS_ZERO_STRUCT(param);
123+
aws_array_list_get_at(&params_list, &param, i);
124+
if (aws_byte_cursor_eq(&param.key, &sdk_str)) {
125+
found_sdk = true;
126+
} else if (aws_byte_cursor_eq(&param.key, &platform_str)) {
127+
found_platform = true;
89128
}
90129
}
91130

92-
/* Build final output */
93-
// TODO: we should consider the case where total size extceeds MQTT username limit
94-
size_t total_size = (original_username ? original_username->len : 0) + metrics_string.len + 1;
95-
96-
if (aws_byte_buf_init(output_username, allocator, total_size)) {
97-
goto error;
131+
if (!found_sdk) {
132+
struct aws_uri_param sdk_params = {
133+
.key = sdk_str,
134+
.value = metrics.library_name.len > 0 ? metrics.library_name : aws_byte_cursor_from_c_str("IoTDeviceSDK/C"),
135+
};
136+
aws_array_list_push_back(&params_list, &sdk_params);
98137
}
99138

100-
/* Add original username */
101-
if (original_username && original_username->len > 0) {
102-
if (aws_byte_buf_append(output_username, original_username)) {
103-
goto error_output;
104-
}
139+
if (!found_platform) {
140+
struct aws_uri_param platform_params = {
141+
.key = platform_str,
142+
.value = aws_get_platform_build_os_string(),
143+
};
144+
aws_array_list_push_back(&params_list, &platform_params);
105145
}
106146

107-
/* Add metrics with separator */
108-
if (metrics_string.len > 0) {
109-
110-
struct aws_byte_cursor metrics_cursor = aws_byte_cursor_from_buf(&metrics_string);
147+
// Rebuild metrics string from params_list
148+
// First path to calculate total size
149+
size_t total_size = 0;
150+
s_build_username_query(original_username, base_username_length, &params_list, NULL, &total_size);
111151

112-
struct aws_byte_cursor separator = has_query ? amp : question_mark_str;
113-
if (aws_byte_buf_append(output_username, &separator)) {
114-
goto error_output;
115-
}
116-
117-
if (aws_byte_buf_append(output_username, &metrics_cursor)) {
118-
goto error_output;
119-
}
152+
if (output_username && aws_byte_buf_init(output_username, allocator, total_size)) {
153+
goto cleanup;
120154
}
121155

122-
aws_byte_buf_clean_up(&metrics_string);
123-
return AWS_OP_SUCCESS;
156+
// build final output username
157+
s_build_username_query(
158+
original_username, base_username_length, &params_list, output_username, out_full_username_size);
124159

125-
error_output:
126-
aws_byte_buf_clean_up(output_username);
127-
error:
128160
aws_byte_buf_clean_up(&metrics_string);
129-
return AWS_OP_ERR;
130-
}
131-
132-
size_t aws_mqtt_append_sdk_metrics_to_username_size(
133-
const struct aws_byte_cursor *original_username,
134-
const struct aws_mqtt_iot_sdk_metrics metrics) {
135-
136-
/* Build metrics string */
137-
size_t metrics_string_size = 0;
138-
139-
/* Check if the attributes already exists in username */
140-
bool has_sdk = false;
141-
bool has_platform = false;
142-
bool has_query = false;
143-
144-
struct aws_byte_cursor sdk_str = aws_byte_cursor_from_c_str("SDK=");
145-
struct aws_byte_cursor platform_str = aws_byte_cursor_from_c_str("Platform=");
146-
struct aws_byte_cursor question_mark_str = aws_byte_cursor_from_c_str("?");
161+
result = AWS_OP_SUCCESS;
147162

148-
// TODO: we ignored the metadata field for now, need to add them in the future
149-
150-
if (original_username && original_username->len > 0) {
151-
struct aws_byte_cursor question_mark_find;
152-
has_query =
153-
AWS_OP_SUCCESS == aws_byte_cursor_find_exact(original_username, &question_mark_str, &question_mark_find);
154-
if (has_query) {
155-
struct aws_byte_cursor temp_cursor;
156-
has_sdk = AWS_OP_SUCCESS == aws_byte_cursor_find_exact(&question_mark_find, &sdk_str, &temp_cursor);
157-
has_platform =
158-
AWS_OP_SUCCESS == aws_byte_cursor_find_exact(&question_mark_find, &platform_str, &temp_cursor);
159-
}
160-
}
161-
162-
/* Add SDK if not present */
163-
if (!has_sdk) {
164-
metrics_string_size += sdk_str.len;
165-
166-
struct aws_byte_cursor sdk_attr_value =
167-
metrics.library_name.len > 0 ? metrics.library_name : aws_byte_cursor_from_c_str("IoTDeviceSDK/C");
168-
169-
metrics_string_size += sdk_attr_value.len;
163+
cleanup:
164+
if (aws_array_list_is_valid(&params_list)) {
165+
aws_array_list_clean_up(&params_list);
170166
}
171167

172-
/* Add Platform if not present */
173-
if (!has_platform) {
174-
struct aws_byte_cursor platform_cursor = aws_get_platform_build_os_string();
175-
if (platform_cursor.len > 0) {
176-
// Add amp sign if metrics is not empty
177-
metrics_string_size += metrics_string_size + metrics_string_size > 0 ? 1 : 0;
178-
179-
metrics_string_size += platform_str.len + platform_cursor.len;
180-
}
168+
if (result == AWS_OP_ERR && aws_byte_buf_is_valid(output_username)) {
169+
aws_byte_buf_clean_up(output_username);
181170
}
182-
183-
/* Build final output */
184-
/* original_username + 1 character of "?" or "&" + metrics string*/
185-
size_t total_size = (original_username ? original_username->len : 0) + 1 + metrics_string_size;
186-
return total_size;
171+
aws_byte_buf_clean_up(&metrics_string);
172+
return result;
187173
}

‎tests/shared_utils_tests.c‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ static int s_test_mqtt_append_sdk_metrics_empty(struct aws_allocator *allocator,
169169

170170
struct aws_byte_cursor original_username = aws_byte_cursor_from_c_str("");
171171

172-
ASSERT_SUCCESS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
172+
ASSERT_SUCCESS(
173+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
173174

174175
struct aws_byte_cursor output_cursor = aws_byte_cursor_from_buf(&output_username);
175176

@@ -204,7 +205,8 @@ static int s_test_mqtt_append_sdk_metrics_basic(struct aws_allocator *allocator,
204205

205206
struct aws_byte_cursor original_username = aws_byte_cursor_from_c_str("TEST_USERNAME");
206207

207-
ASSERT_SUCCESS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
208+
ASSERT_SUCCESS(
209+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
208210

209211
struct aws_byte_cursor output_cursor = aws_byte_cursor_from_buf(&output_username);
210212

@@ -240,7 +242,8 @@ static int s_test_mqtt_append_sdk_metrics_existing_attributes(struct aws_allocat
240242
struct aws_byte_cursor original_username =
241243
aws_byte_cursor_from_c_str("user?SDK=ExistingSDK&Platform=ExistingPlatform");
242244

243-
ASSERT_SUCCESS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
245+
ASSERT_SUCCESS(
246+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
244247

245248
struct aws_byte_cursor output_cursor = aws_byte_cursor_from_buf(&output_username);
246249

@@ -263,7 +266,8 @@ static int s_test_mqtt_append_sdk_metrics_special_chars(struct aws_allocator *al
263266

264267
struct aws_byte_cursor original_username = aws_byte_cursor_from_c_str("user@domain.com");
265268

266-
ASSERT_SUCCESS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
269+
ASSERT_SUCCESS(
270+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
267271

268272
struct aws_byte_cursor output_cursor = aws_byte_cursor_from_buf(&output_username);
269273

@@ -306,9 +310,10 @@ static int s_test_mqtt_append_sdk_metrics_long_strings(struct aws_allocator *all
306310

307311
// aws_mqtt_append_sdk_metrics_to_username does not valid original username length
308312
ASSERT_SUCCESS(
309-
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, short_metrics, &output_username));
313+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, short_metrics, &output_username, NULL));
310314
// aws_mqtt_append_sdk_metrics_to_username fails when the extra metrics string exceeds buffer limit
311-
ASSERT_FAILS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
315+
ASSERT_FAILS(
316+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
312317
ASSERT_INT_EQUALS(aws_last_error(), AWS_ERROR_DEST_COPY_TOO_SMALL);
313318

314319
aws_byte_buf_clean_up(&output_username);
@@ -332,7 +337,8 @@ static int s_test_mqtt_append_sdk_metrics_invalid_utf8(struct aws_allocator *all
332337
struct aws_byte_cursor original_username = aws_byte_cursor_from_c_str("testuser");
333338

334339
/* Should fail due to invalid UTF-8 */
335-
ASSERT_FAILS(aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username));
340+
ASSERT_FAILS(
341+
aws_mqtt_append_sdk_metrics_to_username(allocator, &original_username, metrics, &output_username, NULL));
336342
ASSERT_INT_EQUALS(aws_last_error(), AWS_ERROR_INVALID_UTF8);
337343

338344
aws_byte_buf_clean_up(&output_username);

0 commit comments

Comments
 (0)