]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
http2: use a reference counter for headers
authorPhilippe Antoine <pantoine@oisf.net>
Wed, 27 Mar 2024 13:33:54 +0000 (14:33 +0100)
committerVictor Julien <vjulien@oisf.net>
Mon, 22 Apr 2024 07:22:35 +0000 (09:22 +0200)
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
rust/src/http2/http2.rs
rust/src/http2/parser.rs

index 0b663cd3421832c1a583a969cd31fd8a210d2410..ddc3d4a452253e9cb3e6b7ed8723f17939a2c3e9 100644 (file)
@@ -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,
         };
index 0590f796e322e1950c2134f1874ad1233d378fe3..716cacd5c639fcbb3de510fb1fe35cb6de275363 100644 (file)
@@ -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") {
index 2d4f363bfaf8ade73fe740e09f81b575d2565ca7..0a073679f431ae35bd08bc9d8908b29c763e6b47 100644 (file)
@@ -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<HTTP
     };
     if name.len() > 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<HTTP
         //use dynamic table
         if n == 0 {
             return Some(HTTP2FrameHeaderBlock {
-                name: Vec::new(),
-                value: Vec::new(),
+                name: Rc::new(Vec::new()),
+                value: Rc::new(Vec::new()),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIndex0,
                 sizeupdate: 0,
             });
         } else if dyn_headers.table.len() + HTTP2_STATIC_HEADERS_NUMBER < n as usize {
             return Some(HTTP2FrameHeaderBlock {
-                name: Vec::new(),
-                value: Vec::new(),
+                name: Rc::new(Vec::new()),
+                value: Rc::new(Vec::new()),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed,
                 sizeupdate: 0,
             });
         } else {
             let indyn = dyn_headers.table.len() - (n as usize - HTTP2_STATIC_HEADERS_NUMBER);
             let headcopy = HTTP2FrameHeaderBlock {
-                name: dyn_headers.table[indyn].name.to_vec(),
-                value: dyn_headers.table[indyn].value.to_vec(),
+                name: dyn_headers.table[indyn].name.clone(),
+                value: dyn_headers.table[indyn].value.clone(),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess,
                 sizeupdate: 0,
             };
@@ -334,8 +335,10 @@ impl fmt::Display for HTTP2HeaderDecodeStatus {
 
 #[derive(Clone, Debug)]
 pub struct HTTP2FrameHeaderBlock {
-    pub name: Vec<u8>,
-    pub value: Vec<u8>,
+    // Use Rc reference counted so that indexed headers do not get copied.
+    // Otherwise, this leads to quadratic complexity in memory occupation.
+    pub name: Rc<Vec<u8>>,
+    pub value: Rc<Vec<u8>>,
     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);