From d6fa19002523cc5849d608fcfeb74159b235c9d8 Mon Sep 17 00:00:00 2001 From: Andre K Date: Thu, 25 Jan 2024 11:42:34 -0300 Subject: [PATCH] fix: allow MQTT `encryption_enabled` with `json_enabled` (#3126) * fix: allow MQTT `encryption_enabled` with `json_enabled` * fix: copy decoded MeshPacket and release memory after use * fix: use `packetPool` allocCopy and release methods --------- Co-authored-by: Ben Meadors --- src/mesh/Router.cpp | 19 ++++++------------- src/mqtt/MQTT.cpp | 12 +++++++++--- src/mqtt/MQTT.h | 4 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 977a1215a..492ed962b 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -248,28 +248,21 @@ ErrorCode Router::send(meshtastic_MeshPacket *p) // If the packet is not yet encrypted, do so now if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { ChannelIndex chIndex = p->channel; // keep as a local because we are about to change it - - if (moduleConfig.mqtt.enabled) { - - LOG_INFO("Should encrypt MQTT?: %d\n", moduleConfig.mqtt.encryption_enabled); - - // the packet is currently in a decrypted state. send it now if they want decrypted packets - if (mqtt && !moduleConfig.mqtt.encryption_enabled) - mqtt->onSend(*p, chIndex); - } + meshtastic_MeshPacket *p_decoded = packetPool.allocCopy(*p); auto encodeResult = perhapsEncode(p); if (encodeResult != meshtastic_Routing_Error_NONE) { + packetPool.release(p_decoded); abortSendAndNak(encodeResult, p); return encodeResult; // FIXME - this isn't a valid ErrorCode } if (moduleConfig.mqtt.enabled) { - // the packet is now encrypted. - // check if we should send encrypted packets to mqtt - if (mqtt && moduleConfig.mqtt.encryption_enabled) - mqtt->onSend(*p, chIndex); + LOG_INFO("Should encrypt MQTT?: %d\n", moduleConfig.mqtt.encryption_enabled); + if (mqtt) + mqtt->onSend(*p, *p_decoded, chIndex); } + packetPool.release(p_decoded); } assert(iface); // This should have been detected already in sendLocal (or we just received a packet from outside) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index d2d3f3b21..87dacde7a 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -466,7 +466,7 @@ void MQTT::publishQueuedMessages() } } -void MQTT::onSend(const meshtastic_MeshPacket &mp, ChannelIndex chIndex) +void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex) { if (mp.via_mqtt) return; // Don't send messages that came from MQTT back into MQTT @@ -486,7 +486,13 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, ChannelIndex chIndex) meshtastic_ServiceEnvelope *env = mqttPool.allocZeroed(); env->channel_id = (char *)channelId; env->gateway_id = owner.id; - env->packet = (meshtastic_MeshPacket *)∓ + + if (moduleConfig.mqtt.encryption_enabled) { + env->packet = (meshtastic_MeshPacket *)∓ + } else { + env->packet = (meshtastic_MeshPacket *)&mp_decoded; + } + LOG_DEBUG("MQTT onSend - Publishing portnum %i message\n", env->packet->decoded.portnum); if (moduleConfig.mqtt.proxy_to_client_enabled || this->isConnectedDirectly()) { @@ -501,7 +507,7 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, ChannelIndex chIndex) if (moduleConfig.mqtt.json_enabled) { // handle json topic - auto jsonString = this->meshPacketToJson((meshtastic_MeshPacket *)&mp); + auto jsonString = this->meshPacketToJson((meshtastic_MeshPacket *)&mp_decoded); if (jsonString.length() != 0) { std::string topicJson = jsonTopic + channelId + "/" + owner.id; LOG_INFO("JSON publish message to %s, %u bytes: %s\n", topicJson.c_str(), jsonString.length(), diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h index 565f46ecf..fc9f9d454 100644 --- a/src/mqtt/MQTT.h +++ b/src/mqtt/MQTT.h @@ -48,14 +48,14 @@ class MQTT : private concurrency::OSThread MQTT(); /** - * Publish a packet on the glboal MQTT server. + * Publish a packet on the global MQTT server. * This hook must be called **after** the packet is encrypted (including the channel being changed to a hash). * @param chIndex the index of the channel for this message * * Note: for messages we are forwarding on the mesh that we can't find the channel for (because we don't have the keys), we * can not forward those messages to the cloud - because no way to find a global channel ID. */ - void onSend(const meshtastic_MeshPacket &mp, ChannelIndex chIndex); + void onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket &mp_decoded, ChannelIndex chIndex); /** Attempt to connect to server if necessary */