From e68ec4b227d19498f364a41eb25d3182f0383ca5 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Wed, 27 Mar 2024 14:33:54 +0100 Subject: [PATCH] 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) --- rust/src/http2/detect.rs | 19 +++++++------ rust/src/http2/http2.rs | 2 +- rust/src/http2/parser.rs | 61 +++++++++++++++++++++------------------- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs index 52b4119055..67933b6c24 100644 --- a/rust/src/http2/detect.rs +++ b/rust/src/http2/detect.rs @@ -23,6 +23,7 @@ use crate::core::Direction; use crate::detect::uint::{detect_match_uint, DetectUintData}; use std::ffi::CStr; use std::str::FromStr; +use std::rc::Rc; fn http2_tx_has_frametype( tx: &mut HTTP2Transaction, direction: Direction, value: u8, @@ -404,7 +405,7 @@ fn http2_frames_get_header_firstvalue<'a>( for frame in frames { if let Some(blocks) = http2_header_blocks(frame) { for block in blocks.iter() { - if block.name == name.as_bytes() { + if block.name.as_ref() == name.as_bytes() { return Ok(&block.value); } } @@ -428,7 +429,7 @@ pub fn http2_frames_get_header_value_vec( for frame in frames { if let Some(blocks) = http2_header_blocks(frame) { for block in blocks.iter() { - if block.name == name.as_bytes() { + if block.name.as_ref() == name.as_bytes() { if found == 0 { vec.extend_from_slice(&block.value); found = 1; @@ -465,7 +466,7 @@ fn http2_frames_get_header_value<'a>( for frame in frames { if let Some(blocks) = http2_header_blocks(frame) { 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; @@ -920,8 +921,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, }; @@ -1063,15 +1064,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 b62ccb9850..23aaf261ff 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -208,7 +208,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" { self.decoder.http2_encoding_fromvec(&block.value, dir); } else if block.name.eq_ignore_ascii_case(b":authority") { authority = Some(&block.value); diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs index adabeb28c6..1a46437d65 100644 --- a/rust/src/http2/parser.rs +++ b/rust/src/http2/parser.rs @@ -30,6 +30,7 @@ use nom7::sequence::tuple; use nom7::{Err, IResult}; use std::fmt; use std::str::FromStr; +use std::rc::Rc; #[repr(u8)] #[derive(Clone, Copy, PartialEq, Eq, FromPrimitive, Debug)] @@ -295,8 +296,8 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option 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, } @@ -391,7 +394,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 { @@ -403,7 +406,7 @@ fn http2_parse_headers_block_literal_common<'a>( )), None => Ok(( input, - Vec::new(), + Rc::new(Vec::new()), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed, )), } @@ -413,7 +416,7 @@ fn http2_parse_headers_block_literal_common<'a>( i4, HTTP2FrameHeaderBlock { name, - value, + value: Rc::new(value), error, sizeupdate: 0, }, @@ -435,8 +438,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, }; @@ -556,8 +559,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, }, @@ -614,8 +617,8 @@ fn http2_parse_headers_blocks<'a>( // if we error from http2_parse_var_uint, we keep the first parsed headers if err.code == ErrorKind::LengthValue { blocks.push(HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), + name: Rc::new(Vec::new()), + value: Rc::new(Vec::new()), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, sizeupdate: 0, }); @@ -765,8 +768,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); } @@ -782,8 +785,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); @@ -802,8 +805,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); @@ -820,8 +823,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); @@ -856,8 +859,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); -- 2.47.2