Skip to content

Fix malformed disconnect frame after inbound parse errors#197

Merged
shamblett merged 1 commit into
shamblett:pr197from
Konoha-orz:fix/mqtt-buffer-corruption
May 15, 2026
Merged

Fix malformed disconnect frame after inbound parse errors#197
shamblett merged 1 commit into
shamblett:pr197from
Konoha-orz:fix/mqtt-buffer-corruption

Conversation

@Konoha-orz

Copy link
Copy Markdown
Contributor

Fix corrupted DISCONNECT frame on inbound parse failure

Summary

This PR fixes a buffer corruption issue in MqttServerConnection._onData() that can cause the client to send a malformed DISCONNECT packet after an inbound message parsing error.

The core fix is to clear messageStream before serializing the DISCONNECT frame in the _onData() catch block. This prevents residual bytes from a failed inbound parse from being included in the outbound DISCONNECT packet.

Problem

When _onData() catches an irrecoverable parsing exception, it currently attempts to send a normal DISCONNECT message:

messageStream.reset();
disconnect.writeTo(messageStream);
messageStream.seek(0);
send(messageStream);

MqttByteBuffer.reset() only resets the buffer position to 0; it does not clear the underlying byte buffer.

If messageStream still contains bytes from a malformed or partially parsed inbound packet, disconnect.writeTo(messageStream) overwrites from position 0, but send(messageStream) still sends message.length, which includes any residual bytes left after the newly written DISCONNECT frame.

This can result in corrupted outbound data being sent to the broker.

Root Cause

MqttServerConnection.send() sends the full buffer length:

final length = message.length;
final messageBytes = message.read(length);

So after reset() + disconnect.writeTo(...), old bytes still present in the buffer can remain part of the outgoing frame.

Changes

mqtt_server_connection.dart

Replaces reset() with seek(0) + clear() before writing the DISCONNECT message:

messageStream.seek(0);
messageStream.clear();
disconnect.writeTo(messageStream);
messageStream.seek(0);
send(messageStream);

This ensures the outbound DISCONNECT packet contains only the serialized DISCONNECT frame.

mqtt_header.dart

Adds a defensive guard in MqttHeader.headerBytes() so headers cannot be serialized with:

  • messageType == null
  • messageType == MqttMessageType.reserved1

This prevents accidentally generating a fixed header with MQTT message type 0, which is reserved and invalid for transmission.

This check only affects serialization. Existing header parsing behavior is unchanged.

mqtt_byte_buffer.dart

Tightens MqttByteBuffer.isMessageAvailable() by:

  • checking availableBytes rather than total length
  • only skipping a leading zero byte when more than one byte is available
  • re-checking header availability after the skip

This avoids a range error when the buffer position points at a single remaining 0x00 byte.

Tests Added

This PR adds regression coverage for the affected paths:

  • malformed inbound message handling sends a clean DISCONNECT frame
  • MqttHeader.headerBytes() rejects null message types
  • MqttHeader.headerBytes() rejects reserved1
  • MqttByteBuffer.isMessageAvailable() safely handles a single remaining zero byte

The new connection regression test failed before the fix because the sent frame contained residual data before the DISCONNECT bytes. After the fix, it sends only:

[0xE0, 0x00]

Test Plan

Ran locally:

dart format --output=none --set-exit-if-changed .
dart analyze
dart test
dart run coverage:test_with_coverage

All passed.

Compatibility

This change does not modify any public API signatures.

The main behavior change is limited to error handling and defensive validation during outbound header serialization.

@shamblett shamblett changed the base branch from master to pr197 May 15, 2026 08:16
@shamblett shamblett merged commit 66d9173 into shamblett:pr197 May 15, 2026
2 checks passed
shamblett added a commit that referenced this pull request May 15, 2026
Co-authored-by: Konoha <31205129+Konoha-orz@users.noreply.github.com>
@shamblett

Copy link
Copy Markdown
Owner

Thanks for the PR. Package updated and re published at version 4.17.1

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.

2 participants