diff --git a/crates/ipp/src/lib.rs b/crates/ipp/src/lib.rs index d0535a86ce..bbf6efffec 100644 --- a/crates/ipp/src/lib.rs +++ b/crates/ipp/src/lib.rs @@ -67,6 +67,8 @@ pub struct Printer { /// Timeouts: connecting, and each request (a Print-Job carries the whole PDF). const CONNECT: Duration = Duration::from_secs(3); const REQUEST: Duration = Duration::from_secs(60); +/// The whole reply, however it trickles in: the timeouts above only cover each read. +const REPLY_DEADLINE: Duration = Duration::from_secs(120); /// Largest reply read. const MAX_REPLY: usize = 16 << 20; /// Largest number of attributes taken from a reply. @@ -434,7 +436,13 @@ fn read_response(stream: &mut Stream) -> Result> { let mut body_at = None; let mut expected: Option = None; let mut chunked = false; + // Where the chunk walk resumes, so each read costs its own bytes, not the whole body again. + let mut chunk_at = 0usize; + let started = std::time::Instant::now(); loop { + if started.elapsed() > REPLY_DEADLINE { + return Err(Error::Protocol("the print server took too long to reply".into())); + } let n = stream.read(&mut buf).map_err(|e| Error::Protocol(format!("reading: {e}")))?; if n == 0 { return Ok(raw); @@ -455,7 +463,7 @@ fn read_response(stream: &mut Stream) -> Result> { let body = raw.get(at..).unwrap_or_default(); let complete = match expected { Some(len) if !chunked => body.len() >= len, - _ if chunked => chunked_complete(body), + _ if chunked => chunked_complete(body, &mut chunk_at), _ => false, }; if complete { @@ -483,8 +491,11 @@ fn http_response(raw: &[u8]) -> Result<(u16, Headers, &[u8])> { /// `0\r\n\r\n` inside a chunk's payload (an attribute value can hold it) does not count. A /// malformed or impossible size counts as complete, so [`dechunk`] reports it instead of /// waiting for more. -fn chunked_complete(mut b: &[u8]) -> bool { +fn chunked_complete(body: &[u8], resume: &mut usize) -> bool { + let mut b = body.get(*resume..).unwrap_or_default(); loop { + // Chunks already walked are not walked again: `resume` marks the start of this one. + *resume = body.len() - b.len(); let Some(line_end) = b.windows(2).position(|w| w == b"\r\n") else { return false }; let size_str = String::from_utf8_lossy(b.get(..line_end).unwrap_or(&[])); let Ok(size) = usize::from_str_radix(size_str.split(';').next().unwrap_or_default().trim(), 16) else { return true }; diff --git a/crates/ipp/src/tests.rs b/crates/ipp/src/tests.rs index 394ff758d1..518c9dfa39 100644 --- a/crates/ipp/src/tests.rs +++ b/crates/ipp/src/tests.rs @@ -429,10 +429,29 @@ fn a_last_chunk_pattern_inside_a_value_does_not_end_the_reply() { assert_eq!(list.len(), 1); assert_eq!(list[0].name, "Of\r\n0\r\n\r\nfice"); // The boundary walk itself: complete only at a zero-size chunk, never on payload bytes. - assert!(!chunked_complete(b"5\r\nab\r\n0")); - assert!(!chunked_complete(b"7\r\n\r\n0\r\n\r\nx\r\n")); - assert!(chunked_complete(b"7\r\n\r\n0\r\n\r\nx\r\n0\r\n\r\n")); - assert!(chunked_complete(b"zz\r\n") && dechunk(b"zz\r\n").is_err()); + assert!(!complete(b"5\r\nab\r\n0")); + assert!(!complete(b"7\r\n\r\n0\r\n\r\nx\r\n")); + assert!(complete(b"7\r\n\r\n0\r\n\r\nx\r\n0\r\n\r\n")); + assert!(complete(b"zz\r\n") && dechunk(b"zz\r\n").is_err()); +} + +/// [`chunked_complete`] on a whole body at once. +fn complete(body: &[u8]) -> bool { + chunked_complete(body, &mut 0) +} + +/// Fed a byte at a time, as a server dripping the reply would, the walk resumes where it stopped +/// and agrees with the one-shot answer at every length. +#[test] +fn the_chunk_walk_resumes_instead_of_starting_over() { + let body = b"3\r\nabc\r\n10\r\n0123456789abcdef\r\n0\r\n\r\n"; + let mut at = 0; + for len in 0..=body.len() { + let prefix = &body[..len]; + assert_eq!(chunked_complete(prefix, &mut at), complete(prefix), "{len}"); + assert!(at <= len); + } + assert!(at > 0, "it moved past the chunks already walked"); } /// A chunk size that cannot be satisfied (`usize::MAX`, `usize::MAX - 1`, anything past the @@ -440,7 +459,7 @@ fn a_last_chunk_pattern_inside_a_value_does_not_end_the_reply() { #[test] fn an_impossible_chunk_size_is_a_protocol_error_not_a_panic() { for size_line in ["ffffffffffffffff", "fffffffffffffffe", "1000001"] { - assert!(chunked_complete(format!("{size_line}\r\n").as_bytes())); + assert!(complete(format!("{size_line}\r\n").as_bytes())); assert!(dechunk(format!("{size_line}\r\nab\r\n").as_bytes()).is_err()); let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let port = listener.local_addr().unwrap().port();