Skip to content

fix(thp): decode a packet that exactly fills the packet size - #2

Open
munzzyy wants to merge 1 commit into
cake-tech:trunkfrom
munzzyy:fix/thp-exact-fit-packet
Open

fix(thp): decode a packet that exactly fills the packet size#2
munzzyy wants to merge 1 commit into
cake-tech:trunkfrom
munzzyy:fix/thp-exact-fit-packet

Conversation

@munzzyy

@munzzyy munzzyy commented Aug 3, 2026

Copy link
Copy Markdown

TrezorDecoder.decodePackage decides whether a THP v2 packet already holds the whole message by comparing the length field against the bytes left in the reader:

if (length >= reader.remainingLength) {
  final payload = reader.read(reader.remainingLength);
  return TrezorPackageV2(headers: headers, payload: payload);
}

final payload = reader.read(length - crcLength);
final crc = reader.readUint32();

length counts the payload plus the 4 byte CRC, and the second branch reads exactly that many bytes, so the message is complete whenever remainingLength >= length. With >= on the incomplete side the equality case lands in the wrong branch. That case is the largest message that still fits in a single packet: 235 payload bytes over a 244 byte BLE MTU, or 55 payload bytes over a 64 byte USB packet. It comes back with crc == null, so needsContinuationPacket is true and both transports wait for a packet the device is never going to send. _readResponse in trezor_usb_manager.dart keeps calling transferIn(), and the notify handler in trezor_gatt_gateway.dart returns early without completing the request.

The change is the comparison. I also added test/trezor/protocol/decoder_test.dart with a helper that builds a real THP packet (control byte, channel, length, payload, CRC32, padded the way the device pads) and covers three cases: a short padded packet, the exact fit packet, and a message too large for one packet.

Running the new test on trunk with the one line change reverted:

00:00 +0: TrezorDecoder decodePackage decodes a padded packet
00:00 +1: TrezorDecoder decodePackage decodes a packet that exactly fills the packet size
00:00 +1 -1: TrezorDecoder decodePackage decodes a packet that exactly fills the packet size [E]
  Expected: <235>
    Actual: <239>

  package:matcher                              expect
  test/trezor/protocol/decoder_test.dart 52:7  main.<fn>.<fn>

00:00 +1 -1: TrezorDecoder decodePackage asks for a continuation packet when the message does not fit
00:00 +2 -1: Some tests failed.

239 is the whole remainder including the CRC. With the fix:

00:00 +0: TrezorDecoder decodePackage decodes a padded packet
00:00 +1: TrezorDecoder decodePackage decodes a packet that exactly fills the packet size
00:00 +2: TrezorDecoder decodePackage asks for a continuation packet when the message does not fit
00:00 +3: All tests passed!

flutter test for the whole package is green at 31 tests, and flutter analyze reports nothing on either changed file. Flutter 3.44.6, Dart 3.12.2, Linux.

decodePackage treated a message as incomplete when its length field was
equal to the bytes left in the packet, so the largest message that still
fits in one packet was reported as needing a continuation packet. The
transports then blocked waiting for a packet the device never sends.

The complete branch consumes exactly length bytes, so it only needs
remainingLength >= length.
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