Skip to content

Commit

Permalink
http2: use a reference counter for headers
Browse files Browse the repository at this point in the history 
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 390f096)
  • Loading branch information
@catenacyber @victorjulien
catenacyber authored and victorjulien committed Apr 22, 2024
1 parent 311002b commit e68ec4b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 39 deletions.
19 changes: 10 additions & 9 deletions rust/src/http2/detect.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
};
Expand Down
2 changes: 1 addition & 1 deletion rust/src/http2/http2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
61 changes: 32 additions & 29 deletions rust/src/http2/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -295,32 +296,32 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option<HTTP
};
if !name.is_empty() {
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,
});
} else {
//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,
};
Expand Down Expand Up @@ -348,8 +349,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,
}
Expand Down Expand Up @@ -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 {
Expand All @@ -403,7 +406,7 @@ fn http2_parse_headers_block_literal_common<'a>(
)),
None => Ok((
input,
Vec::new(),
Rc::new(Vec::new()),
HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed,
)),
}
Expand All @@ -413,7 +416,7 @@ fn http2_parse_headers_block_literal_common<'a>(
i4,
HTTP2FrameHeaderBlock {
name,
value,
value: Rc::new(value),
error,
sizeupdate: 0,
},
Expand All @@ -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,
};
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit e68ec4b

Please sign in to comment.