From c98e1c98ba23df632f754113ee4430072180b989 Mon Sep 17 00:00:00 2001 From: Zhang Jingqiang Date: Tue, 14 May 2024 18:53:26 +0800 Subject: [PATCH] simplify http1 body type --- lib/g3-http/src/body/chunked_transfer.rs | 10 +- lib/g3-http/src/body/mod.rs | 3 +- lib/g3-http/src/body/preview.rs | 132 ++++++------------ lib/g3-http/src/body/reader.rs | 53 +++---- lib/g3-http/src/client/response.rs | 20 +-- lib/g3-http/src/client/transparent.rs | 20 +-- lib/g3-http/src/connect/response.rs | 28 ++-- lib/g3-http/src/server/request.rs | 22 +-- lib/g3-http/src/server/transparent.rs | 18 +-- .../src/reqmod/h1/bidirectional.rs | 12 +- .../src/reqmod/h1/forward_body.rs | 14 +- lib/g3-icap-client/src/reqmod/h1/mod.rs | 8 +- lib/g3-icap-client/src/reqmod/h1/preview.rs | 8 +- .../src/reqmod/h1/recv_request.rs | 7 +- .../src/reqmod/h1/recv_response.rs | 6 +- .../src/respmod/h1/bidirectional.rs | 12 +- .../src/respmod/h1/forward_body.rs | 6 +- lib/g3-icap-client/src/respmod/h1/preview.rs | 6 +- .../src/respmod/h1/recv_response.rs | 7 +- 19 files changed, 120 insertions(+), 272 deletions(-) diff --git a/lib/g3-http/src/body/chunked_transfer.rs b/lib/g3-http/src/body/chunked_transfer.rs index fa7e4ff7d..e480d3b90 100644 --- a/lib/g3-http/src/body/chunked_transfer.rs +++ b/lib/g3-http/src/body/chunked_transfer.rs @@ -90,7 +90,7 @@ where ChunkedNoTrailerEncodeTransfer::new(reader, writer, copy_config.yield_size()); ChunkedTransferState::Encode(encoder) } - HttpBodyType::ChunkedWithoutTrailer | HttpBodyType::ChunkedWithTrailer => { + HttpBodyType::Chunked => { let body_reader = HttpBodyReader::new(reader, body_type, body_line_max_len); let copy = ROwnedLimitedCopy::new(body_reader, writer, copy_config); ChunkedTransferState::Copy(copy) @@ -136,7 +136,7 @@ where ChunkedNoTrailerEncodeTransfer::new(reader, writer, copy_config.yield_size()); ChunkedTransferState::Encode(encoder) } - HttpBodyType::ChunkedWithoutTrailer | HttpBodyType::ChunkedWithTrailer => { + HttpBodyType::Chunked => { let next_chunk_size = preview_state.chunked_next_size; if next_chunk_size > 0 { let head = format!("{next_chunk_size:x}\r\n"); @@ -411,7 +411,7 @@ mod test { let mut body_transfer = ChunkedTransfer::new( &mut buf_stream, &mut write_buf, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 1024, Default::default(), ); @@ -441,7 +441,7 @@ mod test { let mut body_transfer = ChunkedTransfer::new( &mut buf_stream, &mut write_buf, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 1024, Default::default(), ); @@ -465,7 +465,7 @@ mod test { let mut body_transfer = ChunkedTransfer::new( &mut buf_stream, &mut write_buf, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 1024, Default::default(), ); diff --git a/lib/g3-http/src/body/mod.rs b/lib/g3-http/src/body/mod.rs index fbd04c3de..c60238399 100644 --- a/lib/g3-http/src/body/mod.rs +++ b/lib/g3-http/src/body/mod.rs @@ -17,8 +17,7 @@ #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum HttpBodyType { ContentLength(u64), - ChunkedWithoutTrailer, - ChunkedWithTrailer, + Chunked, ReadUntilEnd, } diff --git a/lib/g3-http/src/body/preview.rs b/lib/g3-http/src/body/preview.rs index 177747508..015b19b6f 100644 --- a/lib/g3-http/src/body/preview.rs +++ b/lib/g3-http/src/body/preview.rs @@ -129,8 +129,7 @@ fn push_preview_data( chunked_next_size: 0, }) } - HttpBodyType::ChunkedWithoutTrailer => push_chunked_preview_data(header, limit, buf, false), - HttpBodyType::ChunkedWithTrailer => push_chunked_preview_data(header, limit, buf, true), + HttpBodyType::Chunked => push_chunked_preview_data(header, limit, buf), } } @@ -138,7 +137,6 @@ fn push_chunked_preview_data( header: &mut Vec, limit: usize, buf: &[u8], - has_trailer: bool, ) -> Result { let mut consume_size = 0; let mut preview_size = 0; @@ -164,17 +162,6 @@ fn push_chunked_preview_data( } if chunk_size == 0 { - if has_trailer { - // do not consume the ending chunk, so we can pass Trailer along with the ending chunk in the continue request - header.put_slice(b"0\r\n\r\n"); - return Ok(PreviewDataState { - consume_size, - preview_size, - preview_eof: false, - chunked_next_size: 0, - }); - } - let end = &left[p + 1..]; let end_len = end.len(); if end_len >= 1 { @@ -200,7 +187,15 @@ fn push_chunked_preview_data( chunked_next_size: 0, }); } - return Err(PreviewError::InvalidChunkedBody); + + // do not consume the ending chunk, so we can pass Trailer along with the ending chunk in the continue request + header.put_slice(b"0\r\n\r\n"); + return Ok(PreviewDataState { + consume_size, + preview_size, + preview_eof: false, + chunked_next_size: 0, + }); } else { // do not consume the ending chunk, send them in the continue request header.put_slice(b"0\r\n\r\n"); @@ -395,13 +390,7 @@ mod tests { fn preview_data_chunked() { let mut headers = Vec::with_capacity(256); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"1\r\n", - ) - .unwrap(); + let s = push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"1\r\n").unwrap(); assert_eq!(s.consume_size, 0); assert_eq!(s.preview_size, 0); assert!(!s.preview_eof); @@ -409,13 +398,7 @@ mod tests { assert_eq!(headers.as_slice(), b"0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"1\r\na", - ) - .unwrap(); + let s = push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"1\r\na").unwrap(); assert_eq!(s.consume_size, 0); assert_eq!(s.preview_size, 0); assert!(!s.preview_eof); @@ -423,13 +406,7 @@ mod tests { assert_eq!(headers.len(), 0); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"1\r\na\r\n", - ) - .unwrap(); + let s = push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"1\r\na\r\n").unwrap(); assert_eq!(s.consume_size, 6); assert_eq!(s.preview_size, 1); assert!(!s.preview_eof); @@ -437,13 +414,8 @@ mod tests { assert_eq!(headers.as_slice(), b"1\r\na\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"1\r\na\r\n1\r\n", - ) - .unwrap(); + let s = + push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"1\r\na\r\n1\r\n").unwrap(); assert_eq!(s.consume_size, 6); assert_eq!(s.preview_size, 1); assert!(!s.preview_eof); @@ -453,7 +425,7 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 4, b"1\r\na\r\n1\r\nb\r\n", ) @@ -467,7 +439,7 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 4, b"1\r\na\r\n3\r\nbcd\r\n", ) @@ -479,13 +451,7 @@ mod tests { assert_eq!(headers.as_slice(), b"1\r\na\r\n3\r\nbcd\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"2\r\nab\r\n", - ) - .unwrap(); + let s = push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"2\r\nab\r\n").unwrap(); assert_eq!(s.consume_size, 7); assert_eq!(s.preview_size, 2); assert!(!s.preview_eof); @@ -493,13 +459,8 @@ mod tests { assert_eq!(headers.as_slice(), b"2\r\nab\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"4\r\nabcd\r\n", - ) - .unwrap(); + let s = + push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"4\r\nabcd\r\n").unwrap(); assert_eq!(s.consume_size, 9); assert_eq!(s.preview_size, 4); assert!(!s.preview_eof); @@ -507,13 +468,8 @@ mod tests { assert_eq!(headers.as_slice(), b"4\r\nabcd\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"5\r\nabcde\r\n", - ) - .unwrap(); + let s = + push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"5\r\nabcde\r\n").unwrap(); assert_eq!(s.consume_size, 7); assert_eq!(s.preview_size, 4); assert!(!s.preview_eof); @@ -523,7 +479,7 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 4, b"1\r\na\r\n4\r\nbcde\r\n", ) @@ -535,13 +491,8 @@ mod tests { assert_eq!(headers.as_slice(), b"1\r\na\r\n3\r\nbcd\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"3\r\nabc\r\n0", - ) - .unwrap(); + let s = + push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"3\r\nabc\r\n0").unwrap(); assert_eq!(s.consume_size, 8); assert_eq!(s.preview_size, 3); assert!(!s.preview_eof); @@ -549,13 +500,8 @@ mod tests { assert_eq!(headers.as_slice(), b"3\r\nabc\r\n0\r\n\r\n"); headers.clear(); - let s = push_preview_data( - &mut headers, - HttpBodyType::ChunkedWithoutTrailer, - 4, - b"4\r\nabcd\r\n0", - ) - .unwrap(); + let s = + push_preview_data(&mut headers, HttpBodyType::Chunked, 4, b"4\r\nabcd\r\n0").unwrap(); assert_eq!(s.consume_size, 9); assert_eq!(s.preview_size, 4); assert!(!s.preview_eof); @@ -565,7 +511,7 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 4, b"4\r\nabcd\r\n0\r\n", ) @@ -579,7 +525,7 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, 4, b"3\r\nabc\r\n0\r\n\r\n", ) @@ -593,7 +539,21 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithoutTrailer, + HttpBodyType::Chunked, + 4, + b"3\r\nabc\r\n0\r\nA: B\r\n\r\n", + ) + .unwrap(); + assert_eq!(s.consume_size, 8); + assert_eq!(s.preview_size, 3); + assert!(!s.preview_eof); + assert_eq!(s.chunked_next_size, 0); + assert_eq!(headers.as_slice(), b"3\r\nabc\r\n0\r\n\r\n"); + + headers.clear(); + let s = push_preview_data( + &mut headers, + HttpBodyType::Chunked, 4, b"4\r\nabcd\r\n0\r\n\r\n", ) @@ -607,9 +567,9 @@ mod tests { headers.clear(); let s = push_preview_data( &mut headers, - HttpBodyType::ChunkedWithTrailer, + HttpBodyType::Chunked, 4, - b"4\r\nabcd\r\n0\r\n\r\n", + b"4\r\nabcd\r\n0\r\nA: B\r\n\r\n", ) .unwrap(); assert_eq!(s.consume_size, 9); diff --git a/lib/g3-http/src/body/reader.rs b/lib/g3-http/src/body/reader.rs index 6ff44a83c..f022b54d8 100644 --- a/lib/g3-http/src/body/reader.rs +++ b/lib/g3-http/src/body/reader.rs @@ -29,7 +29,7 @@ enum NextReadType { UntilEnd, FixedLength, ChunkSize, - ChunkEnd(u8), + ChunkDataEnd(u8), Trailer, } @@ -65,9 +65,7 @@ where content_length = *size; NextReadType::FixedLength } - HttpBodyType::ChunkedWithoutTrailer | HttpBodyType::ChunkedWithTrailer => { - NextReadType::ChunkSize - } + HttpBodyType::Chunked => NextReadType::ChunkSize, HttpBodyType::ReadUntilEnd => NextReadType::UntilEnd, }; let mut r = HttpBodyReader { @@ -164,9 +162,9 @@ where // all data in this chunk/slice has been read out match self.body_type { HttpBodyType::ContentLength(_) => self.next_read_type = NextReadType::EndOfFile, - HttpBodyType::ChunkedWithTrailer | HttpBodyType::ChunkedWithoutTrailer => { - // continue to next chunk - self.next_read_type = NextReadType::ChunkEnd(b'\r') + HttpBodyType::Chunked => { + // read chunk data end + self.next_read_type = NextReadType::ChunkDataEnd(b'\r') } _ => unreachable!(), } @@ -271,7 +269,7 @@ where Ok(()) } - fn poll_chunk_end( + fn poll_chunk_data_end( &mut self, cx: &mut Context<'_>, buf: &mut [u8], @@ -311,17 +309,17 @@ where // a small trick to speed up let next_char = cache[1]; reader.as_mut().consume(2); - self.check_chunk_end_last_char(next_char)?; + self.check_chunk_data_end_last_char(next_char)?; buf[1] = b'\n'; nw = 2; } else { reader.as_mut().consume(1); - self.next_read_type = NextReadType::ChunkEnd(b'\n'); + self.next_read_type = NextReadType::ChunkDataEnd(b'\n'); } } b'\n' => { reader.as_mut().consume(1); - self.update_next_read_type_after_chunk_end(); + self.update_next_read_type_after_chunk_data_end(); } _ => unreachable!(), } @@ -329,27 +327,19 @@ where Poll::Ready(Ok(nw)) } - fn check_chunk_end_last_char(&mut self, char: u8) -> io::Result<()> { + fn check_chunk_data_end_last_char(&mut self, char: u8) -> io::Result<()> { if char != b'\n' { return Err(io::Error::new( io::ErrorKind::InvalidData, "invalid chunk ending last char", )); } - self.update_next_read_type_after_chunk_end(); + self.update_next_read_type_after_chunk_data_end(); Ok(()) } - fn update_next_read_type_after_chunk_end(&mut self) { - self.next_read_type = if self.current_chunk_size == 0 { - match self.body_type { - HttpBodyType::ChunkedWithoutTrailer => NextReadType::EndOfFile, - HttpBodyType::ChunkedWithTrailer => NextReadType::Trailer, - _ => unreachable!(), - } - } else { - NextReadType::ChunkSize - }; + fn update_next_read_type_after_chunk_data_end(&mut self) { + self.next_read_type = NextReadType::ChunkSize; } fn poll_trailer( @@ -469,9 +459,9 @@ where .poll_fixed(cx, &mut inner_buf) .map_ok(|_| inner_buf.filled().len()) } - NextReadType::ChunkEnd(char) => { + NextReadType::ChunkDataEnd(char) => { self.as_mut() - .poll_chunk_end(cx, buf.initialize_unfilled(), char) + .poll_chunk_data_end(cx, buf.initialize_unfilled(), char) } NextReadType::Trailer => self.as_mut().poll_trailer(cx, buf.initialize_unfilled()), _ => unreachable!(), @@ -514,9 +504,7 @@ where NextReadType::FixedLength => self.poll_fixed(cx, buf), _ => unreachable!(), }, - HttpBodyType::ChunkedWithoutTrailer | HttpBodyType::ChunkedWithTrailer => { - self.poll_chunked(cx, buf) - } + HttpBodyType::Chunked => self.poll_chunked(cx, buf), } } } @@ -629,8 +617,7 @@ mod tests { let stream = tokio_stream::iter(vec![Result::Ok(Bytes::from_static(content))]); let stream = StreamReader::new(stream); let mut buf_stream = BufReader::new(stream); - let mut body_reader = - HttpBodyReader::new(&mut buf_stream, HttpBodyType::ChunkedWithoutTrailer, 1024); + let mut body_reader = HttpBodyReader::new(&mut buf_stream, HttpBodyType::Chunked, 1024); let mut buf = [0u8; 32]; let len = body_reader.read(&mut buf).await.unwrap(); @@ -650,8 +637,7 @@ mod tests { ]); let stream = StreamReader::new(stream); let mut buf_stream = BufReader::new(stream); - let mut body_reader = - HttpBodyReader::new(&mut buf_stream, HttpBodyType::ChunkedWithoutTrailer, 1024); + let mut body_reader = HttpBodyReader::new(&mut buf_stream, HttpBodyType::Chunked, 1024); let mut buf = [0u8; 32]; let len = body_reader.read(&mut buf).await.unwrap(); @@ -671,8 +657,7 @@ mod tests { let stream = tokio_stream::iter(vec![Result::Ok(Bytes::from_static(content))]); let stream = StreamReader::new(stream); let mut buf_stream = BufReader::new(stream); - let mut body_reader = - HttpBodyReader::new(&mut buf_stream, HttpBodyType::ChunkedWithTrailer, 1024); + let mut body_reader = HttpBodyReader::new(&mut buf_stream, HttpBodyType::Chunked, 1024); let mut buf = [0u8; 64]; let len = body_reader.read(&mut buf).await.unwrap(); diff --git a/lib/g3-http/src/client/response.rs b/lib/g3-http/src/client/response.rs index 87ce90b08..e89517376 100644 --- a/lib/g3-http/src/client/response.rs +++ b/lib/g3-http/src/client/response.rs @@ -40,7 +40,6 @@ pub struct HttpForwardRemoteResponse { keep_alive: bool, content_length: u64, chunked_transfer: bool, - chunked_with_trailer: bool, has_transfer_encoding: bool, has_content_length: bool, has_trailer: bool, @@ -61,7 +60,6 @@ impl HttpForwardRemoteResponse { keep_alive: false, content_length: 0, chunked_transfer: false, - chunked_with_trailer: false, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -75,7 +73,6 @@ impl HttpForwardRemoteResponse { for v in adapted.trailer() { hop_by_hop_headers.append(http::header::TRAILER, v.clone()); } - let chunked_with_trailer = !adapted.trailer().is_empty(); HttpForwardRemoteResponse { version: adapted.version, code: adapted.status.as_u16(), @@ -88,7 +85,6 @@ impl HttpForwardRemoteResponse { keep_alive: self.keep_alive, content_length: self.content_length, chunked_transfer: true, - chunked_with_trailer, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -124,11 +120,7 @@ impl HttpForwardRemoteResponse { if self.expect_no_body(method) { None } else if self.chunked_transfer { - if self.chunked_with_trailer { - Some(HttpBodyType::ChunkedWithTrailer) - } else { - Some(HttpBodyType::ChunkedWithoutTrailer) - } + Some(HttpBodyType::Chunked) } else if self.has_content_length { if self.content_length > 0 { Some(HttpBodyType::ContentLength(self.content_length)) @@ -213,14 +205,14 @@ impl HttpForwardRemoteResponse { } if self.expect_no_body(method) { - // ignore the check of content-length as no body is expected + // ignore the check of content-length as body is unexpected } else if !self.has_content_length { // read to end and close the connection self.keep_alive = false; } } - // Don't move non standard connection headers to hop-by-hop headers, as we don't support them + // Don't move non-standard connection headers to hop-by-hop headers, as we don't support them } fn build_from_status_line(line_buf: &[u8]) -> Result { @@ -302,9 +294,6 @@ impl HttpForwardRemoteResponse { } "trailer" => { self.has_trailer = true; - if self.chunked_transfer { - self.chunked_with_trailer = true; - } return self.insert_hop_by_hop_header(name, &header); } "transfer-encoding" => { @@ -320,9 +309,6 @@ impl HttpForwardRemoteResponse { let v = header.value.to_lowercase(); if v.ends_with("chunked") { self.chunked_transfer = true; - if self.has_trailer { - self.chunked_with_trailer = true; - } } else if v.contains("chunked") { return Err(HttpResponseParseError::InvalidChunkedTransferEncoding); } diff --git a/lib/g3-http/src/client/transparent.rs b/lib/g3-http/src/client/transparent.rs index f16a86edd..47ad22ed2 100644 --- a/lib/g3-http/src/client/transparent.rs +++ b/lib/g3-http/src/client/transparent.rs @@ -42,7 +42,6 @@ pub struct HttpTransparentResponse { pub upgrade: Option, content_length: u64, chunked_transfer: bool, - chunked_with_trailer: bool, has_transfer_encoding: bool, has_content_length: bool, has_trailer: bool, @@ -65,7 +64,6 @@ impl HttpTransparentResponse { upgrade: None, content_length: 0, chunked_transfer: false, - chunked_with_trailer: false, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -79,7 +77,6 @@ impl HttpTransparentResponse { for v in adapted.trailer() { hop_by_hop_headers.append(http::header::TRAILER, v.clone()); } - let chunked_with_trailer = !adapted.trailer().is_empty(); HttpTransparentResponse { version: adapted.version, code: adapted.status.as_u16(), @@ -94,7 +91,6 @@ impl HttpTransparentResponse { upgrade: self.upgrade.clone(), content_length: self.content_length, chunked_transfer: true, - chunked_with_trailer, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -126,11 +122,7 @@ impl HttpTransparentResponse { if self.expect_no_body(method) { None } else if self.chunked_transfer { - if self.chunked_with_trailer { - Some(HttpBodyType::ChunkedWithTrailer) - } else { - Some(HttpBodyType::ChunkedWithoutTrailer) - } + Some(HttpBodyType::Chunked) } else if self.has_content_length { if self.content_length > 0 { Some(HttpBodyType::ContentLength(self.content_length)) @@ -213,7 +205,7 @@ impl HttpTransparentResponse { } if self.expect_no_body(method) { - // ignore the check of content-length as no body is expected + // ignore the check of content-length as body is unexpected } else if !self.has_content_length { // read to end and close the connection self.keep_alive = false; @@ -225,7 +217,7 @@ impl HttpTransparentResponse { self.hop_by_hop_headers.remove(http::header::UPGRADE); } - // Don't move non standard connection headers to hop-by-hop headers, as we don't support them + // Don't move non-standard connection headers to hop-by-hop headers, as we don't support them } fn build_from_status_line(line_buf: &[u8]) -> Result { @@ -306,9 +298,6 @@ impl HttpTransparentResponse { } "trailer" => { self.has_trailer = true; - if self.chunked_transfer { - self.chunked_with_trailer = true; - } return self.insert_hop_by_hop_header(name, &header); } "transfer-encoding" => { @@ -322,9 +311,6 @@ impl HttpTransparentResponse { let v = header.value.to_lowercase(); if v.ends_with("chunked") { self.chunked_transfer = true; - if self.has_trailer { - self.chunked_with_trailer = true; - } } else if v.contains("chunked") { return Err(HttpResponseParseError::InvalidChunkedTransferEncoding); } diff --git a/lib/g3-http/src/connect/response.rs b/lib/g3-http/src/connect/response.rs index 7ad697f51..0befe2c34 100644 --- a/lib/g3-http/src/connect/response.rs +++ b/lib/g3-http/src/connect/response.rs @@ -31,7 +31,6 @@ pub struct HttpConnectResponse { pub headers: HttpHeaderMap, content_length: u64, chunked_transfer: bool, - chunked_with_trailer: bool, has_transfer_encoding: bool, has_content_length: bool, has_trailer: bool, @@ -45,7 +44,6 @@ impl HttpConnectResponse { headers: HttpHeaderMap::default(), content_length: 0, chunked_transfer: false, - chunked_with_trailer: false, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -54,11 +52,7 @@ impl HttpConnectResponse { fn body_type(&self) -> Option { if self.chunked_transfer { - if self.chunked_with_trailer { - Some(HttpBodyType::ChunkedWithTrailer) - } else { - Some(HttpBodyType::ChunkedWithoutTrailer) - } + Some(HttpBodyType::Chunked) } else if self.content_length > 0 { Some(HttpBodyType::ContentLength(self.content_length)) } else { @@ -122,9 +116,19 @@ impl HttpConnectResponse { rsp.parse_header_line(line_buf.as_ref())?; } + rsp.post_check_and_fix(); Ok(rsp) } + /// do some necessary check and fix + fn post_check_and_fix(&mut self) { + if self.has_trailer && !self.chunked_transfer { + self.headers.remove(http::header::TRAILER); + } + + // Don't move non-standard connection headers to hop-by-hop headers, as we don't support them + } + fn build_from_status_line(line_buf: &[u8]) -> Result { let rsp = HttpStatusLine::parse(line_buf).map_err(HttpConnectResponseError::InvalidStatusLine)?; @@ -144,12 +148,7 @@ impl HttpConnectResponse { match name.as_str() { "connection" | "proxy-connection" => {} - "trailer" => { - self.has_trailer = true; - if self.chunked_transfer { - self.chunked_with_trailer = true; - } - } + "trailer" => self.has_trailer = true, "transfer-encoding" => { self.has_transfer_encoding = true; if self.has_content_length { @@ -161,9 +160,6 @@ impl HttpConnectResponse { let v = header.value.to_lowercase(); if v.ends_with("chunked") { self.chunked_transfer = true; - if self.has_trailer { - self.chunked_with_trailer = true; - } } else if v.contains("chunked") { return Err(HttpConnectResponseError::InvalidChunkedTransferEncoding); } diff --git a/lib/g3-http/src/server/request.rs b/lib/g3-http/src/server/request.rs index 95e0d5cff..58943babe 100644 --- a/lib/g3-http/src/server/request.rs +++ b/lib/g3-http/src/server/request.rs @@ -43,7 +43,6 @@ pub struct HttpProxyClientRequest { keep_alive: bool, content_length: u64, chunked_transfer: bool, - chunked_with_trailer: bool, has_transfer_encoding: bool, has_content_length: bool, has_trailer: bool, @@ -65,7 +64,6 @@ impl HttpProxyClientRequest { keep_alive: false, content_length: 0, chunked_transfer: false, - chunked_with_trailer: false, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -74,11 +72,10 @@ impl HttpProxyClientRequest { pub fn clone_by_adaptation(&self, adapted: HttpAdaptedRequest) -> Self { let mut hop_by_hop_headers = self.hop_by_hop_headers.clone(); - hop_by_hop_headers.remove(http::header::TRAILER); + hop_by_hop_headers.remove(header::TRAILER); for v in adapted.trailer() { - hop_by_hop_headers.append(http::header::TRAILER, v.clone()); + hop_by_hop_headers.append(header::TRAILER, v.clone()); } - let chunked_with_trailer = !adapted.trailer().is_empty(); HttpProxyClientRequest { version: adapted.version, method: adapted.method, @@ -93,7 +90,6 @@ impl HttpProxyClientRequest { keep_alive: self.keep_alive, content_length: self.content_length, chunked_transfer: true, - chunked_with_trailer, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -117,11 +113,7 @@ impl HttpProxyClientRequest { pub fn body_type(&self) -> Option { if self.chunked_transfer { - if self.chunked_with_trailer { - Some(HttpBodyType::ChunkedWithTrailer) - } else { - Some(HttpBodyType::ChunkedWithoutTrailer) - } + Some(HttpBodyType::Chunked) } else if self.content_length > 0 { Some(HttpBodyType::ContentLength(self.content_length)) } else { @@ -238,7 +230,7 @@ impl HttpProxyClientRequest { self.hop_by_hop_headers.remove(header::TRAILER); } - // Don't move non standard connection headers to hop-by-hop headers, as we don't support them + // Don't move non-standard connection headers to hop-by-hop headers, as we don't support them } fn build_from_method_line(line_buf: &[u8]) -> Result { @@ -373,9 +365,6 @@ impl HttpProxyClientRequest { } "trailer" => { self.has_trailer = true; - if self.chunked_transfer { - self.chunked_with_trailer = true; - } return self.insert_hop_by_hop_header(name, &header); } "transfer-encoding" => { @@ -391,9 +380,6 @@ impl HttpProxyClientRequest { let v = header.value.to_lowercase(); if v.ends_with("chunked") { self.chunked_transfer = true; - if self.has_trailer { - self.chunked_with_trailer = true; - } } else { return Err(HttpRequestParseError::InvalidChunkedTransferEncoding); } diff --git a/lib/g3-http/src/server/transparent.rs b/lib/g3-http/src/server/transparent.rs index 613c35854..b25f13b32 100644 --- a/lib/g3-http/src/server/transparent.rs +++ b/lib/g3-http/src/server/transparent.rs @@ -44,7 +44,6 @@ pub struct HttpTransparentRequest { pub upgrade: bool, content_length: u64, chunked_transfer: bool, - chunked_with_trailer: bool, has_transfer_encoding: bool, has_content_length: bool, has_trailer: bool, @@ -67,7 +66,6 @@ impl HttpTransparentRequest { upgrade: false, content_length: 0, chunked_transfer: false, - chunked_with_trailer: false, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -80,7 +78,6 @@ impl HttpTransparentRequest { for v in adapted.trailer() { hop_by_hop_headers.append(http::header::TRAILER, v.clone()); } - let chunked_with_trailer = !adapted.trailer().is_empty(); HttpTransparentRequest { version: adapted.version, method: adapted.method, @@ -96,7 +93,6 @@ impl HttpTransparentRequest { upgrade: self.upgrade, content_length: self.content_length, chunked_transfer: true, - chunked_with_trailer, has_transfer_encoding: false, has_content_length: false, has_trailer: false, @@ -115,11 +111,7 @@ impl HttpTransparentRequest { pub fn body_type(&self) -> Option { if self.chunked_transfer { - if self.chunked_with_trailer { - Some(HttpBodyType::ChunkedWithTrailer) - } else { - Some(HttpBodyType::ChunkedWithoutTrailer) - } + Some(HttpBodyType::Chunked) } else if self.content_length > 0 { Some(HttpBodyType::ContentLength(self.content_length)) } else { @@ -219,7 +211,7 @@ impl HttpTransparentRequest { self.hop_by_hop_headers.remove(http::header::TRAILER); } - // Don't move non standard connection headers to hop-by-hop headers, as we don't support them + // Don't move non-standard connection headers to hop-by-hop headers, as we don't support them } fn build_from_method_line(line_buf: &[u8]) -> Result { @@ -329,9 +321,6 @@ impl HttpTransparentRequest { } "trailer" => { self.has_trailer = true; - if self.chunked_transfer { - self.chunked_with_trailer = true; - } return self.insert_hop_by_hop_header(name, &header); } "transfer-encoding" => { @@ -345,9 +334,6 @@ impl HttpTransparentRequest { let v = header.value.to_lowercase(); if v.ends_with("chunked") { self.chunked_transfer = true; - if self.has_trailer { - self.chunked_with_trailer = true; - } } else { return Err(HttpRequestParseError::InvalidChunkedTransferEncoding); } diff --git a/lib/g3-icap-client/src/reqmod/h1/bidirectional.rs b/lib/g3-icap-client/src/reqmod/h1/bidirectional.rs index 3da044831..6685d1084 100644 --- a/lib/g3-icap-client/src/reqmod/h1/bidirectional.rs +++ b/lib/g3-icap-client/src/reqmod/h1/bidirectional.rs @@ -145,11 +145,8 @@ impl<'a, I: IdleCheck> BidirectionalRecvHttpRequest<'a, I> { .await?; http_req.set_chunked_encoding(); let trailers = self.icap_rsp.take_trailers(); - let body_type = if !trailers.is_empty() { + if !trailers.is_empty() { http_req.set_trailer(trailers); - HttpBodyType::ChunkedWithTrailer - } else { - HttpBodyType::ChunkedWithoutTrailer }; let final_req = orig_http_request.adapt_to(http_req); @@ -159,8 +156,11 @@ impl<'a, I: IdleCheck> BidirectionalRecvHttpRequest<'a, I> { .map_err(H1ReqmodAdaptationError::HttpUpstreamWriteFailed)?; state.mark_ups_send_header(); - let mut ups_body_reader = - HttpBodyReader::new(self.icap_reader, body_type, self.http_body_line_max_size); + let mut ups_body_reader = HttpBodyReader::new( + self.icap_reader, + HttpBodyType::Chunked, + self.http_body_line_max_size, + ); let mut ups_body_transfer = LimitedCopy::new(&mut ups_body_reader, ups_writer, &self.copy_config); diff --git a/lib/g3-icap-client/src/reqmod/h1/forward_body.rs b/lib/g3-icap-client/src/reqmod/h1/forward_body.rs index 7ee999b89..acaf09c6b 100644 --- a/lib/g3-icap-client/src/reqmod/h1/forward_body.rs +++ b/lib/g3-icap-client/src/reqmod/h1/forward_body.rs @@ -30,12 +30,7 @@ use super::{ use crate::reqmod::IcapReqmodResponsePayload; impl HttpRequestAdapter { - fn build_forward_all_request( - &self, - http_request: &H, - http_header_len: usize, - http_body_type: HttpBodyType, - ) -> Vec + fn build_forward_all_request(&self, http_request: &H, http_header_len: usize) -> Vec where H: HttpRequestForAdaptation, { @@ -46,9 +41,7 @@ impl HttpRequestAdapter { header, "Encapsulated: req-hdr=0, req-body={http_header_len}\r\n", ); - if http_body_type == HttpBodyType::ChunkedWithTrailer { - http_request.append_trailer_header(&mut header); - } + http_request.append_trailer_header(&mut header); header.put_slice(b"\r\n"); header } @@ -67,8 +60,7 @@ impl HttpRequestAdapter { UW: HttpRequestUpstreamWriter + Unpin, { let http_header = http_request.serialize_for_adapter(); - let icap_header = - self.build_forward_all_request(http_request, http_header.len(), clt_body_type); + let icap_header = self.build_forward_all_request(http_request, http_header.len()); let icap_w = &mut self.icap_connection.0; icap_w diff --git a/lib/g3-icap-client/src/reqmod/h1/mod.rs b/lib/g3-icap-client/src/reqmod/h1/mod.rs index 50d210a8a..968fd17d2 100644 --- a/lib/g3-icap-client/src/reqmod/h1/mod.rs +++ b/lib/g3-icap-client/src/reqmod/h1/mod.rs @@ -214,17 +214,11 @@ pub struct ReqmodRecvHttpResponseBody { icap_client: Arc, icap_keepalive: bool, icap_connection: IcapClientConnection, - has_trailer: bool, } impl ReqmodRecvHttpResponseBody { pub fn body_reader(&mut self) -> HttpBodyReader<'_, impl AsyncBufRead> { - let body_type = if self.has_trailer { - HttpBodyType::ChunkedWithTrailer - } else { - HttpBodyType::ChunkedWithoutTrailer - }; - HttpBodyReader::new(&mut self.icap_connection.1, body_type, 1024) + HttpBodyReader::new(&mut self.icap_connection.1, HttpBodyType::Chunked, 1024) } pub async fn save_connection(self) { diff --git a/lib/g3-icap-client/src/reqmod/h1/preview.rs b/lib/g3-icap-client/src/reqmod/h1/preview.rs index 150981159..bb0231e56 100644 --- a/lib/g3-icap-client/src/reqmod/h1/preview.rs +++ b/lib/g3-icap-client/src/reqmod/h1/preview.rs @@ -35,7 +35,6 @@ impl HttpRequestAdapter { &self, http_request: &H, http_header_len: usize, - http_body_type: HttpBodyType, preview_state: &PreviewDataState, ) -> Vec where @@ -55,9 +54,7 @@ impl HttpRequestAdapter { "Encapsulated: req-hdr=0, req-body={}\r\nPreview: {}\r\n", http_header_len, preview_state.preview_size ); - if http_body_type == HttpBodyType::ChunkedWithTrailer { - http_request.append_trailer_header(&mut header); - } + http_request.append_trailer_header(&mut header); header.put_slice(b"\r\n"); header } @@ -104,8 +101,7 @@ impl HttpRequestAdapter { .await } }; - let icap_header = - self.build_preview_request(http_request, header_len, clt_body_type, &preview_state); + let icap_header = self.build_preview_request(http_request, header_len, &preview_state); let icap_w = &mut self.icap_connection.0; icap_w diff --git a/lib/g3-icap-client/src/reqmod/h1/recv_request.rs b/lib/g3-icap-client/src/reqmod/h1/recv_request.rs index 05195678d..4b2187d5c 100644 --- a/lib/g3-icap-client/src/reqmod/h1/recv_request.rs +++ b/lib/g3-icap-client/src/reqmod/h1/recv_request.rs @@ -209,11 +209,8 @@ impl HttpRequestAdapter { .await?; http_req.set_chunked_encoding(); let trailers = icap_rsp.take_trailers(); - let body_type = if !trailers.is_empty() { + if !trailers.is_empty() { http_req.set_trailer(trailers); - HttpBodyType::ChunkedWithTrailer - } else { - HttpBodyType::ChunkedWithoutTrailer }; let final_req = orig_http_request.adapt_to(http_req); @@ -225,7 +222,7 @@ impl HttpRequestAdapter { let mut body_reader = HttpBodyReader::new( &mut self.icap_connection.1, - body_type, + HttpBodyType::Chunked, self.http_body_line_max_size, ); let mut body_copy = LimitedCopy::new(&mut body_reader, ups_writer, &self.copy_config); diff --git a/lib/g3-icap-client/src/reqmod/h1/recv_response.rs b/lib/g3-icap-client/src/reqmod/h1/recv_response.rs index 05c727e8c..7ad6b294d 100644 --- a/lib/g3-icap-client/src/reqmod/h1/recv_response.rs +++ b/lib/g3-icap-client/src/reqmod/h1/recv_response.rs @@ -50,17 +50,13 @@ impl HttpRequestAdapter { HttpAdapterErrorResponse::parse(&mut self.icap_connection.1, http_header_size).await?; http_rsp.set_chunked_encoding(); let trailers = icap_rsp.take_trailers(); - let has_trailer = if trailers.is_empty() { - false - } else { + if !trailers.is_empty() { http_rsp.set_trailer(trailers); - true }; let recv_body = ReqmodRecvHttpResponseBody { icap_client: self.icap_client, icap_keepalive: icap_rsp.keep_alive, icap_connection: self.icap_connection, - has_trailer, }; Ok((http_rsp, recv_body)) } diff --git a/lib/g3-icap-client/src/respmod/h1/bidirectional.rs b/lib/g3-icap-client/src/respmod/h1/bidirectional.rs index 88cc5795d..76ba747ee 100644 --- a/lib/g3-icap-client/src/respmod/h1/bidirectional.rs +++ b/lib/g3-icap-client/src/respmod/h1/bidirectional.rs @@ -138,11 +138,8 @@ impl<'a, I: IdleCheck> BidirectionalRecvHttpResponse<'a, I> { let mut http_rsp = HttpAdaptedResponse::parse(self.icap_reader, http_header_size).await?; http_rsp.set_chunked_encoding(); let trailers = self.icap_rsp.take_trailers(); - let body_type = if !trailers.is_empty() { + if !trailers.is_empty() { http_rsp.set_trailer(trailers); - HttpBodyType::ChunkedWithTrailer - } else { - HttpBodyType::ChunkedWithoutTrailer }; let final_rsp = orig_http_response.adapt_to(http_rsp); @@ -153,8 +150,11 @@ impl<'a, I: IdleCheck> BidirectionalRecvHttpResponse<'a, I> { .map_err(H1RespmodAdaptationError::HttpClientWriteFailed)?; state.mark_clt_send_header(); - let mut adp_body_reader = - HttpBodyReader::new(self.icap_reader, body_type, self.http_body_line_max_size); + let mut adp_body_reader = HttpBodyReader::new( + self.icap_reader, + HttpBodyType::Chunked, + self.http_body_line_max_size, + ); let mut adp_body_transfer = LimitedCopy::new(&mut adp_body_reader, clt_writer, &self.copy_config); diff --git a/lib/g3-icap-client/src/respmod/h1/forward_body.rs b/lib/g3-icap-client/src/respmod/h1/forward_body.rs index b86a9f04c..b22144e23 100644 --- a/lib/g3-icap-client/src/respmod/h1/forward_body.rs +++ b/lib/g3-icap-client/src/respmod/h1/forward_body.rs @@ -36,7 +36,6 @@ impl HttpResponseAdapter { http_req_hdr_len: usize, http_response: &H, http_rsp_hdr_len: usize, - http_body_type: HttpBodyType, ) -> Vec where H: HttpResponseForAdaptation, @@ -49,9 +48,7 @@ impl HttpResponseAdapter { "Encapsulated: req-hdr=0, res-hdr={http_req_hdr_len}, res-body={}\r\n", http_req_hdr_len + http_rsp_hdr_len ); - if http_body_type == HttpBodyType::ChunkedWithTrailer { - http_response.append_trailer_header(&mut header); - } + http_response.append_trailer_header(&mut header); header.put_slice(b"\r\n"); header } @@ -77,7 +74,6 @@ impl HttpResponseAdapter { http_req_header.len(), http_response, http_rsp_header.len(), - ups_body_type, ); let icap_w = &mut self.icap_connection.0; diff --git a/lib/g3-icap-client/src/respmod/h1/preview.rs b/lib/g3-icap-client/src/respmod/h1/preview.rs index 155b6c739..397f25cd7 100644 --- a/lib/g3-icap-client/src/respmod/h1/preview.rs +++ b/lib/g3-icap-client/src/respmod/h1/preview.rs @@ -37,7 +37,6 @@ impl HttpResponseAdapter { http_req_hdr_len: usize, http_response: &H, http_rsp_hdr_len: usize, - http_body_type: HttpBodyType, preview_state: &PreviewDataState, ) -> Vec where @@ -58,9 +57,7 @@ impl HttpResponseAdapter { http_req_hdr_len + http_rsp_hdr_len, preview_state.preview_size ); - if http_body_type == HttpBodyType::ChunkedWithTrailer { - http_response.append_trailer_header(&mut header); - } + http_response.append_trailer_header(&mut header); header.put_slice(b"\r\n"); header } @@ -116,7 +113,6 @@ impl HttpResponseAdapter { http_req_header.len(), http_response, http_rsp_hdr_len, - ups_body_type, &preview_state, ); diff --git a/lib/g3-icap-client/src/respmod/h1/recv_response.rs b/lib/g3-icap-client/src/respmod/h1/recv_response.rs index 05203fb9d..62c17439d 100644 --- a/lib/g3-icap-client/src/respmod/h1/recv_response.rs +++ b/lib/g3-icap-client/src/respmod/h1/recv_response.rs @@ -197,11 +197,8 @@ impl HttpResponseAdapter { HttpAdaptedResponse::parse(&mut self.icap_connection.1, http_header_size).await?; http_rsp.set_chunked_encoding(); let trailers = icap_rsp.take_trailers(); - let body_type = if !trailers.is_empty() { + if !trailers.is_empty() { http_rsp.set_trailer(trailers); - HttpBodyType::ChunkedWithTrailer - } else { - HttpBodyType::ChunkedWithoutTrailer }; let final_rsp = orig_http_response.adapt_to(http_rsp); @@ -214,7 +211,7 @@ impl HttpResponseAdapter { let mut body_reader = HttpBodyReader::new( &mut self.icap_connection.1, - body_type, + HttpBodyType::Chunked, self.http_body_line_max_size, ); let mut body_copy = LimitedCopy::new(&mut body_reader, clt_writer, &self.copy_config);