Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/nng/supplemental/nanolib/conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ typedef struct conf_websocket conf_websocket;
#define NO_RETAIN 2 // default retain flag value, none 0, 1
#define NO_QOS 3 // default QoS level value for forwarding bridge msg, 3 = keep old qos

typedef struct {
char *topic;
size_t topic_len;
} exclusions;
Comment on lines +225 to +228
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for usages of exclusions_list or exclusions_count beyond parsing/printing/cleanup

# Search for exclusions usage in the codebase
rg -n "exclusions" --type=c -C3 | grep -v "conf_ver2.c" | grep -v "conf\.c" | grep -v "conf\.h"

Repository: nanomq/NanoNNG

Length of output: 72


🏁 Script executed:

# Search for all exclusions usage without filters to see full picture
rg -n "exclusions" --type=c -B2 -A2

Repository: nanomq/NanoNNG

Length of output: 3934


🏁 Script executed:

# Search for message filtering/forwarding logic that might use exclusions_list
rg -n "forward|filter" --type=c -l | head -20

Repository: nanomq/NanoNNG

Length of output: 654


🏁 Script executed:

# Check the conf.c file specifically for exclusions usage and filtering logic
fd -e c -e h "^conf\.c$" -x cat -n {}

Repository: nanomq/NanoNNG

Length of output: 50371


🏁 Script executed:

# Search for bridge message forwarding/processing functions
rg -n "forward|bridge" --type=c -l | grep -E "bridge|mqtt" | head -20

Repository: nanomq/NanoNNG

Length of output: 462


🏁 Script executed:

# Search for any use of exclusions_list for filtering in the codebase
rg -n "exclusions_list\[" --type=c -A3 -B3

Repository: nanomq/NanoNNG

Length of output: 1099


🏁 Script executed:

# Look for functions that might perform topic filtering during message forwarding
rg -n "topic.*match|match.*topic|filter.*topic" --type=c -l | head -10

Repository: nanomq/NanoNNG

Length of output: 278


🏁 Script executed:

# Find bridge-related source files
fd -e c "bridge" src/ | head -20

Repository: nanomq/NanoNNG

Length of output: 40


🏁 Script executed:

# Look for functions that process bridge messages/forwarding
rg -n "bridge.*forward|forward.*bridge|bridge.*message" --type=c -l | head -10

Repository: nanomq/NanoNNG

Length of output: 166


🏁 Script executed:

# Search for any function that checks or matches exclusions
rg -n "exclusion" --type=c -B5 -A5 | head -100

Repository: nanomq/NanoNNG

Length of output: 6778


Exclusions are parsed but never enforced at runtime.

