Skip to content

fix(mqtt): advertise Maximum Packet Size in the embedded CONNECT - #284

Merged
lxsaah merged 2 commits into
mainfrom
fix/mqtt-max-packet-size
Oct 4, 2026
Merged

lxsaah merged 2 commits into
mainfrom
fix/mqtt-max-packet-size

Conversation

@lxsaah

@lxsaah lxsaah commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

The embedded backend receives packets of up to 3,328 bytes in every case: its 3,584-byte reassembly buffer minus one 256-byte read. Its MQTT 5 CONNECT never told the broker that. A broker could therefore send a larger packet, which ends the session with PacketTooLargeForBuffer. A retained message that large is replayed after every SUBSCRIBE, so the client reconnected forever, once per reconnection delay.

The issue predates design 054 and is independent of it, so it goes to main directly. It was found in the review of #283, which also added a proof of the reconnect loop on the feature branch.

Change

  • session_loop.rs:
    • New MAX_INBOUND_PACKET = PACKET_BUFFER_SIZE − RX_CHUNK (3,328).
    • The CONNECT carries MaximumPacketSize(3328) beside TopicAliasMaximum(0) (Connect<'_, 2, 0>).
    • MQTT 5 §3.1.2.11.4: the broker must not send the client a larger packet, and a retained message over the limit is not delivered.
  • tests/common (the fake broker):
    • Records the Maximum Packet Size each MQTT 5 CONNECT advertises.
    • Withholds a push larger than it and counts it, as a compliant broker must.
    • 3.1.1 clients (the native backend) are unaffected.

Tests

  • the_advertised_maximum_packet_size_is_always_received (unit):
    • A 3,328-byte packet is received at every alignment (offsets 0–255 within a read), even when the read that completes it also carries the head of the next packet.
    • 3,330 bytes fails at some offset. The reader's documented limit is conservative by one byte; 3,329 always fits, which I checked separately.
  • a_retained_message_over_the_maximum_packet_size_is_withheld (tokio_broker, fake broker):
    • The CONNECT advertises 3,328.
    • A 3,000-byte retained message arrives.
    • A 4,000-byte one is withheld, and the client stays on one connection for 3 s. Before this change it reconnected every 2 s.

Verification

  • session_loop 6, tokio_broker 4, embassy_broker 1, tls_session 3, tls_broker 3, backend_parity 8, --features std 40, embedded-tls lib 30: all pass.
  • Every aimdb-mqtt-connector clippy leg from the Makefile (host, test targets, thumbv7em) and cargo fmt --all --check: clean.
  • embassy-mqtt-connector-demo builds for thumbv8m.main-none-eabihf.

Note for the 054 feature branch

Merging main into feat/054-connector-boundary will conflict in the CONNECT block of session_loop.rs: queue(outbound, encode(&connect)?) here against ring.put(&connect, 0) there. Keep the ring call and the new property. After that merge, proof_a_4000_byte_retained_message_reconnects_forever from #283 flips to this PR's behaviour.

🤖 Generated with Claude Code

lxsaah and others added 2 commits October 4, 2026 18:56
The embedded session receives packets of up to 3,328 bytes in every case
(its 3,584-byte buffer minus one read), but its CONNECT never said so. A
broker could send a larger packet, which ends the session; a retained one
ends every session it is replayed into, so the client reconnected forever.

CONNECT now carries MaximumPacketSize = 3,328, and a broker withholds
anything larger. The fake test broker records the property and withholds
oversized pushes as a compliant broker must.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lxsaah
lxsaah merged commit 51ca2d1 into main Oct 4, 2026
7 checks passed
@lxsaah
lxsaah deleted the fix/mqtt-max-packet-size branch October 4, 2026 19:08
lxsaah added a commit that referenced this pull request Oct 6, 2026
Brings #284 (advertise Maximum Packet Size in the embedded CONNECT). The
branch already sends the same CONNECT properties through `connect_packet`;
the conflicts keep the branch's side.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant