Skip to content

Commit 25834ff

Browse files
committed
[nrf fromtree] net: mqtt: Improve disconnect error notification for MQTT 5.0
As MQTT 5.0 allows to specify the disconnect reason in the Disconnect packet, use this new feature to improve error notification to the broker, according to the error guidelines in the MQTT 5.0 spec. For most cases, a generic arbitrary mapping between errno values and reason codes is used, however the parser can specify the disconnect reason code manually to better handle certain corner cases (like invalid topic alias used). Signed-off-by: Robert Lubos <[email protected]> (cherry picked from commit 8935579)
1 parent a3cdc17 commit 25834ff

File tree

4 files changed

+101
-17
lines changed

4 files changed

+101
-17
lines changed

include/zephyr/net/mqtt.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,9 @@ struct mqtt_internal {
879879
#if defined(CONFIG_MQTT_VERSION_5_0)
880880
/** Internal. MQTT 5.0 topic alias mapping. */
881881
struct mqtt_topic_alias topic_aliases[CONFIG_MQTT_TOPIC_ALIAS_MAX];
882+
883+
/** Internal. MQTT 5.0 disconnect reason set in case of processing errors. */
884+
enum mqtt_disconnect_reason_code disconnect_reason;
882885
#endif /* CONFIG_MQTT_VERSION_5_0 */
883886
};
884887

subsys/net/lib/mqtt/mqtt.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,58 @@ static int client_connect(struct mqtt_client *client)
108108
return err_code;
109109
}
110110

