[embhorn]

F-15990: no double close of a rejected broker socket.
F-15405: v5 Will published for non-zero DISCONNECT reason codes.
F-15988: old v5 Will published on session takeover when required.
F-15376: subscriptions persisted when the SUBACK or UNSUBACK is deferred.
F-15989: inbound QoS 2 ID released when the PUBLISH topic copy fails.
F-15991: CONNECT Session Expiry kept when reclaiming an offline session.
F-14357: first payload chunk sent when the PUBLISH header fills tx_buf.
F-15377: zero-byte blocking socket write reported as a network error.
F-15378: MQTT-SN test added for a zero-byte automatic response write (covered by the 15377 fix).
F-13833: MQTT v5 Will Properties given their own property scope.
F-13835: broker skips PUBLISH writes whose payload was clamped to tx_buf.
F-13801: QoS 1/2 Wills queued for offline persistent subscribers.
F-15404: Message Expiry applied to the dynamic broker's queued deliveries.
F-13808: zero Receive Maximum rejected in the property encoder.
F-12452: reserved MQTT-SN TopicIdType rejected in encoders.
F-14609: AUTH without an Authentication Method rejected.
F-14585: client marked disconnected when msg_cb abandons a payload.
F-13834: v5 SUBACK and UNSUBACK without a Property Length rejected.
F-12464: v5 PUBLISH refused when the quota lock can't be taken.
F-12468: SN_Client_WillTopicUpdate can send the empty Will delete.
F-15375: only lowercase hex accepted in POSIX persist record names.
F-12450: property identifier checked against the Property Length.
F-12449: client flags read under lockClient in KeepAlive and WaitType.
F-13805: native semaphore init failures reported (POSIX, FreeRTOS, Windows).
F-14358: SN_Client_Connect state reset when the Will exchange fails.
F-15374: ack wait state reset after Subscribe, Unsubscribe and Publish.
F-16042: PUBREL replay recorded only for the client's own QoS 2 publishes.
F-15403: CONNACK 0x01 sent when an unsupported-level CONNECT won't decode.
F-14621: client IDs escaped in persistence restore log messages.
F-12451: restored OUTQ packet IDs checked against the record key.
F-11491: oversized POSIX persist records reported instead of truncated.
F-14195: v5 DISCONNECT reason sent before closing on broker errors.
F-13257: restored topic copies wiped when freed on malformed persist records.
F-13875: CONNECT credentials wiped when a refused CONNACK is deferred, plus a follow-up limiting that test to nonblocking builds.
F-13258: client tx and rx buffers wiped in MqttClient_DeInit.
F-14386: AES-GCM context wiped after persist record crypto.
F-14183: whole tx_buf wiped when a WILLMSG can't be encoded.
F-13803: POSIX persist keys longer than the filename limit rejected, and the limit documented.
F-13249: MQTT-SN QoS 2 resends delivered to the application only once, plus a follow-up dropping the test workaround.
F-14584: blocking wait fails the connection when it times out partway through a packet.
F-14610: other packets held back until an enhanced-auth CONNACK.
F-16041: new persistent sessions refused at the dynamic broker cap.
F-13806: MqttClient_NetDisconnect_Wait added and the race documented.
F-5117: Wait for an in-flight reader before unlinking a pending response

Documentation fixes
F-11473: WOLFMQTT_CODE_CONTINUE is now MQTT_CODE_CONTINUE in the build-options notes.
F-11474: MqttProps_Init/ShutDown (header and source) now say they are reference counted and must be called in pairs, not "called once".
F-11475: the SN PUBLISH comment now says a predefined topic is a 2-byte ID, not a name.
F-12463: the WOLFMQTT_MAX_QOS comment now lists what the macro actually gates, and the "mqtt_qos_max branch" reference is gone.
F-12466: the SUBSCRIBE decoder comment now says Retain Handling is applied, and No Local / Retain As Published are not stored.
F-12467: the SN_GwAddr comment now says it holds a fixed 2-octet address.
F-12469: SN_Client_WillMsgUpdate now says will must not be NULL.
F-12472: the retained purge comment now says each restore runs at most WMQB_RETAINED_PURGE_PASSES scans and skips keys too long to stage.
F-13232: the persist Client ID comment now cites the 65535-byte string limit, and says the 23-byte figure in [MQTT-3.1.3-5] is a minimum, not a cap.
F-13819: the keep-alive timestamp comment now says it updates on each fully written buffer, including streamed PUBLISH segments.
F-13821/13822:

• The two MQTT Disconnect APIs and SN_Client_Disconnect no longer claim to be non-blocking.
• They now say they don't wait for a response, and block on the write unless built with WOLFMQTT_NONBLOCK.

F-14376: the packetAck field comment now says it is protected by lockClient.
F-14377: the client_receive_max comment now gives the real default of 0, meaning no per-client cap.
F-14591: MqttClient_CancelMessage now says it does no network I/O; in multithreaded builds it can only wait on locks.
F-15396/15397:

• Property adds now say the property goes on the tail of the list.
• The MqttClient_PropsAdd return line now says it returns a pointer.

F-15398: MqttClient_IsMessageActive now documents a return of 1 (active), 0 (idle) or a negative error.
F-15399: both QoS 2 cap comments now say the client is disconnected with Quota Exceeded (0x97), not Malformed Packet.
F-15400: all five "client keeps no outbound session state" comments now explain that replayed messages reserve their packet identifiers again when the session resumes.
F-15401: the comment now names Handle_ConnectAck_Props as the function that lowers max_qos, instead of Handle_Props.
F-15402: the schema-mismatch comments (source and header) now say static builds skip the wipe.
F-16007: the persistence hooks comment now says most shadow-write failures are not logged by the broker, so a hook should report its own errors.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[aidangarske]

Skoll Multi-Scan Review

Modes: review + review-security

Overall recommendation: REQUEST_CHANGES
Findings: 7 total — 7 posted, 0 skipped
7 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] [review+review-security] Accepted Will Properties are discarded before publication / queued offline Wills lose expiry and metadata — src/mqtt_broker.c:6314-6318
• [High] [review] Per-subscription Will queueing can discard the required maximum-QoS copy — src/mqtt_broker.c:6305-6319
• [High] [review] Authentication-window flag update fails open on lock errors — src/mqtt_client.c:3550-3557
• [Medium] [review-security] MQTT-SN responses leave cancellation permanently busy — src/mqtt_client.c:1250-1265
• [Medium] [review-security] CancelMessage self-deadlocks inside a property callback — src/mqtt_client.c:1259-1263
• [Medium] [review-security] NetDisconnect_Wait can self-deadlock from non-message callbacks — src/mqtt_client.c:4987-4992
• [Low] [review-security] Static nonblocking broker closes after a partial error DISCONNECT — src/mqtt_broker.c:8974-8986

Review generated by Skoll

[aidangarske]

Skoll Code Review

Scan type: review

Overall recommendation: REQUEST_CHANGES
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] Preserve zero-valued Message Expiry properties — src/mqtt_broker.c:2252-2271
• [High] Complete takeover DISCONNECT after a deferred self-Will write — src/mqtt_broker.c:7767-7783
• [High] Persist Will properties queued for offline sessions — src/mqtt_broker.c:6555-6570
• [High] Validate payload-only Wills against the static offline slot — src/mqtt_broker.c:7426-7437
• [High] Reset MQTT-SN QoS 2 state when the Client ID changes — src/mqtt_sn_client.c:1575-1580

Review generated by Skoll

[aidangarske]

Excellent!