Apply isDup flag in MQTT5 buildMqttPubMessage (three builder paths) — fixes #283 - #284
Merged
Merged
Conversation
…ths) Re-delivered PUBLISH packets were built with DUP=0, causing standards-compliant clients (e.g. HiveMQ MQTT client) to treat the re-delivery as a protocol error and disconnect — message loss in server-side bridge scenarios. Per MQTT-3.3.1-1, the DUP flag MUST be set on re-delivery. The isDup parameter was accepted but never applied in any of the three builder paths (topic-alias first-use / topic-alias reuse / no-alias). Fixes apache#283 Signed-off-by: DanXie <517964478@qq.com>
Adds qos1RedeliveryCarriesDupFlag to TransientSessionHandlerTest: publish one QoS1 message, keep it in flight, advance past ResendTimeoutSeconds, and assert the re-delivered PUBLISH carries DUP=1 per [MQTT-3.3.1-1] while going through the topic-alias reuse builder path. The handler is rebuilt with stubbed settings because TenantSettings reads its values once at construction. Verified against the unfixed MQTT5ProtocolHelper the new test fails with 'DUP=1 expected true but found false'; with the fix the full TransientSessionHandlerTest (52 tests) passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-delivered PUBLISH packets were built with DUP=0 — the
isDupparameter ofMQTT5ProtocolHelper#buildMqttPubMessagewas accepted but never applied in any of its three builder paths (topic-alias first-use, topic-alias reuse, no-alias).Per MQTT-3.3.1-1 ("The DUP flag MUST be set to 1 by the Client or Server when it attempts to re-deliver a PUBLISH packet"), re-delivery must carry
DUP=1. Standards-compliant clients (e.g. the HiveMQ MQTT client) currently treat the re-delivery as a protocol error and disconnect — message loss in server-side bridge scenarios.Full reproduction path and source-level analysis: #283 (opened as requested by @popduke).
Change
.dup(isDup)in all threeMQTT5MessageBuilders.pub()paths;Testing
mvn -pl bifromq-mqtt/bifromq-mqtt-server -am compilepasses;Fixes #283