The exclusions_list is populated during configuration parsing and freed during destruction, but there's no code that actually uses these exclusions to filter messages during bridge forwarding. This means the feature is incomplete—users can configure exclusions, but they won't have any effect.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/nng/supplemental/nanolib/conf.h` around lines 225 - 228, The
exclusions parsed into the exclusions struct (exclusions { char *topic; size_t
topic_len; }) and stored in exclusions_list are never consulted during bridge
forwarding; update the bridge message-forwarding path (where messages are
forwarded across the bridge) to iterate the exclusions_list and skip forwarding
when a message's topic matches any exclusions[i].topic using topic_len-aware
comparison (memcmp or strncmp) or exact string compare as appropriate; ensure
you use the same lifetime/ownership semantics as the parser (don't free entries
when skipping) and add unit/inline checks so that the forwarding function
returns early when a match is found.


typedef struct {
char *remote_topic;
uint32_t remote_topic_len;
Expand Down Expand Up @@ -309,10 +314,12 @@ struct conf_bridge_node {
uint64_t resend_interval; // resend caching message interval (ms)
uint64_t resend_wait;
size_t sub_count;
size_t exclusions_count;
size_t forwards_count;
size_t max_recv_queue_len;
size_t max_send_queue_len;
topics **forwards_list;
exclusions **exclusions_list;
uint64_t parallel;
topics **sub_list;
conf_tls tls;
Expand Down
26 changes: 23 additions & 3 deletions src/supplemental/nanolib/conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -3580,6 +3580,19 @@ conf_bridge_node_destroy(conf_bridge_node *node)
cvector_free(node->forwards_list);
node->forwards_list = NULL;
}
if (node->exclusions_count > 0 && node->exclusions_list) {
for (size_t j = 0; j < node->exclusions_count; j++) {
exclusions *e = node->exclusions_list[j];
if (e->topic) {
free(e->topic);
e->topic = NULL;
}
NNI_FREE_STRUCT(e);
}
node->exclusions_count = 0;
cvector_free(node->exclusions_list);
node->exclusions_list = NULL;
}
if (node->sub_count > 0 && node->sub_list) {
for (size_t i = 0; i < node->sub_count; i++) {
topics *s = node->sub_list[i];
Expand Down Expand Up @@ -3702,7 +3715,7 @@ print_bridge_conf(conf_bridge *bridge, const char *prefix)
node->name, node->resend_wait);
log_info("%sbridge.mqtt.%s.cancel_timeout: %ld", prefix,
node->name, node->cancel_timeout);
log_info("%sbridge.mqtt.%s.hybrid_bridging : %s", prefix,
log_info("%sbridge.mqtt.%s.hybrid_bridging: %s", prefix,
node->name, node->hybrid ? "true" : "false");
log_info("%sbridge.mqtt.%s.hybrid_servers: ", prefix, node->name);
for (size_t j = 0; j < cvector_size(node->hybrid_servers); j++) {
Expand Down Expand Up @@ -3746,14 +3759,21 @@ print_bridge_conf(conf_bridge *bridge, const char *prefix)

for (size_t j = 0; j < node->forwards_count; j++) {
log_info(
"\t[%ld] remote topic: %.*s", j,
"\t[%ld] remote topic:\t\t%.*s", j,
node->forwards_list[j]->remote_topic_len,
node->forwards_list[j]->remote_topic);
log_info(
"\t[%ld] local topic: %.*s", j,
"\t[%ld] local topic:\t\t%.*s", j,
node->forwards_list[j]->local_topic_len,
node->forwards_list[j]->local_topic);
}
log_info("%sbridge.mqtt.%s.exclusions: ", prefix, node->name);
for (size_t j = 0; j < node->exclusions_count; j++) {
log_info(
"\t[%ld] topic:\t\t%.*s", j,
node->exclusions_list[j]->topic_len,
node->exclusions_list[j]->topic);
}
Comment on lines +3770 to +3776
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential format string issue: %.*s precision expects int, but topic_len is size_t.

The %.*s format specifier expects an int for the precision argument, but topic_len is size_t. On 64-bit platforms where size_t is 64-bit, this could cause undefined behavior or incorrect output. The same issue exists for the loop index j being size_t but used with %ld.

🛡️ Proposed fix to cast the precision argument
 		log_info("%sbridge.mqtt.%s.exclusions: ", prefix, node->name);
 		for (size_t j = 0; j < node->exclusions_count; j++) {
 			log_info(
-				"\t[%ld] topic:\t\t%.*s", j, 
-				node->exclusions_list[j]->topic_len,
+				"\t[%zu] topic:\t\t%.*s", j, 
+				(int)node->exclusions_list[j]->topic_len,
 				node->exclusions_list[j]->topic);
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/supplemental/nanolib/conf.c` around lines 3770 - 3776, The format string
is unsafe: %.*s expects an int precision and %ld expects a long, but topic_len
is size_t and j is size_t; update the logging call in the loop that uses
node->exclusions_count, node->exclusions_list and
node->exclusions_list[j]->topic_len so the types match the format
specifiers—cast topic_len to int for the precision argument (e.g.,
(int)node->exclusions_list[j]->topic_len) and either switch %ld to %zu for j or
cast j to long (prefer %zu to match size_t); adjust the log_info invocation
accordingly.

