Repository navigation
minecraft/nbt: Bound array and list allocations by the input - #528
Open
HashimTheArab wants to merge 3 commits into
Open
HashimTheArab wants to merge 3 commits into
HashimTheArab wants to merge 3 commits into
Conversation
The network encoding's Int32Slice and Int64Slice, TAG_Byte_Array, TAG_List of TAG_Byte and every other TAG_List still allocated the full declared length before reading a single element. Ten bytes of network NBT declaring an int array of 2^31-2 elements allocated 8 GiB (16 GiB for a long array), and a list of any type allocated a slice of that many elements. Variable-width arrays and lists now reject a count that cannot fit in the bytes remaining when the reader reports them, and otherwise start with a small capacity and grow with the elements actually read. Byte arrays and byte lists go through readArrayBytes like the fixed-width arrays do.
reflect.Append allocated a new slice header for every element, which made decoding a valid 10k-element list about four times slower than before the bound. The slice is now grown in place and decoded into by index; with a known input length it is allocated once, as before.
HashimTheArab
added a commit
to HashimTheArab/gophertunnel
that referenced
this pull request
Sep 26, 2026
…) into lunar Brings in the 13 new upstream commits together with the fork's two open upstream fix PRs in one commit, so the diff shows the net result: Sandertv#527 reverts the deferred-packet re-check that master added, so that churn cancels out here. New upstream behaviour folded into the fork's reworked login stack: - ListenConfig.LoginTimeout and the authenticated flag + closeTransport it relies on. - ListenerGroup shared player counts across listeners. - GameType on ServerStatus and the pong game-type field. - ClientBoundDebugRenderer encoding/build fixes. Kept the fork's versions where it deliberately diverges: the reworked decoder (buffer pooling, per-batch algorithm, 1600-packet cap), the login state machine and StartGame flow, the realms client (version negotiation; already carries Stories settings), and dial (no pong-port following, so DisablePortFollowing/EnableLegacyAuth are not added). nbt allocation bounding is already present from an earlier fork change, so Sandertv#528 is a no-op here apart from keeping checkRemaining, which the fork's raw-message decoder still uses.
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
1 participant
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.
#516 bounded the fixed-width
Int32Slice/Int64Sliceof the little- and big-endian encodings, but the paths packets actually use were still open:networkLittleEndian.Int32Slice/Int64Sliceallocatedmake([]intN, n)from the declared count before reading an element. Ten bytes (0a 00 0b 00 fe ff ff ff 0f 00) allocate 8 GiB; the long-array variant 16 GiB. Each attempt takes about 2 ms and then fails withBufferOverrun, so a peer can drive a process into OOM or a GC spiral with a handful of block-actor NBT payloads.TAG_Listof any element type didreflect.MakeSlice(len, len)with the declared length, in every encoding.TAG_Byte_ArrayandTAG_ListofTAG_BytecheckedLen()only when the reader had one, thenmake([]byte, length).Variable-width arrays and lists now go through
offsetReader.sliceCap: with a reader that reports its length, a count that cannot fit in the remaining bytes (at least one byte per element) is aBufferOverrunError; otherwise the slice starts with a small capacity and grows with the elements actually read. Byte arrays and byte lists usereadArrayByteslike the fixed-width arrays.Checked locally (not committed): the six hostile shapes above (int/long array, byte array, byte list, string list,
TAG_Endlist) allocate 0 MiB whether or not the reader exposesLen(), and a struct with[]int32,[]int64,[]string,[][]int32,[4]byteand[]structfields round-trips unchanged through all three encodings, including empty non-nil slices.