From: Philippe Antoine Date: Wed, 27 Mar 2024 13:33:54 +0000 (+0100) Subject: http2: use a reference counter for headers X-Git-Tag: suricata-6.0.19~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=08d93f7c3762781b743f88f9fdc4389eb9c3eb64;p=thirdparty%2Fsuricata.git http2: use a reference counter for headers Ticket: 6892 As HTTP hpack header compression allows one single byte to express a previously seen arbitrary-size header block (name+value) we should avoid to copy the vectors data, but just point to the same data, while reamining memory safe, even in the case of later headers eviction from the dybnamic table. Rust std solution is Rc, and the use of clone, so long as the data is accessed by only one thread. (cherry picked from commit 390f09692eb99809c679d3f350c7cc185d163e1a) --- diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs index 0b663cd342..ddc3d4a452 100644 --- a/rust/src/http2/detect.rs +++ b/rust/src/http2/detect.rs @@ -23,6 +23,7 @@ use crate::core::{STREAM_TOCLIENT, STREAM_TOSERVER}; use std::ffi::CStr; use std::mem::transmute; use std::str::FromStr; +use std::rc::Rc; fn http2_tx_has_frametype( tx: &mut HTTP2Transaction, direction: u8, value: u8, @@ -489,7 +490,7 @@ fn http2_frames_get_header_firstvalue<'a>( for i in 0..frames.len() { if let Some(blocks) = http2_header_blocks(&frames[i]) { for block in blocks.iter() { - if block.name == name.as_bytes() { + if block.name.as_ref() == name.as_bytes() { return Ok(&block.value); } } @@ -512,7 +513,7 @@ fn http2_frames_get_header_value<'a>( for i in 0..frames.len() { if let Some(blocks) = http2_header_blocks(&frames[i]) { for block in blocks.iter() { - if block.name == name.as_bytes() { + if block.name.as_ref() == name.as_bytes() { if found == 0 { single = Ok(&block.value); found = 1; @@ -899,8 +900,8 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) { }; let mut blocks = Vec::new(); let b = parser::HTTP2FrameHeaderBlock { - name: name.to_vec(), - value: input.to_vec(), + name: Rc::new(name.to_vec()), + value: Rc::new(input.to_vec()), error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }; @@ -1153,15 +1154,15 @@ mod tests { }; let mut blocks = Vec::new(); let b = parser::HTTP2FrameHeaderBlock { - name: "Host".as_bytes().to_vec(), - value: "abc.com".as_bytes().to_vec(), + name: "Host".as_bytes().to_vec().into(), + value: "abc.com".as_bytes().to_vec().into(), error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }; blocks.push(b); let b2 = parser::HTTP2FrameHeaderBlock { - name: "Host".as_bytes().to_vec(), - value: "efg.net".as_bytes().to_vec(), + name: "Host".as_bytes().to_vec().into(), + value: "efg.net".as_bytes().to_vec().into(), error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }; diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs index 0590f796e3..716cacd5c6 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -190,7 +190,7 @@ impl HTTP2Transaction { let mut authority = None; let mut host = None; for block in blocks { - if block.name == b"content-encoding" { + if block.name.as_ref() == b"content-encoding" { #[cfg(feature = "decompression")] self.decoder.http2_encoding_fromvec(&block.value, _dir); } else if block.name.eq_ignore_ascii_case(b":authority") { diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs index 2d4f363bfa..0a073679f4 100644 --- a/rust/src/http2/parser.rs +++ b/rust/src/http2/parser.rs @@ -25,6 +25,7 @@ use nom::Err; use nom::IResult; use std::fmt; use std::str::FromStr; +use std::rc::Rc; #[repr(u8)] #[derive(Clone, Copy, PartialEq, FromPrimitive, Debug)] @@ -281,8 +282,8 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option 0 { return Some(HTTP2FrameHeaderBlock { - name: name.as_bytes().to_vec(), - value: value.as_bytes().to_vec(), + name: Rc::new(name.as_bytes().to_vec()), + value: Rc::new(value.as_bytes().to_vec()), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }); @@ -290,23 +291,23 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option, - pub value: Vec, + // Use Rc reference counted so that indexed headers do not get copied. + // Otherwise, this leads to quadratic complexity in memory occupation. + pub name: Rc>, + pub value: Rc>, pub error: HTTP2HeaderDecodeStatus, pub sizeupdate: u64, } @@ -386,7 +389,7 @@ fn http2_parse_headers_block_literal_common<'a>( ) -> IResult<&'a [u8], HTTP2FrameHeaderBlock> { let (i3, name, error) = if index == 0 { match http2_parse_headers_block_string(input) { - Ok((r, n)) => Ok((r, n, HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), + Ok((r, n)) => Ok((r, Rc::new(n), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), Err(e) => Err(e), } } else { @@ -398,7 +401,7 @@ fn http2_parse_headers_block_literal_common<'a>( )), None => Ok(( input, - Vec::new(), + Rc::new(Vec::new()), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed, )), } @@ -408,7 +411,7 @@ fn http2_parse_headers_block_literal_common<'a>( i4, HTTP2FrameHeaderBlock { name, - value, + value: Rc::new(value), error, sizeupdate: 0, }, @@ -436,8 +439,8 @@ fn http2_parse_headers_block_literal_incindex<'a>( match r { Ok((r, head)) => { let headcopy = HTTP2FrameHeaderBlock { - name: head.name.to_vec(), - value: head.value.to_vec(), + name: head.name.clone(), + value: head.value.clone(), error: head.error, sizeupdate: 0, }; @@ -557,8 +560,8 @@ fn http2_parse_headers_block_dynamic_size<'a>( return Ok(( i3, HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), + name: Vec::new().into(), + value: Vec::new().into(), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, sizeupdate: 0, }, @@ -581,8 +584,8 @@ fn http2_parse_headers_block_dynamic_size<'a>( return Ok(( i3, HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), + name: Rc::new(Vec::new()), + value: Rc::new(Vec::new()), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSizeUpdate, sizeupdate: maxsize2, }, @@ -936,8 +939,8 @@ mod tests { match r0 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":method".as_bytes().to_vec()); - assert_eq!(hd.value, "GET".as_bytes().to_vec()); + assert_eq!(hd.name, ":method".as_bytes().to_vec().into()); + assert_eq!(hd.value, "GET".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); } @@ -953,8 +956,8 @@ mod tests { match r1 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, "accept".as_bytes().to_vec()); - assert_eq!(hd.value, "*/*".as_bytes().to_vec()); + assert_eq!(hd.name, "accept".as_bytes().to_vec().into()); + assert_eq!(hd.value, "*/*".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 1); @@ -973,8 +976,8 @@ mod tests { match result { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":authority".as_bytes().to_vec()); - assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); + assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); + assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2); @@ -991,8 +994,8 @@ mod tests { match r3 { Ok((remainder, hd)) => { // same as before - assert_eq!(hd.name, ":authority".as_bytes().to_vec()); - assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); + assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); + assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2); @@ -1027,8 +1030,8 @@ mod tests { match r2 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":path".as_bytes().to_vec()); - assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec()); + assert_eq!(hd.name, ":path".as_bytes().to_vec().into()); + assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2);