111+
static int client_write(struct mqtt_client *client, const uint8_t *data,
112+
uint32_t datalen);
113+
114+
#if defined(CONFIG_MQTT_VERSION_5_0)
115+
static void disconnect_5_0_notify(struct mqtt_client *client, int err)
116+
{
117+
struct mqtt_disconnect_param param = { };
118+
struct buf_ctx packet;
119+
120+
/* Parser might've set custom failure reason, in such case skip generic
121+
* mapping from errno to reason code.
122+
*/
123+
if (client->internal.disconnect_reason != MQTT_DISCONNECT_NORMAL) {
124+
param.reason_code = client->internal.disconnect_reason;
125+
} else {
126+
switch (err) {
127+
case ECONNREFUSED:
128+
case ENOTCONN:
129+
/* Connection rejected/closed, skip disconnect. */
130+
return;
131+
case EINVAL:
132+
param.reason_code = MQTT_DISCONNECT_MALFORMED_PACKET;
133+
break;
134+
case EBADMSG:
135+
param.reason_code = MQTT_DISCONNECT_PROTOCOL_ERROR;
136+
break;
137+
case ENOMEM:
138+
param.reason_code = MQTT_DISCONNECT_PACKET_TOO_LARGE;
139+
break;
140+
default:
141+
param.reason_code = MQTT_DISCONNECT_UNSPECIFIED_ERROR;
142+
break;
143+
}
144+
}
145+
146+
tx_buf_init(client, &packet);
147+
148+
if (disconnect_encode(client, &param, &packet) < 0) {
149+
return;
150+
}
151+
152+
(void)client_write(client, packet.cur, packet.end - packet.cur);
153+
}
154+
#else
155+
static void disconnect_5_0_notify(struct mqtt_client *client, int err)
156+
{
157+
ARG_UNUSED(client);
158+
ARG_UNUSED(err);
159+
}
160+
#endif /* CONFIG_MQTT_VERSION_5_0 */
161+
162+
111163
static int client_read(struct mqtt_client *client)
112164
{
113165
int err_code;
@@ -118,6 +170,11 @@ static int client_read(struct mqtt_client *client)
118170

119171
err_code = mqtt_handle_rx(client);
120172
if (err_code < 0) {
173+
if (mqtt_is_version_5_0(client)) {
174+
/* Best effort send, if it fails just shut the connection. */
175+
disconnect_5_0_notify(client, -err_code);
176+
}
177+
121178
mqtt_client_disconnect(client, err_code, true);
122179
}
123180

subsys/net/lib/mqtt/mqtt_decoder.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,11 @@ int decode_uint32_property(struct property_decoder *prop,
289289
uint32_t *value = prop->data;
290290

291291
if (*remaining_len < sizeof(uint32_t)) {
292-
return -EBADMSG;
292+
return -EINVAL;
293293
}
294294

295295
if (unpack_uint32(buf, value) < 0) {
296-
return -EBADMSG;
296+
return -EINVAL;
297297
}
298298

299299
*remaining_len -= sizeof(uint32_t);
@@ -309,11 +309,11 @@ int decode_uint16_property(struct property_decoder *prop,
309309
uint16_t *value = prop->data;
310310

311311
if (*remaining_len < sizeof(uint16_t)) {
312-
return -EBADMSG;
312+
return -EINVAL;
313313
}
314314

315315
if (unpack_uint16(buf, value) < 0) {
316-
return -EBADMSG;
316+
return -EINVAL;
317317
}
318318

319319
*remaining_len -= sizeof(uint16_t);
@@ -329,11 +329,11 @@ int decode_uint8_property(struct property_decoder *prop,
329329
uint8_t *value = prop->data;
330330

331331
if (*remaining_len < sizeof(uint8_t)) {
332-
return -EBADMSG;
332+
return -EINVAL;
333333
}
334334

335335
if (unpack_uint8(buf, value) < 0) {
336-
return -EBADMSG;
336+
return -EINVAL;
337337
}
338338

339339
*remaining_len -= sizeof(uint8_t);
@@ -349,11 +349,11 @@ int decode_string_property(struct property_decoder *prop,
349349
struct mqtt_utf8 *str = prop->data;
350350

351351
if (unpack_utf8_str(buf, str) < 0) {
352-
return -EBADMSG;
352+
return -EINVAL;
353353
}
354354

355355
if (*remaining_len < sizeof(uint16_t) + str->size) {
356-
return -EBADMSG;
356+
return -EINVAL;
357357
}
358358

359359
*remaining_len -= sizeof(uint16_t) + str->size;
@@ -369,11 +369,11 @@ int decode_binary_property(struct property_decoder *prop,
369369
struct mqtt_binstr *bin = prop->data;
370370

371371
if (unpack_binary_data(buf, bin) < 0) {
372-
return -EBADMSG;
372+
return -EINVAL;
373373
}
374374

375375
if (*remaining_len < sizeof(uint16_t) + bin->len) {
376-
return -EBADMSG;
376+
return -EINVAL;
377377
}
378378

379379
*remaining_len -= sizeof(uint16_t) + bin->len;
@@ -392,16 +392,16 @@ int decode_user_property(struct property_decoder *prop,
392392
size_t prop_len;
393393

394394
if (unpack_utf8_str(buf, &temp.name) < 0) {
395-
return -EBADMSG;
395+
return -EINVAL;
396396
}
397397

398398
if (unpack_utf8_str(buf, &temp.value) < 0) {
399-
return -EBADMSG;
399+
return -EINVAL;
400400
}
401401

402402
prop_len = (2 * sizeof(uint16_t)) + temp.name.size + temp.value.size;
403403
if (*remaining_len < prop_len) {
404-
return -EBADMSG;
404+
return -EINVAL;
405405
}
406406

407407
*remaining_len -= prop_len;
@@ -469,7 +469,7 @@ static int properties_decode(struct property_decoder *prop, uint8_t cnt,
469469

470470
bytes = unpack_variable_int(buf, &properties_len);
471471
if (bytes < 0) {
472-
return -EBADMSG;
472+
return -EINVAL;
473473
}
474474

475475
bytes += (int)properties_len;
@@ -481,7 +481,7 @@ static int properties_decode(struct property_decoder *prop, uint8_t cnt,
481481
/* Decode property type */
482482
err = unpack_uint8(buf, &type);
483483
if (err < 0) {
484-
return -EBADMSG;
484+
return -EINVAL;
485485
}
486486

487487
properties_len--;
@@ -552,7 +552,7 @@ static int properties_decode(struct property_decoder *prop, uint8_t cnt,
552552
}
553553

554554
if (err < 0) {
555-
return -EBADMSG;
555+
return err;
556556
}
557557
}
558558

@@ -759,6 +759,7 @@ static int publish_topic_alias_check(struct mqtt_client *client,
759759

760760
/* Topic alias must not exceed configured CONFIG_MQTT_TOPIC_ALIAS_MAX */
761761
if (alias > CONFIG_MQTT_TOPIC_ALIAS_MAX) {
762+
set_disconnect_reason(client, MQTT_DISCONNECT_TOPIC_ALIAS_INVALID);
762763
return -EBADMSG;
763764
}
764765

@@ -790,7 +791,8 @@ static int publish_topic_alias_check(struct mqtt_client *client,
790791
NET_ERR("Topic too long (%d bytes) to store, increase "
791792
"CONFIG_MQTT_TOPIC_ALIAS_STRING_MAX",
792793
topic_str->size);
793-
return -EBADMSG;
794+
set_disconnect_reason(client, MQTT_DISCONNECT_IMPL_SPECIFIC_ERROR);
795+
return -ENOMEM;
794796
}
795797

796798
memcpy(topic_alias->topic_buf, topic_str->utf8, topic_str->size);

subsys/net/lib/mqtt/mqtt_internal.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,28 @@ int disconnect_decode(const struct mqtt_client *client, struct buf_ctx *buf,
496496
*/
497497
int auth_decode(const struct mqtt_client *client, struct buf_ctx *buf,
498498
struct mqtt_auth_param *param);
499+
500+
/**@brief Set MQTT 5.0 disconnect reason.
501+
*
502+
* Packet parser can use this function to set a custom disconnect reason code,
503+
* if not set, the client will use the default mapping between errno values and
504+
* reason codes.
505+
*
506+
* @param[inout] MQTT client.
507+
* @param[in] reason MQTT 5.0 disconnect reason code.
508+
*/
509+
static inline void set_disconnect_reason(struct mqtt_client *client,
510+
enum mqtt_disconnect_reason_code reason)
511+
{
512+
client->internal.disconnect_reason = reason;
513+
}
514+
#else
515+
static inline void set_disconnect_reason(struct mqtt_client *client,
516+
enum mqtt_disconnect_reason_code reason)
517+
{
518+
ARG_UNUSED(client);
519+
ARG_UNUSED(reason);
520+
}
499521
#endif /* CONFIG_MQTT_VERSION_5_0 */
500522

501523
/**

0 commit comments

Comments
 (0)