Skip to content

Conversation

@HenkjanvDiermen
Copy link

Fix FullPacketParser to handle multi-packet chunks

Problem

The FullPacketParser in src/serializer.js only processes the first packet when a chunk contains multiple packets, resulting in data loss.

Current Behavior

When the TCP stream delivers multiple packets in a single chunk:

  1. Only the first packet is parsed and pushed
  2. The remaining packets are discarded
  3. A warning is logged: "Chunk size is X but only Y was read"

Example Scenario

Chunk arrives: 37 bytes = [13-byte spawn_position packet] + [24-byte other packet]

Current code:
  ✗ Parses spawn_position (13 bytes)
  ✗ Logs: "Chunk size is 37 but only 13 was read"
  ✗ DISCARDS the remaining 24 bytes
  ✗ Second packet is lost

This is data loss, not just a warning. The lost packets never reach the application layer.

Why This Happens

TCP provides a byte stream with no packet boundaries. Multiple protocol packets can arrive in a single chunk from the TCP socket. The Splitter (framing layer) handles varint-length-prefixed packets correctly, but FullPacketParser assumes each chunk contains exactly one packet.

Solution

Modified FullPacketParser._transform() to loop through all packets in a chunk using an offset pointer:

_transform (chunk, enc, cb) {
  let offset = 0

  while (offset < chunk.length) {
    let packet
    try {
      packet = this.parsePacketBuffer(chunk.slice(offset))
    } catch (e) {
      if (e.partialReadError) {
        // Partial read errors can occur for packets in wrong protocol state or malformed data
        // Drop the remainder of this chunk and continue
        return cb()
      } else {
        console.log('EBuf', chunk.slice(offset).toString('hex'))
        return cb(e)
      }
    }

    this.push(packet)
    offset += packet.metadata.size
  }

  cb()
}

After Fix

Chunk arrives: 37 bytes = [13-byte spawn_position] + [24-byte other packet]

New code:
  ✓ Parses spawn_position (13 bytes)
  ✓ Pushes it downstream
  ✓ Advances offset to 13
  ✓ Parses second packet (24 bytes)
  ✓ Pushes it downstream
  ✓ All data processed - no loss

Impact

Fixed

  • ✅ Multi-packet chunks are fully processed (no data loss)
  • ✅ All packets reach the application layer
  • ✅ No more spurious "chunk size" warnings

Unchanged

  • ✅ Single-packet chunks work identically
  • ✅ Partial read errors still drop malformed data (correct behavior)
  • ✅ Backward compatible with existing code

Testing

Tested with Minecraft protocol 1.21.8 and 1.21.9:

  • Before fix: Intermittent packet loss, warnings logged
  • After fix: All packets processed correctly, no data loss

This issue is more prevalent in protocols with high packet rates or when multiple small packets are batched by the TCP stack.

Related

This affects any protocol using FullPacketParser where multiple packets can arrive in a single chunk. The regular Parser class already handles this correctly with buffering - this brings FullPacketParser to feature parity.

The FullPacketParser only processed the first packet when multiple
packets arrived in a single chunk, resulting in data loss.

This fix loops through all packets in the chunk using an offset,
ensuring all packets are processed and pushed downstream.

Fixes intermittent packet loss in high-throughput scenarios where
TCP delivers multiple protocol packets in a single read.
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