ranflarion commented on code in PR #10522:
URL: https://github.com/apache/arrow-rs/pull/10522#discussion_r3705736824
##########
arrow-ipc/src/reader.rs:
##########
@@ -1804,6 +1804,36 @@ pub(crate) enum IpcMessage {
},
}
+/// Upper bound on how much memory a single message length is allowed to
reserve before any
+/// of the bytes it promises have been read.
+///
+/// Message lengths come from the stream itself, so they cannot be trusted: a
truncated or
+/// corrupted stream can declare a body of arbitrary size. Reserving that up
front turns a
+/// malformed input into an allocation failure, which aborts the process
rather than
+/// returning an error the caller can handle. Beyond this size the buffer
grows as the data
+/// arrives instead, so an implausible length costs one bounded allocation and
then fails as
+/// a short read.
+///
+/// The value trades the size of that bounded allocation against how large a
body still gets
+/// read in a single allocation: bodies up to this size behave exactly as
before, larger ones
+/// grow geometrically (`MutableBuffer::reserve` doubles) and pay the
reallocations.
+const MAX_PREALLOC_BYTES: usize = 64 * 1024 * 1024;
+
+/// Reads exactly `len` bytes of message body, without reserving `len` before
reading it.
+fn read_body_bounded<R: Read>(reader: &mut R, len: usize) ->
Result<MutableBuffer, ArrowError> {
Review Comment:
MutableBuffer::resize goes through reserve, which already grows capacity to
max(required, capacity * 2) (arrow-buffer/src/buffer/mutable.rs:256), so the
64MB steps only moved len while capacity doubled underneath: the 8GB case
reallocates 7 times (64M→128M→…→8G) with cumulative copy under one extra pass
over the body, not 128K times. I've made the doubling explicit in
read_body_bounded anyway (resize(len.min(target * 2))), so the growth policy is
visible in the loop instead of relying on reserve internals, and the loop runs
log2(n / 64MB) iterations. Total zeroing is unchanged from before this PR,
from_len_zeroed zeroed the whole body up front too. On a built-in,
take(len).read_to_end is what the metadata path uses, but read_to_end only
fills a Vec and the body has to stay a MutableBuffer for the 64-byte aligned
allocation, so the body loop stays hand-rolled.
--
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]
