Commit 163563e16cb2f8701a7ec6920cf65ae4cfcc6649
1 parent
c652de5a
Add some assert+doc about how packets are used
Also fixed a length check bug.
Showing
2 changed files
with
16 additions
and
1 deletions
mqttpacket.cpp
| ... | ... | @@ -44,6 +44,7 @@ MqttPacket::MqttPacket(CirBuf &buf, size_t packet_len, size_t fixed_header_lengt |
| 44 | 44 | unsigned char _packetType = (first_byte & 0xF0) >> 4; |
| 45 | 45 | packetType = (PacketType)_packetType; |
| 46 | 46 | pos += fixed_header_length; |
| 47 | + externallyReceived = true; | |
| 47 | 48 | } |
| 48 | 49 | |
| 49 | 50 | MqttPacket::MqttPacket(const ConnAck &connAck) : |
| ... | ... | @@ -123,7 +124,7 @@ size_t MqttPacket::getRequiredSizeForPublish(const ProtocolVersion protocolVersi |
| 123 | 124 | MqttPacket::MqttPacket(const ProtocolVersion protocolVersion, const Publish &_publish) : |
| 124 | 125 | bites(getRequiredSizeForPublish(protocolVersion, _publish)) |
| 125 | 126 | { |
| 126 | - if (publishData.topic.length() > 0xFFFF) | |
| 127 | + if (_publish.topic.length() > 0xFFFF) | |
| 127 | 128 | { |
| 128 | 129 | throw ProtocolError("Topic path too long."); |
| 129 | 130 | } |
| ... | ... | @@ -1301,6 +1302,7 @@ const Publish &MqttPacket::getPublishData() |
| 1301 | 1302 | bool MqttPacket::containsClientSpecificProperties() const |
| 1302 | 1303 | { |
| 1303 | 1304 | assert(packetType == PacketType::PUBLISH); |
| 1305 | + assert(this->externallyReceived); | |
| 1304 | 1306 | |
| 1305 | 1307 | if (protocolVersion <= ProtocolVersion::Mqtt311 || !publishData.propertyBuilder) |
| 1306 | 1308 | return false; | ... | ... |
mqttpacket.h
| ... | ... | @@ -36,6 +36,13 @@ License along with FlashMQ. If not, see <https://www.gnu.org/licenses/>. |
| 36 | 36 | #include "variablebyteint.h" |
| 37 | 37 | #include "mqtt5properties.h" |
| 38 | 38 | |
| 39 | +/** | |
| 40 | + * @brief The MqttPacket class represents incoming and outgonig packets. | |
| 41 | + * | |
| 42 | + * Be sure to understand the 'externallyReceived' member. See in-code documentation. | |
| 43 | + * | |
| 44 | + * TODO: I could perhaps make a subclass for the externally received one. | |
| 45 | + */ | |
| 39 | 46 | class MqttPacket |
| 40 | 47 | { |
| 41 | 48 | #ifdef TESTING |
| ... | ... | @@ -55,6 +62,12 @@ class MqttPacket |
| 55 | 62 | size_t payloadStart = 0; |
| 56 | 63 | size_t payloadLen = 0; |
| 57 | 64 | bool hasTopicAlias = false; |
| 65 | + | |
| 66 | + // It's important to understand that this class is used for incoming packets as well as new outgoing packets. When we create | |
| 67 | + // new outgoing packets, we generally know exactly who it's for and the information is only stored in this->bites. So, the | |
| 68 | + // publishData and fields like hasTopicAlias are invalid in those cases. | |
| 69 | + bool externallyReceived = false; | |
| 70 | + | |
| 58 | 71 | Logger *logger = Logger::getInstance(); |
| 59 | 72 | |
| 60 | 73 | char *readBytes(size_t length); | ... | ... |