Skip to content

Commit fe6f8d9

Browse files
authored
fix(web): Buffer incomplete messages (#1528)
Previously, if we did not find trailers in the incoming message and the message len was longer than the actual buffer we returned the buffer. This would mean we would fail to parse incomplete message buffers. This fixes `find_trailers` to support three return types, `Trailers(len)`, `IncompleteBuf`, `Done`. Previously, there was only `Some(len)` and `None`.
1 parent 6af8d5c commit fe6f8d9

File tree

1 file changed

+82
-31
lines changed

1 file changed

+82
-31
lines changed

tonic-web/src/call.rs

Lines changed: 82 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -242,33 +242,41 @@ where
242242
cx: &mut Context<'_>,
243243
) -> Poll<Option<Result<Self::Data, Self::Error>>> {
244244
if self.client && self.direction == Direction::Decode {
245-
let buf = ready!(self.as_mut().poll_decode(cx));
246-
247-
return if let Some(Ok(mut buf)) = buf {
248-
// We found some trailers so extract them since we
249-
// want to return them via `poll_trailers`.
250-
if let Some(len) = find_trailers(&buf[..]) {
251-
// Extract up to len of where the trailers are at
252-
let msg_buf = buf.copy_to_bytes(len);
253-
match decode_trailers_frame(buf) {
254-
Ok(Some(trailers)) => {
255-
self.project().trailers.replace(trailers);
245+
let mut me = self.as_mut();
246+
247+
loop {
248+
let buf = ready!(me.as_mut().poll_decode(cx));
249+
250+
return if let Some(Ok(incoming_buf)) = buf {
251+
let buf = &mut me.as_mut().project().buf;
252+
253+
buf.put(incoming_buf);
254+
255+
match find_trailers(&buf[..]) {
256+
FindTrailers::Trailer(len) => {
257+
// Extract up to len of where the trailers are at
258+
let msg_buf = buf.copy_to_bytes(len);
259+
match decode_trailers_frame(buf.split().freeze()) {
260+
Ok(Some(trailers)) => {
261+
self.project().trailers.replace(trailers);
262+
}
263+
Err(e) => return Poll::Ready(Some(Err(e))),
264+
_ => {}
265+
}
266+
267+
if msg_buf.has_remaining() {
268+
return Poll::Ready(Some(Ok(msg_buf)));
269+
} else {
270+
return Poll::Ready(None);
271+
}
256272
}
257-
Err(e) => return Poll::Ready(Some(Err(e))),
258-
_ => {}
273+
FindTrailers::IncompleteBuf => continue,
274+
FindTrailers::Done => Poll::Ready(Some(Ok(buf.split().freeze()))),
259275
}
260-
261-
if msg_buf.has_remaining() {
262-
return Poll::Ready(Some(Ok(msg_buf)));
263-
} else {
264-
return Poll::Ready(None);
265-
}
266-
}
267-
268-
Poll::Ready(Some(Ok(buf)))
269-
} else {
270-
Poll::Ready(buf)
271-
};
276+
} else {
277+
Poll::Ready(buf)
278+
};
279+
}
272280
}
273281

274282
match self.direction {
@@ -413,21 +421,21 @@ fn make_trailers_frame(trailers: HeaderMap) -> Vec<u8> {
413421
/// its location in the original buf. If `None` is returned we did
414422
/// not find a trailers in this buffer either because its incomplete
415423
/// or the buffer jsut contained grpc message frames.
416-
fn find_trailers(buf: &[u8]) -> Option<usize> {
424+
fn find_trailers(buf: &[u8]) -> FindTrailers {
417425
let mut len = 0;
418426
let mut temp_buf = &buf[..];
419427

420428
loop {
421429
// To check each frame, there must be at least GRPC_HEADER_SIZE
422430
// amount of bytes available otherwise the buffer is incomplete.
423431
if temp_buf.is_empty() || temp_buf.len() < GRPC_HEADER_SIZE {
424-
return None;
432+
return FindTrailers::Done;
425433
}
426434

427435
let header = temp_buf.get_u8();
428436

429437
if header == GRPC_WEB_TRAILERS_BIT {
430-
return Some(len);
438+
return FindTrailers::Trailer(len);
431439
}
432440

433441
let msg_len = temp_buf.get_u32();
@@ -437,13 +445,20 @@ fn find_trailers(buf: &[u8]) -> Option<usize> {
437445
// If the msg len of a non-grpc-web trailer frame is larger than
438446
// the overall buffer we know within that buffer there are no trailers.
439447
if len > buf.len() {
440-
return None;
448+
return FindTrailers::IncompleteBuf;
441449
}
442450

443451
temp_buf = &buf[len as usize..];
444452
}
445453
}
446454

455+
#[derive(Debug, PartialEq, Eq)]
456+
enum FindTrailers {
457+
Trailer(usize),
458+
IncompleteBuf,
459+
Done,
460+
}
461+
447462
#[cfg(test)]
448463
mod tests {
449464
use super::*;
@@ -494,7 +509,7 @@ mod tests {
494509

495510
let out = find_trailers(&buf[..]);
496511

497-
assert_eq!(out, Some(0));
512+
assert_eq!(out, FindTrailers::Trailer(0));
498513
}
499514

500515
#[test]
@@ -511,12 +526,48 @@ mod tests {
511526

512527
let out = find_trailers(&buf[..]);
513528

514-
assert_eq!(out, Some(81));
529+
assert_eq!(out, FindTrailers::Trailer(81));
515530

516531
let trailers = decode_trailers_frame(Bytes::copy_from_slice(&buf[81..]))
517532
.unwrap()
518533
.unwrap();
519534
let status = trailers.get("grpc-status").unwrap();
520535
assert_eq!(status.to_str().unwrap(), "0")
521536
}
537+
538+
#[test]
539+
fn find_trailers_buffered_incomplete_message() {
540+
let buf = vec![
541+
0, 0, 0, 9, 238, 10, 233, 19, 18, 230, 19, 10, 9, 10, 1, 120, 26, 4, 84, 69, 88, 84,
542+
18, 60, 10, 58, 10, 56, 3, 0, 0, 0, 44, 0, 0, 0, 0, 0, 0, 0, 116, 104, 105, 115, 32,
543+
118, 97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114, 105, 116, 116, 101, 110, 32,
544+
118, 105, 97, 32, 119, 114, 105, 116, 101, 32, 100, 101, 108, 101, 103, 97, 116, 105,
545+
111, 110, 33, 18, 62, 10, 60, 10, 58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0, 0, 0, 116, 104,
546+
105, 115, 32, 118, 97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114, 105, 116, 116,
547+
101, 110, 32, 98, 121, 32, 97, 110, 32, 101, 109, 98, 101, 100, 100, 101, 100, 32, 114,
548+
101, 112, 108, 105, 99, 97, 33, 18, 62, 10, 60, 10, 58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0,
549+
0, 0, 116, 104, 105, 115, 32, 118, 97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114,
550+
105, 116, 116, 101, 110, 32, 98, 121, 32, 97, 110, 32, 101, 109, 98, 101, 100, 100,
551+
101, 100, 32, 114, 101, 112, 108, 105, 99, 97, 33, 18, 62, 10, 60, 10, 58, 3, 0, 0, 0,
552+
46, 0, 0, 0, 0, 0, 0, 0, 116, 104, 105, 115, 32, 118, 97, 108, 117, 101, 32, 119, 97,
553+
115, 32, 119, 114, 105, 116, 116, 101, 110, 32, 98, 121, 32, 97, 110, 32, 101, 109, 98,
554+
101, 100, 100, 101, 100, 32, 114, 101, 112, 108, 105, 99, 97, 33, 18, 62, 10, 60, 10,
555+
58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0, 0, 0, 116, 104, 105, 115, 32, 118, 97, 108, 117,
556+
101, 32, 119, 97, 115, 32, 119, 114, 105, 116, 116, 101, 110, 32, 98, 121, 32, 97, 110,
557+
32, 101, 109, 98, 101, 100, 100, 101, 100, 32, 114, 101, 112, 108, 105, 99, 97, 33, 18,
558+
62, 10, 60, 10, 58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0, 0, 0, 116, 104, 105, 115, 32, 118,
559+
97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114, 105, 116, 116, 101, 110, 32, 98,
560+
121, 32, 97, 110, 32, 101, 109, 98, 101, 100, 100, 101, 100, 32, 114, 101, 112, 108,
561+
105, 99, 97, 33, 18, 62, 10, 60, 10, 58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0, 0, 0, 116, 104,
562+
105, 115, 32, 118, 97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114, 105, 116, 116,
563+
101, 110, 32, 98, 121, 32, 97, 110, 32, 101, 109, 98, 101, 100, 100, 101, 100, 32, 114,
564+
101, 112, 108, 105, 99, 97, 33, 18, 62, 10, 60, 10, 58, 3, 0, 0, 0, 46, 0, 0, 0, 0, 0,
565+
0, 0, 116, 104, 105, 115, 32, 118, 97, 108, 117, 101, 32, 119, 97, 115, 32, 119, 114,
566+
105, 116, 116, 101, 110, 32, 98, 121, 32,
567+
];
568+
569+
let out = find_trailers(&buf[..]);
570+
571+
assert_eq!(out, FindTrailers::IncompleteBuf);
572+
}
522573
}

0 commit comments

Comments
 (0)