Skip to content

Fix MQTT v5.0 "beyond size" error in PUBACK parsing#53

Merged
xHasKx merged 2 commits into
xHasKx:masterfrom
haukepribnow:v5ackfix-xhaskx
Mar 27, 2026
Merged

Fix MQTT v5.0 "beyond size" error in PUBACK parsing#53
xHasKx merged 2 commits into
xHasKx:masterfrom
haukepribnow:v5ackfix-xhaskx

Conversation

@haukepribnow

Copy link
Copy Markdown
Contributor

This PR addresses a protocol parsing error in the MQTT v5.0 implementation where certain PUBACK packets would trigger a "beyond size" error and a subsequent connection drop.

The issue was identified during a late-night debugging session involving my MQTT broker that omit the Optional Properties section in 3-byte packets (Packet ID + Reason Code).

Technical Context

Per MQTT v5.0 Specification (Section 3.4.2), the Variable Header for PUBACK is flexible in length:

  • Length 2: Packet Identifier only.
  • Length 3: Packet Identifier + Reason Code.
  • Length 4+: Packet Identifier + Reason Code + Property Length indicator + Properties.

Specifically, Section 3.4.2.2.1 (Property Length) states:

The length of the Properties in the PUBACK packet Variable Header encoded as a Variable Byte Integer. If the Remaining Length is less than 4 there is no Property Length and the value of 0 is used.

The existing logic correctly checked input.available > 0 before parsing the Reason Code. However, it failed to perform a subsequent check before calling parse_properties. If a broker sends exactly 3 bytes, the parser attempts to read a "Property Length" byte that does not exist according to the spec, resulting in a fatal buffer error.

Scope of Changes

  • What this covers: This adds a nested input.available > 0 check in parse_packet_puback to ensure data exists for the Properties section before attempting to read the Property Length.
  • What this does not cover: This does not cover other packet types. This "Optional Reason Code + Optional Properties" pattern also exists in pubrec, pubrel, pubcomp, and unsuback. Due to time constraints, I am only submitting the fix for puback at this time as it was the specific blocker for my environment. I likely will not have the bandwidth to address the others in the near future; I invite any interested contributors to apply this same logic to the remaining functions in protocol5.lua.

Repository & History Strategy

I am raising this PR in both xHasKx/luamqtt and Tieske/luamqtt.

To maintain structural integrity across repository boundaries:

  • The patch was applied to a commit derived from the common fork point (determined via git merge-base).
  • This commit was then merged into the respective "live" branches of both repositories.
  • This ensures that if the forks are ever merged back into a single upstream, the history remains clean and the changes integrate without conflict.

Cross-Reference: I will add a link to the corresponding PR in the comments once both have been successfully created.

@haukepribnow

Copy link
Copy Markdown
Contributor Author

Here's a link to the corresponding PR in the other repository: Tieske#8

@xHasKx

xHasKx commented Mar 27, 2026

Copy link
Copy Markdown
Owner

Thanks @haukepribnow
I will take a look in a few days

@xHasKx xHasKx merged commit 193d414 into xHasKx:master Mar 27, 2026
2 checks passed
@xHasKx

xHasKx commented Mar 27, 2026

Copy link
Copy Markdown
Owner

@haukepribnow , I've merged your pull request. Thanks!
Do you use this packet from luarocks?
I plan to add few more commits later so there will be no version publication there for a few days until I finish.

@Tieske

Tieske commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Do you use this packet from luarocks? I plan to add few more commits later so there will be no version publication there for a few days until I finish.

FYI; You can use luarocks. Use luarocks install luamqtt --dev which will use the SCM rockspec, and build from the master. Until a new release is up.

@Tieske

Tieske commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

@xHasKx here are the remaining fixes; Tieske#10 you should be able to cherry-pick them.

@Tieske

Tieske commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

@haukepribnow LuaMQTTT fix is up on LuaRocks; t1.0.4 (in case you use that fork)

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.

3 participants