fishy commented on code in PR #3057:
URL: https://github.com/apache/thrift/pull/3057#discussion_r1819375005
##########
lib/go/thrift/binary_protocol.go:
##########
@@ -555,7 +555,28 @@ func safeReadBytes(size int32, trans io.Reader) ([]byte,
error) {
return nil, nil
}
- buf := new(bytes.Buffer)
- _, err := io.CopyN(buf, trans, int64(size))
- return buf.Bytes(), err
+ const readLimit = 10 * 1024 * 1024
Review Comment:
since the stated goal here is to optimize for ordinary cases (with size <
readLimit), why not just do `io.CopyN` with `bytes.Buffer` when it's the edge
case? that way we don't have to allocate for an 10MiB buffer upfront if the
size is malformed, and if it's really a very huge payload we just pay the price
for a bit more allocations (same as today).
(I would also consider making this limit configurable, but don't feel too
strongly either way here)
--
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: notifications-unsubscr...@thrift.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
