fix: network stream desync from mismatched write/read byte-array framing - #19
Merged
Merged
Conversation
…cts writeByteArray()'s framing RegistryFriendlyByteBuf.write(serializer, data) wrote the encoded CBOR payload with plain writeBytes() - Netty's raw byte-array write, no length prefix. Its counterpart read(serializer) reads it back with readByteArray() - Minecraft's own length-prefixed format (a VarInt count followed by that many bytes). The two were never compatible; write() just had no real caller previously exercising the mismatch. That changed when streamCodec's implementation was refactored earlier this session to route through write()/read() instead of calling Nbt's/Cbor's encode/decode directly with matching writeByteArray/ readByteArray calls (as it used to). Any StreamCodec built via KSerializer.streamCodec now hit this immediately in CI: io.netty.handler.codec.DecoderException: ByteArray with size 12607 is bigger than allowed 703 readByteArray() misread the front of the raw CBOR bytes as if they were a VarInt length prefix, producing a garbage size - a real stream desync, not a flake. Reproduced locally via ComposeItemContainerMenuClientTests. testExcludedSlotBehaviorViaRealMenuOpen (uses a real menu-open network round trip), which was failing in CI (fabric:client, neoforge:client) on 1.21.x. Fix: write() now uses writeByteArray() to match read()'s framing. Verified: the previously-failing test now passes standalone, and the full :archie-gametest-neoforge:runGametestClient suite is clean (25/25 on a second run; the first run's single failure was the already-documented pre-existing "stale render state" flake, unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Summary
checkCI (fabric:client,neoforge:client) was failing on1.21.xwith:RegistryFriendlyByteBuf.write(serializer, data)wrote the encoded CBOR payload withwriteBytes()- Netty's raw byte-array write, no length prefix. Its counterpartread(serializer)reads it back withreadByteArray()- Minecraft's own length-prefixed format (a VarInt count, then that many bytes). The two were never actually compatible.write()just had no real caller exercising the mismatch untilstreamCodec's implementation was refactored earlier this session to route throughwrite()/read()instead of callingNbt's/Cbor's encode/decode directly with matchingwriteByteArray/readByteArraycalls (as it used to). AnyStreamCodecbuilt viaKSerializer.streamCodecthen hit this immediately -readByteArray()misread the front of the raw CBOR bytes as if they were a VarInt length prefix, producing a garbage size. A real stream desync, not a flake.Fix
write()now useswriteByteArray()to matchread()'s framing.Verification
Reproduced locally via
ComposeItemContainerMenuClientTests.testExcludedSlotBehaviorViaRealMenuOpen(does a real menu-open network round trip) - failed before this fix, passes after. Full:archie-gametest-neoforge:runGametestClientsuite is clean (25/25 on a second run; the first run's lone failure was the already-documented pre-existing "stale render state" flake, unrelated).🤖 Generated with Claude Code