Skip to content

Commit 42ca3a3

Browse files
committed
Implement the same OOM fix for reading Bytes
Nobody noticed likely because `with-bytes` feature is rarely used. CC: #411
1 parent 427e7ed commit 42ca3a3

File tree

2 files changed

+76
-7
lines changed

2 files changed

+76
-7
lines changed

protobuf/src/buf_read_iter.rs

+69-6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use bytes::BytesMut;
1515
use error::WireError;
1616
use ProtobufError;
1717
use ProtobufResult;
18+
use stream::READ_RAW_BYTES_MAX_ALLOC;
19+
1820

1921
// If an input stream is constructed with a `Read`, we create a
2022
// `BufReader` with an internal buffer of this size.
@@ -221,6 +223,61 @@ impl<'ignore> BufReadIter<'ignore> {
221223
Ok(r)
222224
}
223225

226+
/// Read at most `max` bytes, append to `Vec`.
227+
///
228+
/// Returns 0 when EOF or limit reached.
229+
fn read_to_vec(&mut self, vec: &mut Vec<u8>, max: usize) -> ProtobufResult<usize> {
230+
let rem = self.fill_buf()?;
231+
232+
let len = cmp::min(rem.len(), max);
233+
vec.extend_from_slice(&rem[..len]);
234+
self.pos_within_buf += len;
235+
Ok(len)
236+
}
237+
238+
/// Read exact number of bytes into `Vec`.
239+
///
240+
/// `Vec` is cleared in the beginning.
241+
pub fn read_exact_to_vec(&mut self, count: usize, target: &mut Vec<u8>) -> ProtobufResult<()> {
242+
// TODO: also do some limits when reading from unlimited source
243+
if count as u64 > self.bytes_until_limit() {
244+
return Err(ProtobufError::WireError(WireError::TruncatedMessage));
245+
}
246+
247+
target.clear();
248+
249+
if count >= READ_RAW_BYTES_MAX_ALLOC && count > target.capacity() {
250+
// avoid calling `reserve` on buf with very large buffer: could be a malformed message
251+
252+
target.reserve(READ_RAW_BYTES_MAX_ALLOC);
253+
254+
while target.len() < count {
255+
if count - target.len() <= target.len() {
256+
target.reserve_exact(count - target.len());
257+
} else {
258+
target.reserve(1);
259+
}
260+
261+
let max = cmp::min(target.capacity() - target.len(), count - target.len());
262+
let read = self.read_to_vec(target, max)?;
263+
if read == 0 {
264+
return Err(ProtobufError::WireError(WireError::TruncatedMessage));
265+
}
266+
}
267+
} else {
268+
target.reserve_exact(count);
269+
270+
unsafe {
271+
self.read_exact(&mut target.get_unchecked_mut(..count))?;
272+
target.set_len(count);
273+
}
274+
}
275+
276+
debug_assert_eq!(count, target.len());
277+
278+
Ok(())
279+
}
280+
224281
#[cfg(feature = "bytes")]
225282
pub fn read_exact_bytes(&mut self, len: usize) -> ProtobufResult<Bytes> {
226283
if let InputSource::Bytes(bytes) = self.input_source {
@@ -231,15 +288,21 @@ impl<'ignore> BufReadIter<'ignore> {
231288
self.pos_within_buf += len;
232289
Ok(r)
233290
} else {
234-
let mut r = BytesMut::with_capacity(len);
235-
unsafe {
236-
{
237-
let mut buf = &mut r.bytes_mut()[..len];
291+
if len >= READ_RAW_BYTES_MAX_ALLOC {
292+
// We cannot trust `len` because protobuf message could be malformed.
293+
// Reading should not result in OOM when allocating a buffer.
294+
let mut v = Vec::new();
295+
self.read_exact_to_vec(len, &mut v)?;
296+
Ok(Bytes::from(v))
297+
} else {
298+
let mut r = BytesMut::with_capacity(len);
299+
unsafe {
300+
let buf = &mut r.bytes_mut()[..len];
238301
self.read_exact(buf)?;
302+
r.advance_mut(len);
239303
}
240-
r.advance_mut(len);
304+
Ok(r.freeze())
241305
}
242-
Ok(r.freeze())
243306
}
244307
}
245308

protobuf/src/stream.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const OUTPUT_STREAM_BUFFER_SIZE: usize = 8 * 1024;
3636
const DEFAULT_RECURSION_LIMIT: u32 = 100;
3737

3838
/// Max allocated vec when reading length-delimited from unknown input stream
39-
const READ_RAW_BYTES_MAX_ALLOC: usize = 10_000_000;
39+
pub(crate) const READ_RAW_BYTES_MAX_ALLOC: usize = 10_000_000;
4040

4141
/// Serialization constants.
4242
pub mod wire_format {
@@ -712,6 +712,12 @@ impl<'a> CodedInputStream<'a> {
712712
/// Read raw bytes into the supplied vector. The vector will be resized as needed and
713713
/// overwritten.
714714
pub fn read_raw_bytes_into(&mut self, count: u32, target: &mut Vec<u8>) -> ProtobufResult<()> {
715+
if false {
716+
// Master uses this version, but keep existing version for a while
717+
// to avoid possible breakages.
718+
return self.source.read_exact_to_vec(count as usize, target);
719+
}
720+
715721
let count = count as usize;
716722

717723
// TODO: also do some limits when reading from unlimited source

0 commit comments

Comments
 (0)