* MDF [conn_handler] add logging output while return protocol error

Signed-off-by: JaylinYu <letrangerjaylin@gmail.com>
nanomq · Nov 28, 2024 · 1abff3d · 1abff3d
1 parent f14f728
commit 1abff3d
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 9 deletions.
diff --git a/src/sp/protocol/mqtt/mqtt_parser.c b/src/sp/protocol/mqtt/mqtt_parser.c
@@ -584,15 +584,20 @@ conn_handler(uint8_t *packet, conn_param *cparam, size_t max)
 	cparam->will_qos    = (cparam->con_flag & 0x18) >> 3;
 	cparam->will_retain = (cparam->con_flag & 0x20) >> 5;
 	if (((cparam->con_flag & 0x01) != 0) ||
-	    (cparam->will_flag == 0 && cparam->will_retain != 0))
+	    (cparam->will_flag == 0 && cparam->will_retain != 0)) {
+		log_info("Malformed CONNECT: conn flag conflicts with will msg");
 		return PROTOCOL_ERROR;
+	}
 
 	log_trace("conn flag:%x", cparam->con_flag);
 	if ((cparam->will_flag == 1 && cparam->will_qos > 2) ||
 	    (strncmp(cparam->pro_name.body, MQTT_PROTOCOL_NAME, 4) != 0 &&
 	    strncmp(cparam->pro_name.body, MQTT_PROTOCOL_NAME_v31, 6) != 0) ||
-	    cparam->pro_ver > 5 || cparam->pro_ver < 3)
+	    cparam->pro_ver > 5 || cparam->pro_ver < 3) {
+		log_info("Malformed CONNECT: Invalid cparam %d %d %s ver %d",
+				 cparam->will_flag, cparam->will_qos, cparam->pro_name.body, cparam->pro_ver);
 		return PROTOCOL_ERROR;
+	}
 	pos++;
 	// keepalive
 	NNI_GET16(packet + pos, tmp);
@@ -603,12 +608,16 @@ conn_handler(uint8_t *packet, conn_param *cparam, size_t max)
 	if (cparam->pro_ver == MQTT_PROTOCOL_VERSION_v5) {
 		// check length
 		log_trace("Decoding MQTT V5 Properties");
-		if (pos + 3 > max)
+		if (pos + 3 > max) {
+			log_info("Malformed CONNECT: incomplete len");
 			return PROTOCOL_ERROR;
+		}
 		cparam->prop_len = (uint32_t) get_var_integer(packet + pos,
 													  &len_of_var);
-		if (cparam->prop_len > (max - pos - 1 - cparam->will_flag * 2 ))
+		if (cparam->prop_len > (max - pos - 1 - cparam->will_flag * 2 )) {
+			log_info("Malformed CONNECT: incomplete len");
 			return PROTOCOL_ERROR;
+		}
 		log_debug("remain len %d max len %d prop len %d pos %d",
 				   len, max, cparam->prop_len, pos);
 		cparam->properties = decode_buf_properties(
@@ -617,6 +626,7 @@ conn_handler(uint8_t *packet, conn_param *cparam, size_t max)
 			conn_param_set_property(cparam, cparam->properties);
 			if ((rv = check_properties(cparam->properties, NULL)) !=
 			    SUCCESS) {
+				log_info("Malformed CONNECT: property check failed");
 				return rv;
 			}
 		}
@@ -637,11 +647,11 @@ conn_handler(uint8_t *packet, conn_param *cparam, size_t max)
 		cparam->clientid.len  = strlen(clientid_r);
 		cparam->assignedid    = true;
 	} else if (len_of_str < 0) {
-		log_trace("PROTOCOL_ERROR: No clientid is found!");
+		log_info("Malformed CONNECT: incomplete utf8 in clientid");
 		return (PROTOCOL_ERROR);
 	} else if (cparam->pro_ver == MQTT_PROTOCOL_VERSION_v31 &&
 	    len_of_str > 23) {
-		log_trace("CLIENT_IDENTIFIER_NOT_VALID: clientid invalid!");
+		log_info("CLIENT_IDENTIFIER_NOT_VALID: clientid invalid!");
 		return (CLIENT_IDENTIFIER_NOT_VALID);
 	}
 	log_trace("clientid: [%s] [%d]", cparam->clientid.body, len_of_str);
@@ -665,8 +675,8 @@ conn_handler(uint8_t *packet, conn_param *cparam, size_t max)
 				conn_param_set_will_property(
 				    cparam, cparam->will_properties);
 				if ((rv = check_properties(
-				         cparam->will_properties, NULL)) !=
-				    SUCCESS) {
+						cparam->will_properties, NULL)) != SUCCESS) {
+					log_info("Malformed CONNECT: check will property failed");
 					return PROTOCOL_ERROR;
 				}
 			}

diff --git a/src/sp/transport/mqtt/broker_tcp.c b/src/sp/transport/mqtt/broker_tcp.c
@@ -411,7 +411,7 @@ tcptran_pipe_nego_cb(void *arg)
 		} else {
 			log_info("Disconnect Client due to %d parse CONNECT failed", rv);
 			nng_free(p->conn_buf, p->wantrxhead);
-			rv   = NNG_ENOMEM;
+			rv   = NNG_EPROTO;
 			code = MALFORMED_PACKET;
 			if (p->tcp_cparam->pro_ver == 5) {
 				goto close;