Skip to content

Commit

Permalink
refactor(http1): use MaybeUninit for parsing with uninitialized heade…
Browse files Browse the repository at this point in the history
…rs (#2545)

Closes #2532
  • Loading branch information
Soveu authored Aug 19, 2021
1 parent be08648 commit 95a9783
Showing 1 changed file with 38 additions and 17 deletions.
55 changes: 38 additions & 17 deletions src/proto/h1/role.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
// `mem::uninitialized` replaced with `mem::MaybeUninit`,
// can't upgrade yet
#![allow(deprecated)]

use std::fmt::{self, Write};
use std::mem;
use std::mem::{self, MaybeUninit};

#[cfg(any(test, feature = "server", feature = "ffi"))]
use bytes::Bytes;
Expand Down Expand Up @@ -115,17 +111,23 @@ impl Http1Transaction for Server {
// but we *never* read any of it until after httparse has assigned
// values into it. By not zeroing out the stack memory, this saves
// a good ~5% on pipeline benchmarks.
let mut headers_indices: [HeaderIndices; MAX_HEADERS] = unsafe { mem::uninitialized() };
let mut headers_indices: [MaybeUninit<HeaderIndices>; MAX_HEADERS] = unsafe {
// SAFETY: We can go safely from MaybeUninit array to array of MaybeUninit
MaybeUninit::uninit().assume_init()
};
{
let mut headers: [httparse::Header<'_>; MAX_HEADERS] = unsafe { mem::uninitialized() };
/* SAFETY: it is safe to go from MaybeUninit array to array of MaybeUninit */
let mut headers: [MaybeUninit<httparse::Header<'_>>; MAX_HEADERS] = unsafe {
MaybeUninit::uninit().assume_init()
};
trace!(
"Request.parse([Header; {}], [u8; {}])",
headers.len(),
buf.len()
);
let mut req = httparse::Request::new(&mut headers);
let mut req = httparse::Request::new(&mut []);
let bytes = buf.as_ref();
match req.parse(bytes) {
match req.parse_with_uninit_headers(bytes, &mut headers) {
Ok(httparse::Status::Complete(parsed_len)) => {
trace!("Request.parse Complete({})", parsed_len);
len = parsed_len;
Expand Down Expand Up @@ -194,6 +196,8 @@ impl Http1Transaction for Server {
headers.reserve(headers_len);

for header in &headers_indices[..headers_len] {
// SAFETY: array is valid up to `headers_len`
let header = unsafe { &*header.as_ptr() };
let name = header_name!(&slice[header.name.0..header.name.1]);
let value = header_value!(slice.slice(header.value.0..header.value.1));

Expand Down Expand Up @@ -867,18 +871,24 @@ impl Http1Transaction for Client {
// Loop to skip information status code headers (100 Continue, etc).
loop {
// Unsafe: see comment in Server Http1Transaction, above.
let mut headers_indices: [HeaderIndices; MAX_HEADERS] = unsafe { mem::uninitialized() };
let mut headers_indices: [MaybeUninit<HeaderIndices>; MAX_HEADERS] = unsafe {
// SAFETY: We can go safely from MaybeUninit array to array of MaybeUninit
MaybeUninit::uninit().assume_init()
};
let (len, status, reason, version, headers_len) = {
let mut headers: [httparse::Header<'_>; MAX_HEADERS] =
unsafe { mem::uninitialized() };
// SAFETY: We can go safely from MaybeUninit array to array of MaybeUninit
let mut headers: [MaybeUninit<httparse::Header<'_>>; MAX_HEADERS] =
unsafe { MaybeUninit::uninit().assume_init() };
trace!(
"Response.parse([Header; {}], [u8; {}])",
headers.len(),
buf.len()
);
let mut res = httparse::Response::new(&mut headers);
let mut res = httparse::Response::new(&mut []);
let bytes = buf.as_ref();
match ctx.h1_parser_config.parse_response(&mut res, bytes) {
match ctx.h1_parser_config
.parse_response_with_uninit_headers(&mut res, bytes, &mut headers)
{
Ok(httparse::Status::Complete(len)) => {
trace!("Response.parse Complete({})", len);
let status = StatusCode::from_u16(res.code.unwrap())?;
Expand Down Expand Up @@ -934,6 +944,8 @@ impl Http1Transaction for Client {

headers.reserve(headers_len);
for header in &headers_indices[..headers_len] {
// SAFETY: array is valid up to `headers_len`
let header = unsafe { &*header.as_ptr() };
let name = header_name!(&slice[header.name.0..header.name.1]);
let value = header_value!(slice.slice(header.value.0..header.value.1));

Expand Down Expand Up @@ -1288,7 +1300,7 @@ struct HeaderIndices {
fn record_header_indices(
bytes: &[u8],
headers: &[httparse::Header<'_>],
indices: &mut [HeaderIndices],
indices: &mut [MaybeUninit<HeaderIndices>],
) -> Result<(), crate::error::Parse> {
let bytes_ptr = bytes.as_ptr() as usize;

Expand All @@ -1299,10 +1311,19 @@ fn record_header_indices(
}
let name_start = header.name.as_ptr() as usize - bytes_ptr;
let name_end = name_start + header.name.len();
indices.name = (name_start, name_end);
let value_start = header.value.as_ptr() as usize - bytes_ptr;
let value_end = value_start + header.value.len();
indices.value = (value_start, value_end);

// FIXME(maybe_uninit_extra)
// FIXME(addr_of)
// Currently we don't have `ptr::addr_of_mut` in stable rust or
// MaybeUninit::write, so this is some way of assigning into a MaybeUninit
// safely
let new_header_indices = HeaderIndices {
name: (name_start, name_end),
value: (value_start, value_end),
};
*indices = MaybeUninit::new(new_header_indices);
}

Ok(())
Expand Down

0 comments on commit 95a9783

Please sign in to comment.