From b299cc2427370055813a0d09d254586fc4471b95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 21 Jul 2026 09:08:46 +0200 Subject: [PATCH] Honour which_payload_variant on MQTT client-proxy ingress (#11097) --- src/mqtt/MQTT.cpp | 15 ++++++++++++++- test/test_mqtt/MQTT.cpp | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 3d4a20ed7..ddd39148f 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -337,7 +337,20 @@ void MQTT::mqttCallback(char *topic, byte *payload, unsigned int length) void MQTT::onClientProxyReceive(meshtastic_MqttClientProxyMessage msg) { - onReceive(msg.topic, msg.payload_variant.data.bytes, msg.payload_variant.data.size); + // payload_variant is a union: on the text variant, data.size aliases the first bytes of the + // string, so reading it unconditionally let a client name any length up to PB_SIZE_MAX. + switch (msg.which_payload_variant) { + case meshtastic_MqttClientProxyMessage_data_tag: + onReceive(msg.topic, msg.payload_variant.data.bytes, msg.payload_variant.data.size); + break; + case meshtastic_MqttClientProxyMessage_text_tag: + onReceive(msg.topic, (byte *)msg.payload_variant.text, + strnlen(msg.payload_variant.text, sizeof(msg.payload_variant.text))); + break; + default: + LOG_WARN("MQTT proxy message carries no payload, topic %s", msg.topic); + break; + } } void MQTT::onReceive(char *topic, byte *payload, size_t length) diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 3716e0a20..c273394f7 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -583,6 +583,37 @@ void test_receiveEmptyDataFromProxy(void) TEST_ASSERT_TRUE(mockRouter->packets_.empty()); } +// Text must be read as text: data.size aliases the string's first bytes, so reading it regardless +// of the variant let a client name a length of up to PB_SIZE_MAX. There is no delivery control for +// this variant: an encoded ServiceEnvelope always contains NUL, so text can never carry one. +void test_receiveTextVariantFromProxyIsNotReadAsBytes(void) +{ + meshtastic_MqttClientProxyMessage message = meshtastic_MqttClientProxyMessage_init_default; + snprintf(message.topic, sizeof(message.topic), "msh/2/e/test/!87654321"); + message.which_payload_variant = meshtastic_MqttClientProxyMessage_text_tag; + // data.size would read these as the largest length a pb_size_t can name. + memset(message.payload_variant.text, 0xFF, sizeof(message.payload_variant.text) - 1); + message.payload_variant.text[sizeof(message.payload_variant.text) - 1] = '\0'; + + mqtt->onClientProxyReceive(message); + + TEST_ASSERT_TRUE(mockRouter->packets_.empty()); +} + +// A proxy message with no payload variant set must be ignored rather than read as bytes. +void test_receiveNoVariantFromProxyIsIgnored(void) +{ + meshtastic_MqttClientProxyMessage message = meshtastic_MqttClientProxyMessage_init_default; + snprintf(message.topic, sizeof(message.topic), "msh/2/e/test/!87654321"); + message.which_payload_variant = 0; + memset(message.payload_variant.data.bytes, 0xFF, sizeof(message.payload_variant.data.bytes)); + message.payload_variant.data.size = sizeof(message.payload_variant.data.bytes); + + mqtt->onClientProxyReceive(message); + + TEST_ASSERT_TRUE(mockRouter->packets_.empty()); +} + // Packets should be ignored if downlink is not enabled. void test_receiveWithoutChannelDownlink(void) { @@ -1051,6 +1082,8 @@ void setup() RUN_TEST(test_receiveDecodedProto); RUN_TEST(test_receiveDecodedProtoFromProxy); RUN_TEST(test_receiveEmptyDataFromProxy); + RUN_TEST(test_receiveTextVariantFromProxyIsNotReadAsBytes); + RUN_TEST(test_receiveNoVariantFromProxyIsIgnored); RUN_TEST(test_receiveWithoutChannelDownlink); RUN_TEST(test_receiveEncryptedPKITopicToUs); RUN_TEST(test_receiveIgnoresOwnPublishedMessages);