slachiewicz opened a new pull request, #3810: URL: https://github.com/apache/thrift/pull/3810
Fixes THRIFT-5828. Above `bytes.MinRead`, `safeReadBytes` copied into a `bytes.Buffer` with `io.CopyN`. `bytes.Buffer.ReadFrom` grows by doubling and reserves at least `bytes.MinRead` before each read, so a well-formed message always landed in a buffer holding roughly twice what it asked for — reading 32 KiB allocated 64 KiB. That is what the reporter measured in 2024. The buffer now starts at `bytes.MinRead` and doubles as data arrives, capped at the requested size. Both properties hold: - a size the sender made up costs only the bytes actually sent (the CVE-2019-11939 guard that motivated the `bytes.Buffer` in the first place); - a complete read ends in a buffer of exactly that size. ### Tests `TestSafeReadBytesDoesNotOverAllocate` fails on master: ``` binary_protocol_test.go:69: read 32768 bytes into a buffer of capacity 65536, want capacity 32768 ``` `TestSafeReadBytesLargeSizeShortData` pins the other half — `math.MaxInt32` declared, 1 KiB delivered, allocation stays proportional to the data. The pre-existing `TestSafeReadBytesAlloc` (THRIFT-5322) still passes. ### Numbers A benchmark case for the exact-size read is added alongside the existing ones. Both figures include the benchmark's own 32 KiB source buffer: | | B/op | allocs/op | ns/op | |---|---|---|---| | before | 130683 | 11 | 26798 | | after | 65075 | 8 | 17106 | Verified: `go test -race ./lib/go/thrift` passes. *This change was created with AI assistance.* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