log_info(
"%sbridge.mqtt.%s.subscription: ", prefix, node->name);
for (size_t k = 0; k < node->sub_count; k++) {
Expand Down
18 changes: 17 additions & 1 deletion src/supplemental/nanolib/conf_ver2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,23 @@ conf_bridge_node_parse(
}
node->forwards_count = cvector_size(node->forwards_list);

cJSON *exclusions_list = hocon_get_obj("exclusions", obj);
cJSON *exclusion = NULL;
cJSON_ArrayForEach(exclusion, exclusions_list)
{
exclusions *exc = NNI_ALLOC_STRUCT(exc);
hocon_read_str(exc, topic, exclusion);
size_t topic_len = exc->topic ? strlen(exc->topic) : 0;
if (topic_len == 0) {
log_error("Exclusion topic should not be null or empty");
NNI_FREE_STRUCT(exc);
break;
}
exc->topic_len = topic_len;
cvector_push_back(node->exclusions_list, exc);
}
node->exclusions_count = cvector_size(node->exclusions_list);
Comment on lines +1267 to +1282
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Using break on validation failure skips all remaining exclusions.

When an exclusion has an empty or null topic, the code breaks out of the loop entirely, discarding any subsequent valid exclusions. Consider using continue instead to skip only the invalid entry while still processing the rest.

Additionally, unlike the similar forwards/subscription parsing, there's no maximum topic length validation here. Extremely long topic strings could cause issues.

🐛 Proposed fix
 	cJSON *exclusions_list = hocon_get_obj("exclusions", obj);
 	cJSON *exclusion = NULL;
 	cJSON_ArrayForEach(exclusion, exclusions_list)
 	{
 		exclusions *exc = NNI_ALLOC_STRUCT(exc);
 		hocon_read_str(exc, topic, exclusion);
 		size_t topic_len = exc->topic ? strlen(exc->topic) : 0;
-		if (topic_len == 0) {
-			log_error("Exclusion topic should not be null or empty");
-			NNI_FREE_STRUCT(exc); 
+		if (topic_len == 0 || topic_len > UINT16_MAX) {
+			log_warn("Invalid exclusion topic length: %zu, skipping", topic_len);
+			if (exc->topic) {
+				nng_strfree(exc->topic);
+			}
+			NNI_FREE_STRUCT(exc);
-			break;
+			continue;
 		}
 		exc->topic_len = topic_len;
 		cvector_push_back(node->exclusions_list, exc);
 	}
 	node->exclusions_count = cvector_size(node->exclusions_list);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cJSON *exclusions_list = hocon_get_obj("exclusions", obj);
cJSON *exclusion = NULL;
cJSON_ArrayForEach(exclusion, exclusions_list)
{
exclusions *exc = NNI_ALLOC_STRUCT(exc);
hocon_read_str(exc, topic, exclusion);
size_t topic_len = exc->topic ? strlen(exc->topic) : 0;
if (topic_len == 0) {
log_error("Exclusion topic should not be null or empty");
NNI_FREE_STRUCT(exc);
break;
}
exc->topic_len = topic_len;
cvector_push_back(node->exclusions_list, exc);
}
node->exclusions_count = cvector_size(node->exclusions_list);
cJSON *exclusions_list = hocon_get_obj("exclusions", obj);
cJSON *exclusion = NULL;
cJSON_ArrayForEach(exclusion, exclusions_list)
{
exclusions *exc = NNI_ALLOC_STRUCT(exc);
hocon_read_str(exc, topic, exclusion);
size_t topic_len = exc->topic ? strlen(exc->topic) : 0;
if (topic_len == 0 || topic_len > UINT16_MAX) {
log_warn("Invalid exclusion topic length: %zu, skipping", topic_len);
if (exc->topic) {
nng_strfree(exc->topic);
}
NNI_FREE_STRUCT(exc);
continue;
}
exc->topic_len = topic_len;
cvector_push_back(node->exclusions_list, exc);
}
node->exclusions_count = cvector_size(node->exclusions_list);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/supplemental/nanolib/conf_ver2.c` around lines 1267 - 1282, The loop over
exclusions_list currently breaks on an invalid empty/null topic which aborts
processing remaining exclusions; change the behavior in the
cJSON_ArrayForEach(exclusion, exclusions_list) loop so that after
hocon_read_str(exc, topic, exclusion) you validate topic_len and on invalid
topic call log_error, free the allocated struct (NNI_FREE_STRUCT(exc)) and
continue (not break) to skip only that entry; additionally add the same
maximum-topic-length check used in forwards/subscription parsing (compare
topic_len against the established MAX_TOPIC_LEN constant or the same validation
routine), free exc and continue if it exceeds the max, otherwise set
exc->topic_len and cvector_push_back(node->exclusions_list, exc); finally ensure
node->exclusions_count = cvector_size(node->exclusions_list) remains after the
loop.


cJSON *subscriptions = hocon_get_obj("subscription", obj);

cJSON *subscription = NULL;
Expand Down Expand Up @@ -1919,7 +1936,6 @@ conf_authorization_prase_ver2(conf *config, cJSON *jso)
conf_auth_http_parse_ver2(config, jso_auth_http);
}
cJSON *jso_auth_pwd = hocon_get_obj("auth.password", jso);

if (jso_auth_pwd) {
conf_auth_parse_ver2(config, jso_auth_pwd);
}
Expand Down
Loading