Skip to content

Broker: static retained delivery splices PUBLISH frames after a short write #628

Open

Description

@Marti2203

In WOLFMQTT_STATIC_MEMORY builds, retained delivery can splice two PUBLISH frames on the wire when a write to the subscriber stops part way.

Problem

BrokerRetained_DeliverToClient (static branch) encodes every matching retained message into bc->tx_buf and calls BrokerClient_WriteDirect. If the write stops after sending some bytes (MQTT_CODE_CONTINUE, or MQTT_CODE_ERROR_TIMEOUT when the select() in BrokerPosix_Write times out), MqttSocket_Write leaves client.write.pos at the number of bytes sent. The loop continues: the next retained PUBLISH is encoded into the same tx_buf, and its write resumes at write.pos inside the new frame. The subscriber reads the fixed header and first bytes of message A followed by the tail of message B. If the payload lengths differ, it loses framing for the rest of the connection. Nothing is logged.

Dynamic builds avoid this with direct_pending_len and the queued drain. The other static write paths (BrokerHandle_Publish fan-out, BrokerStaticOrphan_Drain) already close the connection when a write does not complete. The retained loop was the remaining path that kept going.

Reproduction

Unmodified broker built with --enable-broker --disable-tls CFLAGS=-DWOLFMQTT_STATIC_MEMORY, accepted sockets given SO_SNDBUF=4096 (via LD_PRELOAD, standing in for a small embedded TCP stack). A publisher stores 16 retained 3500-byte messages. A subscriber subscribes with 3 filters (48 deliveries) and pauses before reading:

pause payload sizes well-formed / malformed frames
none equal or varied 48 / 0
1.5 s equal 46 / 1 (header says t/0, payload is t/0 bytes followed by t/4 bytes)
1.5 s varied 4 / 1607 (stream desynchronised)

A 1.5 s pause is just over BROKER_TIMEOUT_MS. The dynamic build delivers all 48 with no pause. The results were the same over three runs.

Suggested fix

When a retained write did not complete and part of the frame is on the wire (client.write.pos != 0), release the packet identifier, close the transport with BrokerStaticClient_Close (as the queue-full branch of the same loop does), stop the loop and return MQTT_CODE_ERROR_NETWORK. A write that fails before any byte is sent keeps the current behaviour. A patch with tests is in #627.

This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions