diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 00000000..00642f83 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1 @@ +github: seanmonstar diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 365a11e2..3817cf8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable @@ -55,7 +55,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Install Rust (${{ matrix.rust }}) uses: dtolnay/rust-toolchain@master @@ -74,13 +74,21 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 + + - uses: dtolnay/rust-toolchain@stable + + - uses: taiki-e/install-action@cargo-hack + + - name: Remove dev-dependencies + run: cargo hack --remove-dev-deps update + + - name: Pin deps + run: cargo update -p itoa --precise 1.0.15 - name: Get MSRV from package metadata id: metadata - run: | - cargo metadata --no-deps --format-version 1 | - jq -r '"msrv=" + (.packages[] | select(.name == "http")).rust_version' >> $GITHUB_OUTPUT + run: echo "msrv=$(yq '.package.rust-version' Cargo.toml)" >> $GITHUB_OUTPUT - name: Install Rust (${{ steps.metadata.outputs.msrv }}) uses: dtolnay/rust-toolchain@master @@ -98,7 +106,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable @@ -111,7 +119,7 @@ jobs: minimal-versions: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - uses: dtolnay/rust-toolchain@nightly - uses: dtolnay/rust-toolchain@stable - uses: taiki-e/install-action@cargo-hack @@ -124,7 +132,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@nightly @@ -138,7 +146,7 @@ jobs: name: semver runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Check semver uses: obi1kenobi/cargo-semver-checks-action@v2 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f74d6a..76ec753f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,38 @@ +# 1.5.0 (July 29, 2026) + +- Add `Method::QUERY` constant for the new QUERY method defined in RFC 10008. +- Fix `uri::Builder::path_and_query()` to allow empty strings to mean no path. +- Fix `uri::PathAndQuery` parsing to enforce URI max length. + +# 1.4.2 (June 8, 2026) + +- Fix `uri::Builder` to allow `"*"` as the path when scheme and authority are also set, used in HTTP/2 requests. +- Fix `Uri` to properly reject `DEL` characters. + +# 1.4.1 (May 25, 2026) + +- Fix `PathAndQuery::from_static()` and `from_shared()` to reject inputs that do not start with `/`. +- Fix `Extend` for `HeaderMap` to clamp max size hint and not overflow. +- Fix `header::IntoIter` that could use-after-free if the generic value type could panic on drop. +- Fix `header::{IterMut, ValuesIterMut}` to not violate stacked borrows. + +# 1.4.0 (November 24, 2025) + +- Add `StatusCode::EARLY_HINTS` constant for 103 Early Hints. +- Make `StatusCode::from_u16` now a `const fn`. +- Make `Authority::from_static` now a `const fn`. +- Make `PathAndQuery::from_static` now a `const fn`. +- MSRV increased to 1.57 (allows legible const fn panic messages). + +# 1.3.1 (March 11, 2025) + +* Fix validation that all characters are UTF-8 in URI path and query. + +# 1.3.0 (March 11, 2025) + +* Allow most UTF-8 characters in URI path and query. +* Fix `HeaderMap::reserve()` to allocate sufficient capacity. + # 1.2.0 (December 3, 2024) * Add `StatusCode::TOO_EARLY` constant for 425 status. diff --git a/Cargo.toml b/Cargo.toml index 1a6e9c9d..b0ba476b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ name = "http" # - Update html_root_url in lib.rs. # - Update CHANGELOG.md. # - Create git tag -version = "1.2.0" +version = "1.5.0" readme = "README.md" documentation = "https://docs.rs/http" repository = "https://github.com/hyperium/http" @@ -19,9 +19,8 @@ A set of types for representing HTTP requests and responses. """ keywords = ["http"] categories = ["web-programming"] -edition = "2018" -# When updating this value, don't forget to also adjust the GitHub Actions config. -rust-version = "1.49.0" +edition = "2021" +rust-version = "1.57.0" [workspace] members = [ @@ -38,12 +37,11 @@ std = [] [dependencies] bytes = "1" -fnv = "1.0.5" itoa = "1" [dev-dependencies] quickcheck = "1" -rand = "0.8.0" +rand = "0.10" serde = "1.0" serde_json = "1.0" doc-comment = "0.3" diff --git a/README.md b/README.md index a0090032..ab7425ee 100644 --- a/README.md +++ b/README.md @@ -62,9 +62,9 @@ fn main() { # Supported Rust Versions -This project follows the [Tokio MSRV][msrv] and is currently set to `1.49`. +This project follows the [hyper's MSRV _policy_][msrv], though it can be lower, and is currently set to `1.57`. -[msrv]: https://github.com/tokio-rs/tokio/#supported-rust-versions +[msrv]: https://hyper.rs/contrib/msrv/ # License diff --git a/benches/Cargo.toml b/benches/Cargo.toml index 0d881f52..789980e4 100644 --- a/benches/Cargo.toml +++ b/benches/Cargo.toml @@ -4,6 +4,10 @@ version = "0.0.0" edition = "2018" publish = false +# `benches` is excluded from the root workspace, so declare an empty workspace +# table to let it build as a standalone package. +[workspace] + [dependencies] bytes = "1" fnv = "1.0.5" @@ -41,3 +45,8 @@ path = "src/method.rs" [[bench]] name = "uri" path = "src/uri.rs" + +[[bench]] +name = "opt_paths" +path = "src/opt_paths.rs" +harness = false diff --git a/benches/src/opt_paths.rs b/benches/src/opt_paths.rs new file mode 100644 index 00000000..fc164ac5 --- /dev/null +++ b/benches/src/opt_paths.rs @@ -0,0 +1,68 @@ +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use http::header::*; +use http::{HeaderValue, Uri}; + +static SHORT: &[u8] = b"localhost"; +static LONG: &[u8] = b"Mozilla/5.0 (X11; CrOS x86_64 9592.71.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.80 Safari/537.36"; + +const REL: &str = "/wp-content/uploads/2010/03/hello-kitty-darth-vader-pink.jpg"; +const REL_QUERY: &str = "/wp-content/uploads/2010/03/hello-kitty-darth-vader-pink.jpg?foo=bar&baz=quux"; +const ABS: &str = "https://www.example.com/wp-content/uploads/hello.jpg?foo=bar"; + +const STD: &[HeaderName] = &[ + HOST, CONTENT_TYPE, CONTENT_LENGTH, ACCEPT, ACCEPT_ENCODING, USER_AGENT, + CONNECTION, CACHE_CONTROL, DATE, SERVER, +]; + +fn header_value(c: &mut Criterion) { + c.bench_function("hv_from_bytes_short", |b| { + b.iter(|| HeaderValue::from_bytes(black_box(SHORT)).unwrap()) + }); + c.bench_function("hv_from_bytes_long", |b| { + b.iter(|| HeaderValue::from_bytes(black_box(LONG)).unwrap()) + }); + let short = HeaderValue::from_bytes(SHORT).unwrap(); + let long = HeaderValue::from_bytes(LONG).unwrap(); + c.bench_function("hv_to_str_short", |b| { + b.iter(|| black_box(&short).to_str().unwrap()) + }); + c.bench_function("hv_to_str_long", |b| { + b.iter(|| black_box(&long).to_str().unwrap()) + }); +} + +fn uri(c: &mut Criterion) { + c.bench_function("uri_parse_relative_medium", |b| { + b.iter(|| black_box(REL).parse::().unwrap()) + }); + c.bench_function("uri_parse_relative_query", |b| { + b.iter(|| black_box(REL_QUERY).parse::().unwrap()) + }); + let rel: Uri = REL.parse().unwrap(); + let rel_query: Uri = REL_QUERY.parse().unwrap(); + let abs: Uri = ABS.parse().unwrap(); + c.bench_function("uri_to_string_relative", |b| { + b.iter(|| black_box(&rel).to_string()) + }); + c.bench_function("uri_to_string_relative_query", |b| { + b.iter(|| black_box(&rel_query).to_string()) + }); + c.bench_function("uri_to_string_absolute", |b| { + b.iter(|| black_box(&abs).to_string()) + }); +} + +fn header_map(c: &mut Criterion) { + c.bench_function("hm_insert_10_std", |b| { + b.iter(|| { + let mut m = HeaderMap::default(); + for hdr in STD { + m.insert(hdr.clone(), "foo"); + } + black_box(m) + }) + }); +} + +criterion_group!(benches, header_value, uri, header_map); +criterion_main!(benches); diff --git a/src/byte_str.rs b/src/byte_str.rs index 90872ecb..f285e7f2 100644 --- a/src/byte_str.rs +++ b/src/byte_str.rs @@ -45,6 +45,12 @@ impl ByteStr { // Invariant: assumed by the safety requirements of this function. ByteStr { bytes } } + + pub(crate) fn from_utf8(bytes: Bytes) -> Result { + str::from_utf8(&bytes)?; + // Invariant: just checked is utf8 + Ok(ByteStr { bytes }) + } } impl ops::Deref for ByteStr { @@ -68,9 +74,9 @@ impl From for ByteStr { } } -impl<'a> From<&'a str> for ByteStr { +impl From<&str> for ByteStr { #[inline] - fn from(src: &'a str) -> ByteStr { + fn from(src: &str) -> ByteStr { ByteStr { // Invariant: src is a str so contains valid UTF-8. bytes: Bytes::copy_from_slice(src.as_bytes()), diff --git a/src/error.rs b/src/error.rs index 762ee1c2..192a953b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -47,6 +47,13 @@ impl fmt::Display for Error { } impl Error { + pub(crate) fn is_empty_uri(&self) -> bool { + match self.inner { + ErrorKind::Uri(ref err) => err.is_empty(), + _ => false, + } + } + /// Return true if the underlying error has the same type as T. pub fn is(&self) -> bool { self.get_ref().is::() diff --git a/src/extensions.rs b/src/extensions.rs index f16d762e..f1c1b7c1 100644 --- a/src/extensions.rs +++ b/src/extensions.rs @@ -1,4 +1,4 @@ -use std::any::{Any, TypeId}; +use std::any::{type_name, Any, TypeId}; use std::collections::HashMap; use std::fmt; use std::hash::{BuildHasherDefault, Hasher}; @@ -33,8 +33,8 @@ impl Hasher for IdHasher { /// extra data derived from the underlying protocol. #[derive(Clone, Default)] pub struct Extensions { - // If extensions are never used, no need to carry around an empty HashMap. - // That's 3 words. Instead, this is only 1 word. + // Extensions might never be used and carrying an empty HashMap around is + // inefficient (because it's 3 words). This is only 1 word instead. map: Option>, } @@ -267,7 +267,21 @@ impl Extensions { impl fmt::Debug for Extensions { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Extensions").finish() + struct TypeName(&'static str); + impl fmt::Debug for TypeName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.0) + } + } + + let mut set = f.debug_set(); + if let Some(map) = &self.map { + set.entries( + map.values() + .map(|any_clone| TypeName(any_clone.as_ref().type_name())), + ); + } + set.finish() } } @@ -276,6 +290,7 @@ trait AnyClone: Any { fn as_any(&self) -> &dyn Any; fn as_any_mut(&mut self) -> &mut dyn Any; fn into_any(self: Box) -> Box; + fn type_name(&self) -> &'static str; } impl AnyClone for T { @@ -294,6 +309,10 @@ impl AnyClone for T { fn into_any(self: Box) -> Box { self } + + fn type_name(&self) -> &'static str { + type_name::() + } } impl Clone for Box { @@ -308,6 +327,7 @@ fn test_extensions() { struct MyType(i32); let mut extensions = Extensions::new(); + assert_eq!(format!("{extensions:?}"), "{}"); extensions.insert(5i32); extensions.insert(MyType(10)); @@ -315,6 +335,15 @@ fn test_extensions() { assert_eq!(extensions.get(), Some(&5i32)); assert_eq!(extensions.get_mut(), Some(&mut 5i32)); + let dbg = format!("{extensions:?}"); + // map order is NOT deterministic + assert!( + (dbg == "{http::extensions::test_extensions::MyType, i32}") + || (dbg == "{i32, http::extensions::test_extensions::MyType}"), + "{}", + dbg + ); + let ext2 = extensions.clone(); assert_eq!(extensions.remove::(), Some(5i32)); diff --git a/src/header/map.rs b/src/header/map.rs index 07b4554a..6a7628e1 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -14,11 +14,47 @@ use super::HeaderValue; pub use self::as_header_name::AsHeaderName; pub use self::into_header_name::IntoHeaderName; -/// A set of HTTP headers +/// A specialized [multimap]() for +/// header names and values. /// -/// `HeaderMap` is a multimap of [`HeaderName`] to values. +/// # Overview +/// +/// `HeaderMap` is designed specifically for efficient manipulation of HTTP +/// headers. It supports multiple values per header name and provides +/// specialized APIs for insertion, retrieval, and iteration. +/// +/// The internal implementation is optimized for common usage patterns in HTTP, +/// and may change across versions. For example, the current implementation uses +/// [Robin Hood +/// hashing]() to +/// store entries compactly and enable high load factors with good performance. +/// However, the collision resolution strategy and storage mechanism are not +/// part of the public API and may be altered in future releases. +/// +/// # Iteration order +/// +/// Unless otherwise specified, the order in which items are returned by +/// iterators from `HeaderMap` methods is arbitrary; there is no guaranteed +/// ordering among the elements yielded by such an iterator. Changes to the +/// iteration order are not considered breaking changes, so users must not rely +/// on any incidental order produced by such an iterator. However, for a given +/// crate version, the iteration order will be consistent across all platforms. +/// +/// # Adaptive hashing +/// +/// `HeaderMap` uses an adaptive strategy for hashing to maintain fast lookups +/// while resisting hash collision attacks. The default hash function +/// prioritizes performance. In scenarios where high collision rates are +/// detected—typically indicative of denial-of-service attacks—the +/// implementation switches to a more secure, collision-resistant hash function. +/// +/// # Limitations +/// +/// A `HeaderMap` can store at most 32,768 entries \(header name/value pairs\). +/// Attempting to exceed this limit will result in a panic. /// /// [`HeaderName`]: struct.HeaderName.html +/// [`HeaderMap`]: struct.HeaderMap.html /// /// # Examples /// @@ -93,7 +129,13 @@ pub struct Iter<'a, T> { /// yielded more than once if it has more than one associated value. #[derive(Debug)] pub struct IterMut<'a, T> { - map: *mut HeaderMap, + // Raw access avoids reborrowing the whole `HeaderMap` on every `next()`, + // which would invalidate previously yielded `&mut T`s. + entries: *mut Bucket, + entries_len: usize, + // This points at the original `HeaderMap::extra_values` allocation for the + // lifetime of the iterator. + extra_values: *mut ExtraValue, entry: usize, cursor: Option, lt: PhantomData<&'a mut HeaderMap>, @@ -198,7 +240,11 @@ pub struct ValueIter<'a, T> { /// A mutable iterator of all values associated with a single header name. #[derive(Debug)] pub struct ValueIterMut<'a, T> { - map: *mut HeaderMap, + // Raw access avoids reborrowing the whole `HeaderMap` on every step. + entries: *mut Bucket, + // This points at the original `HeaderMap::extra_values` allocation for the + // lifetime of the iterator. + extra_values: *mut ExtraValue, index: usize, front: Option, back: Option, @@ -335,7 +381,7 @@ const FORWARD_SHIFT_THRESHOLD: usize = 512; // If growing the hash map would cause the load factor to drop bellow this // threshold, then instead of growing, the headermap is switched to the red // danger state and safe hashing is used instead. -const LOAD_FACTOR_THRESHOLD: f32 = 0.2; +const LOAD_FACTOR_THRESHOLD: usize = 5; // Macro used to iterate the hash table starting at a given point, looping when // the end is hit. @@ -445,8 +491,21 @@ impl HeaderMap { /// assert!(map.is_empty()); /// assert_eq!(0, map.capacity()); /// ``` + #[inline] pub fn new() -> Self { - HeaderMap::try_with_capacity(0).unwrap() + Self::default() + } +} + +impl Default for HeaderMap { + fn default() -> Self { + HeaderMap { + mask: 0, + indices: Box::new([]), // as a ZST, this doesn't actually allocate anything + entries: Vec::new(), + extra_values: Vec::new(), + danger: Danger::Green, + } } } @@ -501,15 +560,10 @@ impl HeaderMap { /// ``` pub fn try_with_capacity(capacity: usize) -> Result, MaxSizeReached> { if capacity == 0 { - Ok(HeaderMap { - mask: 0, - indices: Box::new([]), // as a ZST, this doesn't actually allocate anything - entries: Vec::new(), - extra_values: Vec::new(), - danger: Danger::Green, - }) + Ok(Self::default()) } else { - let raw_cap = match to_raw_capacity(capacity).checked_next_power_of_two() { + let raw_cap = to_raw_capacity(capacity)?; + let raw_cap = match raw_cap.checked_next_power_of_two() { Some(c) => c, None => return Err(MaxSizeReached { _priv: () }), }; @@ -707,20 +761,22 @@ impl HeaderMap { .checked_add(additional) .ok_or_else(MaxSizeReached::new)?; - if cap > self.indices.len() { - let cap = cap + let raw_cap = to_raw_capacity(cap)?; + + if raw_cap > self.indices.len() { + let raw_cap = raw_cap .checked_next_power_of_two() .ok_or_else(MaxSizeReached::new)?; - if cap > MAX_SIZE { + if raw_cap > MAX_SIZE { return Err(MaxSizeReached::new()); } if self.entries.is_empty() { - self.mask = cap as Size - 1; - self.indices = vec![Pos::none(); cap].into_boxed_slice(); - self.entries = Vec::with_capacity(usable_capacity(cap)); + self.mask = raw_cap as Size - 1; + self.indices = vec![Pos::none(); raw_cap].into_boxed_slice(); + self.entries = Vec::with_capacity(usable_capacity(raw_cap)); } else { - self.try_grow(cap)?; + self.try_grow(raw_cap)?; } } @@ -905,7 +961,9 @@ impl HeaderMap { /// ``` pub fn iter_mut(&mut self) -> IterMut<'_, T> { IterMut { - map: self as *mut _, + entries: self.entries.as_mut_ptr(), + entries_len: self.entries.len(), + extra_values: self.extra_values.as_mut_ptr(), entry: 0, cursor: self.entries.first().map(|_| Cursor::Head), lt: PhantomData, @@ -1083,7 +1141,8 @@ impl HeaderMap { }; ValueIterMut { - map: self as *mut _, + entries: self.entries.as_mut_ptr(), + extra_values: self.extra_values.as_mut_ptr(), index: idx, front: Some(Head), back: Some(back), @@ -1693,9 +1752,10 @@ impl HeaderMap { let len = self.entries.len(); if self.danger.is_yellow() { - let load_factor = self.entries.len() as f32 / self.indices.len() as f32; - - if load_factor >= LOAD_FACTOR_THRESHOLD { + // Overflow is not a concern here: entries.len() is bounded by + // MAX_SIZE (2^15) and LOAD_FACTOR_THRESHOLD is 5, so the product + // fits comfortably within a usize. + if self.entries.len() * LOAD_FACTOR_THRESHOLD >= self.indices.len() { // Transition back to green danger level self.danger.set_green(); @@ -2081,6 +2141,23 @@ impl Extend<(Option, T)> for HeaderMap { fn extend, T)>>(&mut self, iter: I) { let mut iter = iter.into_iter(); + // Reserve capacity similar to the (HeaderName, T) impl. + // Keys may be already present or show multiple times in the iterator. + // Reserve the entire hint lower bound if the map is empty. + // Otherwise reserve half the hint (rounded up), so the map + // will only resize twice in the worst case. + let hint = if self.is_empty() { + iter.size_hint().0 + } else { + (iter.size_hint().0 + 1) / 2 + }; + + // Clamp the hint so an over-estimate cannot overflow `reserve`. + let max_reserve = usable_capacity(MAX_SIZE).saturating_sub(self.entries.len()); + let reserve = hint.min(max_reserve); + + self.reserve(reserve); + // The structure of this is a bit weird, but it is mostly to make the // borrow checker happy. let (mut key, mut val) = match iter.next() { @@ -2129,12 +2206,16 @@ impl Extend<(HeaderName, T)> for HeaderMap { // will only resize twice in the worst case. let iter = iter.into_iter(); - let reserve = if self.is_empty() { + let hint = if self.is_empty() { iter.size_hint().0 } else { (iter.size_hint().0 + 1) / 2 }; + // Clamp the hint so an over-estimate cannot overflow `reserve`. + let max_reserve = usable_capacity(MAX_SIZE).saturating_sub(self.entries.len()); + let reserve = hint.min(max_reserve); + self.reserve(reserve); for (k, v) in iter { @@ -2162,12 +2243,6 @@ impl fmt::Debug for HeaderMap { } } -impl Default for HeaderMap { - fn default() -> Self { - HeaderMap::try_with_capacity(0).expect("zero capacity should never fail") - } -} - impl ops::Index for HeaderMap where K: AsHeaderName, @@ -2301,11 +2376,11 @@ unsafe impl<'a, T: Sync> Send for Iter<'a, T> {} // ===== impl IterMut ===== impl<'a, T> IterMut<'a, T> { - fn next_unsafe(&mut self) -> Option<(&'a HeaderName, *mut T)> { + fn next_unsafe(&mut self) -> Option<(*const HeaderName, *mut T)> { use self::Cursor::*; if self.cursor.is_none() { - if (self.entry + 1) >= unsafe { &*self.map }.entries.len() { + if (self.entry + 1) >= self.entries_len { return None; } @@ -2313,22 +2388,46 @@ impl<'a, T> IterMut<'a, T> { self.cursor = Some(Cursor::Head); } - let entry = unsafe { &mut (*self.map).entries[self.entry] }; + // SAFETY: `self.entry < self.entries_len`, and the iterator has + // exclusive access to the underlying map for `'a`, so the `entries` + // allocation remains valid for the lifetime of the iterator. + let entry = unsafe { self.entries.add(self.entry) }; match self.cursor.unwrap() { Head => { - self.cursor = entry.links.map(|l| Values(l.next)); - Some((&entry.key, &mut entry.value as *mut _)) + // SAFETY: `entry` points at a live bucket in `entries`. + self.cursor = unsafe { (*entry).links }.map(|l| Values(l.next)); + // SAFETY: `entry` points at a live bucket, and the iterator only + // yields each slot at most once, so materializing these field + // pointers does not alias another yielded `&mut T`. + Some(unsafe { + ( + ptr::addr_of!((*entry).key), + ptr::addr_of_mut!((*entry).value), + ) + }) } Values(idx) => { - let extra = unsafe { &mut (*self.map).extra_values[idx] }; + // SAFETY: `idx` comes from the `links` chain stored in a live + // bucket / extra value, so it points at a live `extra_values` + // slot for the duration of iteration. + let extra = unsafe { self.extra_values.add(idx) }; - match extra.next { + // SAFETY: `extra` points at a live extra value. + match unsafe { (*extra).next } { Link::Entry(_) => self.cursor = None, Link::Extra(i) => self.cursor = Some(Values(i)), } - Some((&entry.key, &mut extra.value as *mut _)) + // SAFETY: `entry` and `extra` both point at live elements in the + // map backing storage, and the iterator only yields each value + // slot at most once. + Some(unsafe { + ( + ptr::addr_of!((*entry).key), + ptr::addr_of_mut!((*extra).value), + ) + }) } } } @@ -2339,14 +2438,13 @@ impl<'a, T> Iterator for IterMut<'a, T> { fn next(&mut self) -> Option { self.next_unsafe() - .map(|(key, ptr)| (key, unsafe { &mut *ptr })) + .map(|(key, ptr)| (unsafe { &*key }, unsafe { &mut *ptr })) } fn size_hint(&self) -> (usize, Option) { - let map = unsafe { &*self.map }; - debug_assert!(map.entries.len() >= self.entry); + debug_assert!(self.entries_len >= self.entry); - let lower = map.entries.len() - self.entry; + let lower = self.entries_len - self.entry; // We could pessimistically guess at the upper bound, saying // that its lower + map.extra_values.len(). That could be // way over though, such as if we're near the end, and have @@ -2961,7 +3059,9 @@ impl<'a, T: 'a> Iterator for ValueIterMut<'a, T> { fn next(&mut self) -> Option { use self::Cursor::*; - let entry = unsafe { &mut (*self.map).entries[self.index] }; + // SAFETY: `self.index` was created from a live occupied entry and stays + // fixed for the lifetime of this iterator. + let entry = unsafe { self.entries.add(self.index) }; match self.front { Some(Head) => { @@ -2970,7 +3070,8 @@ impl<'a, T: 'a> Iterator for ValueIterMut<'a, T> { self.back = None; } else { // Update the iterator state - match entry.links { + // SAFETY: `entry` points at a live bucket in `entries`. + match unsafe { (*entry).links } { Some(links) => { self.front = Some(Values(links.next)); } @@ -2978,22 +3079,29 @@ impl<'a, T: 'a> Iterator for ValueIterMut<'a, T> { } } - Some(&mut entry.value) + // SAFETY: `entry` points at a live bucket, and `front`/`back` + // ensure this value slot is yielded at most once. + Some(unsafe { &mut *ptr::addr_of_mut!((*entry).value) }) } Some(Values(idx)) => { - let extra = unsafe { &mut (*self.map).extra_values[idx] }; + // SAFETY: `idx` comes from the live linked list rooted at + // `self.index`, so it refers to a live extra value slot. + let extra = unsafe { self.extra_values.add(idx) }; if self.front == self.back { self.front = None; self.back = None; } else { - match extra.next { + // SAFETY: `extra` points at a live extra value. + match unsafe { (*extra).next } { Link::Entry(_) => self.front = None, Link::Extra(i) => self.front = Some(Values(i)), } } - Some(&mut extra.value) + // SAFETY: `extra` points at a live extra value, and + // `front`/`back` ensure this value slot is yielded at most once. + Some(unsafe { &mut *ptr::addr_of_mut!((*extra).value) }) } None => None, } @@ -3004,28 +3112,37 @@ impl<'a, T: 'a> DoubleEndedIterator for ValueIterMut<'a, T> { fn next_back(&mut self) -> Option { use self::Cursor::*; - let entry = unsafe { &mut (*self.map).entries[self.index] }; + // SAFETY: `self.index` was created from a live occupied entry and stays + // fixed for the lifetime of this iterator. + let entry = unsafe { self.entries.add(self.index) }; match self.back { Some(Head) => { self.front = None; self.back = None; - Some(&mut entry.value) + // SAFETY: `entry` points at a live bucket, and `front`/`back` + // ensure this value slot is yielded at most once. + Some(unsafe { &mut *ptr::addr_of_mut!((*entry).value) }) } Some(Values(idx)) => { - let extra = unsafe { &mut (*self.map).extra_values[idx] }; + // SAFETY: `idx` comes from the live linked list rooted at + // `self.index`, so it refers to a live extra value slot. + let extra = unsafe { self.extra_values.add(idx) }; if self.front == self.back { self.front = None; self.back = None; } else { - match extra.prev { + // SAFETY: `extra` points at a live extra value. + match unsafe { (*extra).prev } { Link::Entry(_) => self.back = Some(Head), Link::Extra(idx) => self.back = Some(Values(idx)), } } - Some(&mut extra.value) + // SAFETY: `extra` points at a live extra value, and + // `front`/`back` ensure this value slot is yielded at most once. + Some(unsafe { &mut *ptr::addr_of_mut!((*extra).value) }) } None => None, } @@ -3078,13 +3195,20 @@ impl FusedIterator for IntoIter {} impl Drop for IntoIter { fn drop(&mut self) { - // Ensure the iterator is consumed - for _ in self.by_ref() {} + struct Guard<'a, T>(&'a mut IntoIter); - // All the values have already been yielded out. - unsafe { - self.extra_values.set_len(0); + impl<'a, T> Drop for Guard<'a, T> { + fn drop(&mut self) { + unsafe { + self.0.extra_values.set_len(0); + } + } } + + let guard = Guard(self); + + // Ensure the iterator is consumed + for _ in guard.0.by_ref() {} } } @@ -3582,14 +3706,8 @@ fn usable_capacity(cap: usize) -> usize { } #[inline] -fn to_raw_capacity(n: usize) -> usize { - match n.checked_add(n / 3) { - Some(n) => n, - None => panic!( - "requested capacity {} too large: overflow while converting to raw capacity", - n - ), - } +fn to_raw_capacity(n: usize) -> Result { + n.checked_add(n / 3).ok_or_else(MaxSizeReached::new) } #[inline] @@ -3603,12 +3721,11 @@ fn probe_distance(mask: Size, hash: HashValue, current: usize) -> usize { current.wrapping_sub(desired_pos(mask, hash)) & mask as usize } +#[inline] fn hash_elem_using(danger: &Danger, k: &K) -> HashValue where K: Hash + ?Sized, { - use fnv::FnvHasher; - const MASK: u64 = (MAX_SIZE as u64) - 1; let hash = match *danger { @@ -3620,7 +3737,7 @@ where } // Fast hash _ => { - let mut h = FnvHasher::default(); + let mut h = FnvHasher::new(); k.hash(&mut h); h.finish() } @@ -3629,6 +3746,32 @@ where HashValue((hash & MASK) as u16) } +struct FnvHasher(u64); + +impl FnvHasher { + #[inline] + fn new() -> Self { + FnvHasher(0xcbf29ce484222325) + } +} + +impl std::hash::Hasher for FnvHasher { + #[inline] + fn finish(&self) -> u64 { + self.0 + } + + #[inline] + fn write(&mut self, bytes: &[u8]) { + let mut hash = self.0; + for &b in bytes { + hash ^= b as u64; + hash = hash.wrapping_mul(0x100000001b3); + } + self.0 = hash; + } +} + /* * * ===== impl IntoHeaderName / AsHeaderName ===== @@ -3687,7 +3830,7 @@ mod into_header_name { impl IntoHeaderName for HeaderName {} - impl<'a> Sealed for &'a HeaderName { + impl Sealed for &HeaderName { #[inline] fn try_insert( self, @@ -3707,7 +3850,7 @@ mod into_header_name { } } - impl<'a> IntoHeaderName for &'a HeaderName {} + impl IntoHeaderName for &HeaderName {} impl Sealed for &'static str { #[inline] @@ -3797,7 +3940,7 @@ mod as_header_name { impl AsHeaderName for HeaderName {} - impl<'a> Sealed for &'a HeaderName { + impl Sealed for &HeaderName { #[inline] fn try_entry(self, map: &mut HeaderMap) -> Result, TryEntryError> { Ok(map.try_entry2(self)?) @@ -3813,9 +3956,9 @@ mod as_header_name { } } - impl<'a> AsHeaderName for &'a HeaderName {} + impl AsHeaderName for &HeaderName {} - impl<'a> Sealed for &'a str { + impl Sealed for &str { #[inline] fn try_entry(self, map: &mut HeaderMap) -> Result, TryEntryError> { Ok(HdrName::from_bytes(self.as_bytes(), move |hdr| { @@ -3833,7 +3976,7 @@ mod as_header_name { } } - impl<'a> AsHeaderName for &'a str {} + impl AsHeaderName for &str {} impl Sealed for String { #[inline] @@ -3853,7 +3996,7 @@ mod as_header_name { impl AsHeaderName for String {} - impl<'a> Sealed for &'a String { + impl Sealed for &String { #[inline] fn try_entry(self, map: &mut HeaderMap) -> Result, TryEntryError> { self.as_str().try_entry(map) @@ -3869,7 +4012,7 @@ mod as_header_name { } } - impl<'a> AsHeaderName for &'a String {} + impl AsHeaderName for &String {} } #[test] diff --git a/src/header/mod.rs b/src/header/mod.rs index 5d405767..1a5f1ede 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -28,47 +28,13 @@ //! //! # `HeaderMap` //! -//! `HeaderMap` is a map structure of header names highly optimized for use -//! cases common with HTTP. It is a [multimap] structure, where each header name -//! may have multiple associated header values. Given this, some of the APIs -//! diverge from [`HashMap`]. +//! The [`HeaderMap`] type is a specialized +//! [multimap]() structure for storing +//! header names and values. It is designed specifically for efficient +//! manipulation of HTTP headers. It supports multiple values per header name +//! and provides specialized APIs for insertion, retrieval, and iteration. //! -//! ## Overview -//! -//! Just like `HashMap` in Rust's stdlib, `HeaderMap` is based on [Robin Hood -//! hashing]. This algorithm tends to reduce the worst case search times in the -//! table and enables high load factors without seriously affecting performance. -//! Internally, keys and values are stored in vectors. As such, each insertion -//! will not incur allocation overhead. However, once the underlying vector -//! storage is full, a larger vector must be allocated and all values copied. -//! -//! ## Deterministic ordering -//! -//! Unlike Rust's `HashMap`, values in `HeaderMap` are deterministically -//! ordered. Roughly, values are ordered by insertion. This means that a -//! function that deterministically operates on a header map can rely on the -//! iteration order to remain consistent across processes and platforms. -//! -//! ## Adaptive hashing -//! -//! `HeaderMap` uses an adaptive hashing strategy in order to efficiently handle -//! most common cases. All standard headers have statically computed hash values -//! which removes the need to perform any hashing of these headers at runtime. -//! The default hash function emphasizes performance over robustness. However, -//! `HeaderMap` detects high collision rates and switches to a secure hash -//! function in those events. The threshold is set such that only denial of -//! service attacks should trigger it. -//! -//! ## Limitations -//! -//! `HeaderMap` can store a maximum of 32,768 headers (header name / value -//! pairs). Attempting to insert more will result in a panic. -//! -//! [`HeaderName`]: struct.HeaderName.html -//! [`HeaderMap`]: struct.HeaderMap.html -//! [multimap]: https://en.wikipedia.org/wiki/Multimap -//! [`HashMap`]: https://doc.rust-lang.org/std/collections/struct.HashMap.html -//! [Robin Hood hashing]: https://en.wikipedia.org/wiki/Hash_table#Robin_Hood_hashing +//! [*See also the `HeaderMap` type.*](HeaderMap) mod map; mod name; diff --git a/src/header/name.rs b/src/header/name.rs index 3d563f4e..02af57e1 100644 --- a/src/header/name.rs +++ b/src/header/name.rs @@ -203,7 +203,7 @@ standard_headers! { /// not to compress if a server use more than 80 % of its computational /// power. /// - /// As long as the identity value, meaning no encryption, is not explicitly + /// As long as the identity value, meaning no compression, is not explicitly /// forbidden, by an identity;q=0 or a *;q=0 without another explicitly set /// value for identity, the server must never send back a 406 Not Acceptable /// error. @@ -1205,27 +1205,6 @@ impl HeaderName { /// /// This function panics when the static string is a invalid header. /// - /// Until [Allow panicking in constants](https://github.com/rust-lang/rfcs/pull/2345) - /// makes its way into stable, the panic message at compile-time is - /// going to look cryptic, but should at least point at your header value: - /// - /// ```text - /// error: any use of this value will cause an error - /// --> http/src/header/name.rs:1241:13 - /// | - /// 1241 | ([] as [u8; 0])[0]; // Invalid header name - /// | ^^^^^^^^^^^^^^^^^^ - /// | | - /// | index out of bounds: the length is 0 but the index is 0 - /// | inside `http::HeaderName::from_static` at http/src/header/name.rs:1241:13 - /// | inside `INVALID_NAME` at src/main.rs:3:34 - /// | - /// ::: src/main.rs:3:1 - /// | - /// 3 | const INVALID_NAME: HeaderName = HeaderName::from_static("Capitalized"); - /// | ------------------------------------------------------------------------ - /// ``` - /// /// # Examples /// /// ``` @@ -1245,14 +1224,13 @@ impl HeaderName { /// ```should_panic /// # use http::header::*; /// # - /// // Parsing a header that contains invalid symbols(s): + /// // Parsing a header that contains invalid symbols: /// HeaderName::from_static("content{}{}length"); // This line panics! /// /// // Parsing a header that contains invalid uppercase characters. /// let a = HeaderName::from_static("foobar"); /// let b = HeaderName::from_static("FOOBAR"); // This line panics! /// ``` - #[allow(unconditional_panic)] // required for the panic circumvention pub const fn from_static(src: &'static str) -> HeaderName { let name_bytes = src.as_bytes(); if let Some(standard) = StandardHeader::from_bytes(name_bytes) { @@ -1272,13 +1250,8 @@ impl HeaderName { i += 1; } } { - // TODO: When msrv is bumped to larger than 1.57, this should be - // replaced with `panic!` macro. - // https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html#panic-in-const-contexts - // - // See the panics section of this method's document for details. - #[allow(clippy::no_effect, clippy::out_of_bounds_indexing)] - ([] as [u8; 0])[0]; // Invalid header name + // Invalid header name + panic!("HeaderName::from_static with invalid bytes") } HeaderName { @@ -1346,8 +1319,8 @@ impl InvalidHeaderName { } } -impl<'a> From<&'a HeaderName> for HeaderName { - fn from(src: &'a HeaderName) -> HeaderName { +impl From<&HeaderName> for HeaderName { + fn from(src: &HeaderName) -> HeaderName { src.clone() } } @@ -1372,26 +1345,26 @@ impl From for Bytes { } } -impl<'a> TryFrom<&'a str> for HeaderName { +impl TryFrom<&str> for HeaderName { type Error = InvalidHeaderName; #[inline] - fn try_from(s: &'a str) -> Result { + fn try_from(s: &str) -> Result { Self::from_bytes(s.as_bytes()) } } -impl<'a> TryFrom<&'a String> for HeaderName { +impl TryFrom<&String> for HeaderName { type Error = InvalidHeaderName; #[inline] - fn try_from(s: &'a String) -> Result { + fn try_from(s: &String) -> Result { Self::from_bytes(s.as_bytes()) } } -impl<'a> TryFrom<&'a [u8]> for HeaderName { +impl TryFrom<&[u8]> for HeaderName { type Error = InvalidHeaderName; #[inline] - fn try_from(s: &'a [u8]) -> Result { + fn try_from(s: &[u8]) -> Result { Self::from_bytes(s) } } @@ -1432,14 +1405,14 @@ impl From for HeaderName { } } -impl<'a> PartialEq<&'a HeaderName> for HeaderName { +impl PartialEq<&HeaderName> for HeaderName { #[inline] - fn eq(&self, other: &&'a HeaderName) -> bool { + fn eq(&self, other: &&HeaderName) -> bool { *self == **other } } -impl<'a> PartialEq for &'a HeaderName { +impl PartialEq for &HeaderName { #[inline] fn eq(&self, other: &HeaderName) -> bool { *other == *self @@ -1484,16 +1457,16 @@ impl PartialEq for str { } } -impl<'a> PartialEq<&'a str> for HeaderName { +impl PartialEq<&str> for HeaderName { /// Performs a case-insensitive comparison of the string against the header /// name #[inline] - fn eq(&self, other: &&'a str) -> bool { + fn eq(&self, other: &&str) -> bool { *self == **other } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { /// Performs a case-insensitive comparison of the string against the header /// name #[inline] diff --git a/src/header/value.rs b/src/header/value.rs index 99d1e155..97bb49ef 100644 --- a/src/header/value.rs +++ b/src/header/value.rs @@ -15,7 +15,7 @@ use crate::header::name::HeaderName; /// HTTP spec allows for a header value to contain opaque bytes as well. In this /// case, the header field value is not able to be represented as a string. /// -/// To handle this, the `HeaderValue` is useable as a type and can be compared +/// To handle this, the `HeaderValue` is usable as a type and can be compared /// with strings and implements `Debug`. A `to_str` fn is provided that returns /// an `Err` if the header value contains non visible ascii characters. #[derive(Clone)] @@ -44,34 +44,13 @@ impl HeaderValue { /// /// This function will not perform any copying, however the string is /// checked to ensure that no invalid characters are present. Only visible - /// ASCII characters (32-127) are permitted. + /// ASCII characters (32-126) and horizontal tab are permitted. /// /// # Panics /// /// This function panics if the argument contains invalid header value /// characters. /// - /// Until [Allow panicking in constants](https://github.com/rust-lang/rfcs/pull/2345) - /// makes its way into stable, the panic message at compile-time is - /// going to look cryptic, but should at least point at your header value: - /// - /// ```text - /// error: any use of this value will cause an error - /// --> http/src/header/value.rs:67:17 - /// | - /// 67 | ([] as [u8; 0])[0]; // Invalid header value - /// | ^^^^^^^^^^^^^^^^^^ - /// | | - /// | index out of bounds: the length is 0 but the index is 0 - /// | inside `HeaderValue::from_static` at http/src/header/value.rs:67:17 - /// | inside `INVALID_HEADER` at src/main.rs:73:33 - /// | - /// ::: src/main.rs:73:1 - /// | - /// 73 | const INVALID_HEADER: HeaderValue = HeaderValue::from_static("жsome value"); - /// | ---------------------------------------------------------------------------- - /// ``` - /// /// # Examples /// /// ``` @@ -80,19 +59,12 @@ impl HeaderValue { /// assert_eq!(val, "hello"); /// ``` #[inline] - #[allow(unconditional_panic)] // required for the panic circumvention pub const fn from_static(src: &'static str) -> HeaderValue { let bytes = src.as_bytes(); let mut i = 0; while i < bytes.len() { - if !is_visible_ascii(bytes[i]) { - // TODO: When msrv is bumped to larger than 1.57, this should be - // replaced with `panic!` macro. - // https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html#panic-in-const-contexts - // - // See the panics section of this method's document for details. - #[allow(clippy::no_effect, clippy::out_of_bounds_indexing)] - ([] as [u8; 0])[0]; // Invalid header value + if !is_valid_ascii(bytes[i]) { + panic!("HeaderValue::from_static with invalid bytes") } i += 1; } @@ -106,7 +78,8 @@ impl HeaderValue { /// Attempt to convert a string to a `HeaderValue`. /// /// If the argument contains invalid header value characters, an error is - /// returned. Only visible ASCII characters (32-127) are permitted. Use + /// returned. Only visible ASCII characters (32-126) and horizontal tab are + /// permitted. Use /// `from_bytes` to create a `HeaderValue` that includes opaque octets /// (128-255). /// @@ -131,7 +104,11 @@ impl HeaderValue { #[inline] #[allow(clippy::should_implement_trait)] pub fn from_str(src: &str) -> Result { - HeaderValue::try_from_generic(src, |s| Bytes::copy_from_slice(s.as_bytes())) + HeaderValue::try_from_generic( + src, + |s| Bytes::copy_from_slice(s.as_bytes()), + is_valid_ascii, + ) } /// Converts a HeaderName into a HeaderValue @@ -177,7 +154,7 @@ impl HeaderValue { /// ``` #[inline] pub fn from_bytes(src: &[u8]) -> Result { - HeaderValue::try_from_generic(src, Bytes::copy_from_slice) + HeaderValue::try_from_generic(src, Bytes::copy_from_slice, is_valid_ascii_or_opaque_byte) } /// Attempt to convert a `Bytes` buffer to a `HeaderValue`. @@ -234,17 +211,21 @@ impl HeaderValue { } fn from_shared(src: Bytes) -> Result { - HeaderValue::try_from_generic(src, std::convert::identity) + HeaderValue::try_from_generic(src, std::convert::identity, is_valid_ascii_or_opaque_byte) } - fn try_from_generic, F: FnOnce(T) -> Bytes>( + fn try_from_generic, F: FnOnce(T) -> Bytes, V: Fn(u8) -> bool>( src: T, into: F, + is_valid: V, ) -> Result { + // Avoid an early return so the loop vectorizes. + let mut bad = false; for &b in src.as_ref() { - if !is_valid(b) { - return Err(InvalidHeaderValue { _priv: () }); - } + bad |= !is_valid(b); + } + if bad { + return Err(InvalidHeaderValue { _priv: () }); } Ok(HeaderValue { inner: into(src), @@ -268,10 +249,13 @@ impl HeaderValue { pub fn to_str(&self) -> Result<&str, ToStrError> { let bytes = self.as_ref(); + // Avoid an early return so the loop vectorizes. + let mut bad = false; for &b in bytes { - if !is_visible_ascii(b) { - return Err(ToStrError { _priv: () }); - } + bad |= !is_valid_ascii(b); + } + if bad { + return Err(ToStrError { _priv: () }); } unsafe { Ok(str::from_utf8_unchecked(bytes)) } @@ -391,7 +375,7 @@ impl fmt::Debug for HeaderValue { let mut from = 0; let bytes = self.as_bytes(); for (i, &b) in bytes.iter().enumerate() { - if !is_visible_ascii(b) || b == b'"' { + if !is_valid_ascii(b) || b == b'"' { if from != i { f.write_str(unsafe { str::from_utf8_unchecked(&bytes[from..i]) })?; } @@ -439,7 +423,7 @@ macro_rules! from_integers { let val = HeaderValue::from(n); assert_eq!(val, &n.to_string()); - let n = ::std::$t::MAX; + let n = <$t>::MAX; let val = HeaderValue::from(n); assert_eq!(val, &n.to_string()); } @@ -512,35 +496,35 @@ impl FromStr for HeaderValue { } } -impl<'a> From<&'a HeaderValue> for HeaderValue { +impl From<&HeaderValue> for HeaderValue { #[inline] - fn from(t: &'a HeaderValue) -> Self { + fn from(t: &HeaderValue) -> Self { t.clone() } } -impl<'a> TryFrom<&'a str> for HeaderValue { +impl TryFrom<&str> for HeaderValue { type Error = InvalidHeaderValue; #[inline] - fn try_from(t: &'a str) -> Result { + fn try_from(t: &str) -> Result { t.parse() } } -impl<'a> TryFrom<&'a String> for HeaderValue { +impl TryFrom<&String> for HeaderValue { type Error = InvalidHeaderValue; #[inline] - fn try_from(s: &'a String) -> Result { - Self::from_bytes(s.as_bytes()) + fn try_from(s: &String) -> Result { + Self::from_str(s) } } -impl<'a> TryFrom<&'a [u8]> for HeaderValue { +impl TryFrom<&[u8]> for HeaderValue { type Error = InvalidHeaderValue; #[inline] - fn try_from(t: &'a [u8]) -> Result { + fn try_from(t: &[u8]) -> Result { HeaderValue::from_bytes(t) } } @@ -550,7 +534,7 @@ impl TryFrom for HeaderValue { #[inline] fn try_from(t: String) -> Result { - HeaderValue::from_shared(t.into()) + HeaderValue::try_from_generic(t, |s| s.into(), is_valid_ascii) } } @@ -577,12 +561,15 @@ mod try_from_header_name_tests { } } -const fn is_visible_ascii(b: u8) -> bool { +const fn is_valid_ascii(b: u8) -> bool { b >= 32 && b < 127 || b == b'\t' } +// This validator is only for byte-oriented constructors. HTTP field values +// may contain opaque bytes, even though those bytes cannot be exposed by +// `HeaderValue::to_str`. #[inline] -fn is_valid(b: u8) -> bool { +fn is_valid_ascii_or_opaque_byte(b: u8) -> bool { b >= 32 && b != 127 || b == b'\t' } @@ -725,48 +712,48 @@ impl PartialOrd for String { } } -impl<'a> PartialEq for &'a HeaderValue { +impl PartialEq for &HeaderValue { #[inline] fn eq(&self, other: &HeaderValue) -> bool { **self == *other } } -impl<'a> PartialOrd for &'a HeaderValue { +impl PartialOrd for &HeaderValue { #[inline] fn partial_cmp(&self, other: &HeaderValue) -> Option { (**self).partial_cmp(other) } } -impl<'a, T: ?Sized> PartialEq<&'a T> for HeaderValue +impl PartialEq<&T> for HeaderValue where HeaderValue: PartialEq, { #[inline] - fn eq(&self, other: &&'a T) -> bool { + fn eq(&self, other: &&T) -> bool { *self == **other } } -impl<'a, T: ?Sized> PartialOrd<&'a T> for HeaderValue +impl PartialOrd<&T> for HeaderValue where HeaderValue: PartialOrd, { #[inline] - fn partial_cmp(&self, other: &&'a T) -> Option { + fn partial_cmp(&self, other: &&T) -> Option { self.partial_cmp(*other) } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { #[inline] fn eq(&self, other: &HeaderValue) -> bool { *other == *self } } -impl<'a> PartialOrd for &'a str { +impl PartialOrd for &str { #[inline] fn partial_cmp(&self, other: &HeaderValue) -> Option { self.as_bytes().partial_cmp(other.as_bytes()) @@ -778,6 +765,37 @@ fn test_try_from() { HeaderValue::try_from(vec![127]).unwrap_err(); } +#[test] +fn test_string_constructors_reject_non_ascii() { + let value = String::from("hello \u{e9}"); + + assert!(HeaderValue::from_str(&value).is_err()); + assert!(HeaderValue::try_from(value.as_str()).is_err()); + assert!(HeaderValue::try_from(&value).is_err()); + assert!(HeaderValue::try_from(value).is_err()); +} + +#[test] +fn test_byte_constructors_allow_opaque_bytes_but_reject_del() { + assert!(HeaderValue::from_bytes(b"hello\xff").is_ok()); + assert!(HeaderValue::try_from(&b"hello\xff"[..]).is_ok()); + assert!(HeaderValue::try_from(b"hello\xff".to_vec()).is_ok()); + + assert!(HeaderValue::from_bytes(b"hello\x7f").is_err()); +} + +#[test] +fn test_string_and_byte_constructors_allow_horizontal_tab() { + assert!(HeaderValue::from_str("hello\tworld").is_ok()); + assert!(HeaderValue::from_bytes(b"hello\tworld").is_ok()); +} + +#[test] +#[should_panic(expected = "HeaderValue::from_static with invalid bytes")] +fn test_static_constructor_rejects_non_ascii() { + HeaderValue::from_static("hello \u{e9}"); +} + #[test] fn test_debug() { let cases = &[ diff --git a/src/method.rs b/src/method.rs index 7b4584ab..e945c1de 100644 --- a/src/method.rs +++ b/src/method.rs @@ -60,6 +60,7 @@ enum Inner { Trace, Connect, Patch, + Query, // If the extension is short enough, store it inline ExtensionInline(InlineExtension), // Otherwise, allocate it @@ -94,6 +95,9 @@ impl Method { /// TRACE pub const TRACE: Method = Method(Trace); + /// QUERY + pub const QUERY: Method = Method(Query); + /// Converts a slice of bytes to an HTTP method. pub fn from_bytes(src: &[u8]) -> Result { match src.len() { @@ -111,6 +115,7 @@ impl Method { 5 => match src { b"PATCH" => Ok(Method(Patch)), b"TRACE" => Ok(Method(Trace)), + b"QUERY" => Ok(Method(Query)), _ => Method::extension_inline(src), }, 6 => match src { @@ -146,7 +151,7 @@ impl Method { /// See [the spec](https://tools.ietf.org/html/rfc7231#section-4.2.1) /// for more words. pub fn is_safe(&self) -> bool { - matches!(self.0, Get | Head | Options | Trace) + matches!(self.0, Get | Head | Options | Trace | Query) } /// Whether a method is considered "idempotent", meaning the request has @@ -174,6 +179,7 @@ impl Method { Trace => "TRACE", Connect => "CONNECT", Patch => "PATCH", + Query => "QUERY", ExtensionInline(ref inline) => inline.as_str(), ExtensionAllocated(ref allocated) => allocated.as_str(), } @@ -187,14 +193,28 @@ impl AsRef for Method { } } -impl<'a> PartialEq<&'a Method> for Method { +impl Ord for Method { + #[inline] + fn cmp(&self, other: &Method) -> std::cmp::Ordering { + self.as_ref().cmp(other.as_ref()) + } +} + +impl PartialOrd for Method { + #[inline] + fn partial_cmp(&self, other: &Method) -> Option { + Some(self.cmp(other)) + } +} + +impl PartialEq<&Method> for Method { #[inline] - fn eq(&self, other: &&'a Method) -> bool { + fn eq(&self, other: &&Method) -> bool { self == *other } } -impl<'a> PartialEq for &'a Method { +impl PartialEq for &Method { #[inline] fn eq(&self, other: &Method) -> bool { *self == other @@ -215,14 +235,14 @@ impl PartialEq for str { } } -impl<'a> PartialEq<&'a str> for Method { +impl PartialEq<&str> for Method { #[inline] - fn eq(&self, other: &&'a str) -> bool { + fn eq(&self, other: &&str) -> bool { self.as_ref() == *other } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { #[inline] fn eq(&self, other: &Method) -> bool { *self == other.as_ref() @@ -248,27 +268,27 @@ impl Default for Method { } } -impl<'a> From<&'a Method> for Method { +impl From<&Method> for Method { #[inline] - fn from(t: &'a Method) -> Self { + fn from(t: &Method) -> Self { t.clone() } } -impl<'a> TryFrom<&'a [u8]> for Method { +impl TryFrom<&[u8]> for Method { type Error = InvalidMethod; #[inline] - fn try_from(t: &'a [u8]) -> Result { + fn try_from(t: &[u8]) -> Result { Method::from_bytes(t) } } -impl<'a> TryFrom<&'a str> for Method { +impl TryFrom<&str> for Method { type Error = InvalidMethod; #[inline] - fn try_from(t: &'a str) -> Result { + fn try_from(t: &str) -> Result { TryFrom::try_from(t.as_bytes()) } } @@ -452,6 +472,7 @@ mod test { assert!(Method::DELETE.is_idempotent()); assert!(Method::HEAD.is_idempotent()); assert!(Method::TRACE.is_idempotent()); + assert!(Method::QUERY.is_idempotent()); assert!(!Method::POST.is_idempotent()); assert!(!Method::CONNECT.is_idempotent()); diff --git a/src/request.rs b/src/request.rs index 324b676c..f47c6630 100644 --- a/src/request.rs +++ b/src/request.rs @@ -118,9 +118,6 @@ use crate::{Extensions, Result, Uri}; /// Deserialize a request of bytes via json: /// /// ``` -/// # extern crate serde; -/// # extern crate serde_json; -/// # extern crate http; /// use http::Request; /// use serde::de; /// @@ -138,9 +135,6 @@ use crate::{Extensions, Result, Uri}; /// Or alternatively, serialize the body of a request to json /// /// ``` -/// # extern crate serde; -/// # extern crate serde_json; -/// # extern crate http; /// use http::Request; /// use serde::ser; /// @@ -186,7 +180,7 @@ pub struct Parts { /// An HTTP request builder /// -/// This type can be used to construct an instance or `Request` +/// This type can be used to construct an instance of `Request` /// through a builder-like pattern. #[derive(Debug)] pub struct Builder { @@ -413,6 +407,10 @@ impl Request<()> { { Builder::new().method(Method::TRACE).uri(uri) } + + // This is purposefully excluded because of potential conflict with the + // URI query. + // pub fn query() -> Builder } impl Request { diff --git a/src/response.rs b/src/response.rs index ab9e49bc..b8fc9b2a 100644 --- a/src/response.rs +++ b/src/response.rs @@ -140,9 +140,6 @@ use crate::{Extensions, Result}; /// Deserialize a response of bytes via json: /// /// ``` -/// # extern crate serde; -/// # extern crate serde_json; -/// # extern crate http; /// use http::Response; /// use serde::de; /// @@ -160,9 +157,6 @@ use crate::{Extensions, Result}; /// Or alternatively, serialize the body of a response to json /// /// ``` -/// # extern crate serde; -/// # extern crate serde_json; -/// # extern crate http; /// use http::Response; /// use serde::ser; /// diff --git a/src/status.rs b/src/status.rs index 9ad04d20..aa9dc308 100644 --- a/src/status.rs +++ b/src/status.rs @@ -1,6 +1,6 @@ //! HTTP status codes //! -//! This module contains HTTP-status code related structs an errors. The main +//! This module contains HTTP-status code related structs and errors. The main //! type in this module is `StatusCode` which is not intended to be used through //! this module but rather the `http::StatusCode` type. //! @@ -44,7 +44,7 @@ use std::str::FromStr; #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct StatusCode(NonZeroU16); -/// A possible error value when converting a `StatusCode` from a `u16` or `&str` +/// A possible error value when converting a `StatusCode` from a `u16` or `&str`. /// /// This error indicates that the supplied input was not a valid number, was less /// than 100, or was greater than 999. @@ -70,17 +70,16 @@ impl StatusCode { /// assert!(err.is_err()); /// ``` #[inline] - pub fn from_u16(src: u16) -> Result { - if !(100..1000).contains(&src) { - return Err(InvalidStatusCode::new()); + pub const fn from_u16(src: u16) -> Result { + if let 100..=999 = src { + if let Some(code) = NonZeroU16::new(src) { + return Ok(StatusCode(code)); + } } - - NonZeroU16::new(src) - .map(StatusCode) - .ok_or_else(InvalidStatusCode::new) + Err(InvalidStatusCode::new()) } - /// Converts a &[u8] to a status code + /// Converts a `&[u8]` to a status code. pub fn from_bytes(src: &[u8]) -> Result { if src.len() != 3 { return Err(InvalidStatusCode::new()); @@ -117,7 +116,7 @@ impl StatusCode { /// ``` #[inline] pub const fn as_u16(&self) -> u16 { - (*self).0.get() + self.0.get() } /// Returns a &str representation of the `StatusCode` @@ -175,31 +174,31 @@ impl StatusCode { /// Check if status is within 100-199. #[inline] pub fn is_informational(&self) -> bool { - 200 > self.0.get() && self.0.get() >= 100 + (100..200).contains(&self.0.get()) } /// Check if status is within 200-299. #[inline] pub fn is_success(&self) -> bool { - 300 > self.0.get() && self.0.get() >= 200 + (200..300).contains(&self.0.get()) } /// Check if status is within 300-399. #[inline] pub fn is_redirection(&self) -> bool { - 400 > self.0.get() && self.0.get() >= 300 + (300..400).contains(&self.0.get()) } /// Check if status is within 400-499. #[inline] pub fn is_client_error(&self) -> bool { - 500 > self.0.get() && self.0.get() >= 400 + (400..500).contains(&self.0.get()) } /// Check if status is within 500-599. #[inline] pub fn is_server_error(&self) -> bool { - 600 > self.0.get() && self.0.get() >= 500 + (500..600).contains(&self.0.get()) } } @@ -264,27 +263,27 @@ impl FromStr for StatusCode { } } -impl<'a> From<&'a StatusCode> for StatusCode { +impl From<&StatusCode> for StatusCode { #[inline] - fn from(t: &'a StatusCode) -> Self { + fn from(t: &StatusCode) -> Self { t.to_owned() } } -impl<'a> TryFrom<&'a [u8]> for StatusCode { +impl TryFrom<&[u8]> for StatusCode { type Error = InvalidStatusCode; #[inline] - fn try_from(t: &'a [u8]) -> Result { + fn try_from(t: &[u8]) -> Result { StatusCode::from_bytes(t) } } -impl<'a> TryFrom<&'a str> for StatusCode { +impl TryFrom<&str> for StatusCode { type Error = InvalidStatusCode; #[inline] - fn try_from(t: &'a str) -> Result { + fn try_from(t: &str) -> Result { t.parse() } } @@ -334,6 +333,9 @@ status_codes! { /// 102 Processing /// [[RFC2518, Section 10.1](https://datatracker.ietf.org/doc/html/rfc2518#section-10.1)] (102, PROCESSING, "Processing"); + /// 103 Early Hints + /// [[RFC8297, Section 2](https://datatracker.ietf.org/doc/html/rfc8297#section-2)] + (103, EARLY_HINTS, "Early Hints"); /// 200 OK /// [[RFC9110, Section 15.3.1](https://datatracker.ietf.org/doc/html/rfc9110#section-15.3.1)] @@ -523,7 +525,7 @@ status_codes! { } impl InvalidStatusCode { - fn new() -> InvalidStatusCode { + const fn new() -> InvalidStatusCode { InvalidStatusCode { _priv: () } } } diff --git a/src/uri/authority.rs b/src/uri/authority.rs index 07aa6795..c5479cb2 100644 --- a/src/uri/authority.rs +++ b/src/uri/authority.rs @@ -8,6 +8,19 @@ use bytes::Bytes; use super::{ErrorKind, InvalidUri, Port, URI_CHARS}; use crate::byte_str::ByteStr; +/// Validation result for authority parsing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AuthorityError { + Empty, + InvalidUriChar, + InvalidAuthority, + TooManyColons, + MismatchedBrackets, + InvalidBracketUsage, + EmptyAfterAt, + InvalidPercent, +} + /// Represents the authority component of a URI. #[derive(Clone)] pub struct Authority { @@ -45,9 +58,14 @@ impl Authority { /// let authority = Authority::from_static("example.com"); /// assert_eq!(authority.host(), "example.com"); /// ``` - pub fn from_static(src: &'static str) -> Self { - Authority::from_shared(Bytes::from_static(src.as_bytes())) - .expect("static str is not valid authority") + #[inline] + pub const fn from_static(src: &'static str) -> Self { + match validate_authority_bytes(src.as_bytes()) { + Ok(_) => Authority { + data: ByteStr::from_static(src), + }, + Err(_) => panic!("static str is not valid authority"), + } } /// Attempt to convert a `Bytes` buffer to a `Authority`. @@ -69,95 +87,19 @@ impl Authority { // Postcondition: for all Ok() returns, s[..ret.unwrap()] is valid UTF-8 where // ret is the return value. pub(super) fn parse(s: &[u8]) -> Result { - let mut colon_cnt = 0u32; - let mut start_bracket = false; - let mut end_bracket = false; - let mut has_percent = false; - let mut end = s.len(); - let mut at_sign_pos = None; - const MAX_COLONS: u32 = 8; // e.g., [FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:80 - - // Among other things, this loop checks that every byte in s up to the - // first '/', '?', or '#' is a valid URI character (or in some contexts, - // a '%'). This means that each such byte is a valid single-byte UTF-8 - // code point. - for (i, &b) in s.iter().enumerate() { - match URI_CHARS[b as usize] { - b'/' | b'?' | b'#' => { - end = i; - break; - } - b':' => { - if colon_cnt >= MAX_COLONS { - return Err(ErrorKind::InvalidAuthority.into()); - } - colon_cnt += 1; - } - b'[' => { - if has_percent || start_bracket { - // Something other than the userinfo has a `%`, so reject it. - return Err(ErrorKind::InvalidAuthority.into()); - } - start_bracket = true; - } - b']' => { - if (!start_bracket) || end_bracket { - return Err(ErrorKind::InvalidAuthority.into()); - } - end_bracket = true; - - // Those were part of an IPv6 hostname, so forget them... - colon_cnt = 0; - has_percent = false; - } - b'@' => { - at_sign_pos = Some(i); - - // Those weren't a port colon, but part of the - // userinfo, so it needs to be forgotten. - colon_cnt = 0; - has_percent = false; - } - 0 if b == b'%' => { - // Per https://tools.ietf.org/html/rfc3986#section-3.2.1 and - // https://url.spec.whatwg.org/#authority-state - // the userinfo can have a percent-encoded username and password, - // so record that a `%` was found. If this turns out to be - // part of the userinfo, this flag will be cleared. - // Also per https://tools.ietf.org/html/rfc6874, percent-encoding can - // be used to indicate a zone identifier. - // If the flag hasn't been cleared at the end, that means this - // was part of the hostname (and not part of an IPv6 address), and - // will fail with an error. - has_percent = true; - } - 0 => { - return Err(ErrorKind::InvalidUriChar.into()); - } - _ => {} + validate_authority_bytes(s).map_err(|e| { + match e { + AuthorityError::Empty => ErrorKind::Empty, + AuthorityError::InvalidUriChar => ErrorKind::InvalidUriChar, + AuthorityError::InvalidAuthority + | AuthorityError::MismatchedBrackets + | AuthorityError::InvalidBracketUsage + | AuthorityError::EmptyAfterAt + | AuthorityError::InvalidPercent + | AuthorityError::TooManyColons => ErrorKind::InvalidAuthority, } - } - - if start_bracket ^ end_bracket { - return Err(ErrorKind::InvalidAuthority.into()); - } - - if colon_cnt > 1 { - // Things like 'localhost:8080:3030' are rejected. - return Err(ErrorKind::InvalidAuthority.into()); - } - - if end > 0 && at_sign_pos == Some(end - 1) { - // If there's nothing after an `@`, this is bonkers. - return Err(ErrorKind::InvalidAuthority.into()); - } - - if has_percent { - // Something after the userinfo has a `%`, so reject it. - return Err(ErrorKind::InvalidAuthority.into()); - } - - Ok(end) + .into() + }) } // Parse bytes as an Authority, not allowing an empty string. @@ -302,14 +244,14 @@ impl PartialEq for str { } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { fn eq(&self, other: &Authority) -> bool { self.eq_ignore_ascii_case(other.as_str()) } } -impl<'a> PartialEq<&'a str> for Authority { - fn eq(&self, other: &&'a str) -> bool { +impl PartialEq<&str> for Authority { + fn eq(&self, other: &&str) -> bool { self.data.eq_ignore_ascii_case(other) } } @@ -360,7 +302,7 @@ impl PartialOrd for str { } } -impl<'a> PartialOrd for &'a str { +impl PartialOrd for &str { fn partial_cmp(&self, other: &Authority) -> Option { let left = self.as_bytes().iter().map(|b| b.to_ascii_lowercase()); let right = other.data.as_bytes().iter().map(|b| b.to_ascii_lowercase()); @@ -368,8 +310,8 @@ impl<'a> PartialOrd for &'a str { } } -impl<'a> PartialOrd<&'a str> for Authority { - fn partial_cmp(&self, other: &&'a str) -> Option { +impl PartialOrd<&str> for Authority { + fn partial_cmp(&self, other: &&str) -> Option { let left = self.data.as_bytes().iter().map(|b| b.to_ascii_lowercase()); let right = other.as_bytes().iter().map(|b| b.to_ascii_lowercase()); left.partial_cmp(right) @@ -426,10 +368,10 @@ impl Hash for Authority { } } -impl<'a> TryFrom<&'a [u8]> for Authority { +impl TryFrom<&[u8]> for Authority { type Error = InvalidUri; #[inline] - fn try_from(s: &'a [u8]) -> Result { + fn try_from(s: &[u8]) -> Result { // parse first, and only turn into Bytes if valid // Preconditon on create_authority: copy_from_slice() copies all of @@ -438,10 +380,10 @@ impl<'a> TryFrom<&'a [u8]> for Authority { } } -impl<'a> TryFrom<&'a str> for Authority { +impl TryFrom<&str> for Authority { type Error = InvalidUri; #[inline] - fn try_from(s: &'a str) -> Result { + fn try_from(s: &str) -> Result { TryFrom::try_from(s.as_bytes()) } } @@ -528,6 +470,105 @@ where }) } +/// Shared validation logic for authority bytes. +/// Returns the end position of valid authority bytes, or an error. +const fn validate_authority_bytes(s: &[u8]) -> Result { + if s.is_empty() { + return Err(AuthorityError::Empty); + } + + let mut colon_cnt: u32 = 0; + let mut start_bracket = false; + let mut end_bracket = false; + let mut has_percent = false; + let mut end = s.len(); + let mut at_sign_pos: usize = s.len(); + const MAX_COLONS: u32 = 8; // e.g., [FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:80 + + let mut i = 0; + // Among other things, this loop checks that every byte in s up to the + // first '/', '?', or '#' is a valid URI character (or in some contexts, + // a '%'). This means that each such byte is a valid single-byte UTF-8 + // code point. + while i < s.len() { + let b = s[i]; + let ch = URI_CHARS[b as usize]; + + if ch == b'/' || ch == b'?' || ch == b'#' { + end = i; + break; + } + + if ch == 0 { + if b == b'%' { + // Per https://tools.ietf.org/html/rfc3986#section-3.2.1 and + // https://url.spec.whatwg.org/#authority-state + // the userinfo can have a percent-encoded username and password, + // so record that a `%` was found. If this turns out to be + // part of the userinfo, this flag will be cleared. + // Also per https://tools.ietf.org/html/rfc6874, percent-encoding can + // be used to indicate a zone identifier. + // If the flag hasn't been cleared at the end, that means this + // was part of the hostname (and not part of an IPv6 address), and + // will fail with an error. + has_percent = true; + } else { + return Err(AuthorityError::InvalidUriChar); + } + } else if ch == b':' { + if colon_cnt >= MAX_COLONS { + return Err(AuthorityError::TooManyColons); + } + colon_cnt += 1; + } else if ch == b'[' { + if has_percent || start_bracket { + // Something other than the userinfo has a `%`, so reject it. + return Err(AuthorityError::InvalidBracketUsage); + } + start_bracket = true; + } else if ch == b']' { + if !start_bracket || end_bracket { + return Err(AuthorityError::InvalidBracketUsage); + } + end_bracket = true; + + // Those were part of an IPv6 hostname, so forget them... + colon_cnt = 0; + has_percent = false; + } else if ch == b'@' { + at_sign_pos = i; + + // Those weren't a port colon, but part of the + // userinfo, so it needs to be forgotten. + colon_cnt = 0; + has_percent = false; + } + + i += 1; + } + + if start_bracket != end_bracket { + return Err(AuthorityError::MismatchedBrackets); + } + + if colon_cnt > 1 { + // Things like 'localhost:8080:3030' are rejected. + return Err(AuthorityError::InvalidAuthority); + } + + if end > 0 && at_sign_pos == end - 1 { + // If there's nothing after an `@`, this is bonkers. + return Err(AuthorityError::EmptyAfterAt); + } + + if has_percent { + // Something after the userinfo has a `%`, so reject it. + return Err(AuthorityError::InvalidPercent); + } + + Ok(end) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/uri/builder.rs b/src/uri/builder.rs index d5f7f49b..046a116a 100644 --- a/src/uri/builder.rs +++ b/src/uri/builder.rs @@ -96,7 +96,17 @@ impl Builder { >::Error: Into, { self.map(move |mut parts| { - let p_and_q = p_and_q.try_into().map_err(Into::into)?; + let p_and_q = match p_and_q.try_into() { + Ok(p_and_q) => p_and_q, + Err(err) => { + let err = err.into(); + if err.is_empty_uri() { + PathAndQuery::empty() + } else { + return Err(err); + } + } + }; parts.path_and_query = Some(p_and_q); Ok(parts) }) @@ -202,10 +212,50 @@ mod tests { } } + #[test] + fn build_from_empty_path_and_query() { + let uri = Builder::new() + .scheme(Scheme::HTTP) + .authority("localhost:8080") + .path_and_query("") + .build() + .unwrap(); + + assert_eq!(uri, "http://localhost:8080"); + assert_eq!(uri.path(), "/"); + } + + #[test] + fn empty_path_and_query_remains_strict() { + assert!(PathAndQuery::try_from("").is_err()); + } + + #[test] + fn authority_form_path_and_query_remains_strict() { + assert!(Builder::new() + .path_and_query("localhost:8080") + .build() + .is_err()); + } + #[test] fn build_from_uri() { let original_uri = Uri::default(); let uri = Builder::from(original_uri.clone()).build().unwrap(); assert_eq!(original_uri, uri); } + + #[test] + fn build_star_for_http2() { + let uri = Builder::new() + .scheme("https") + .authority("example.com") + .path_and_query("*") + .build() + .unwrap(); + + assert_eq!(uri.scheme(), Some(&Scheme::HTTPS)); + assert_eq!(uri.host(), Some("example.com")); + assert_eq!(uri.path(), "*"); + } } diff --git a/src/uri/mod.rs b/src/uri/mod.rs index 767f0743..9a551819 100644 --- a/src/uri/mod.rs +++ b/src/uri/mod.rs @@ -135,6 +135,7 @@ enum ErrorKind { SchemeMissing, AuthorityMissing, PathAndQueryMissing, + PathDoesNotStartWithSlash, TooLong, Empty, SchemeTooLong, @@ -705,29 +706,29 @@ impl Uri { } } -impl<'a> TryFrom<&'a [u8]> for Uri { +impl TryFrom<&[u8]> for Uri { type Error = InvalidUri; #[inline] - fn try_from(t: &'a [u8]) -> Result { + fn try_from(t: &[u8]) -> Result { Uri::from_shared(Bytes::copy_from_slice(t)) } } -impl<'a> TryFrom<&'a str> for Uri { +impl TryFrom<&str> for Uri { type Error = InvalidUri; #[inline] - fn try_from(t: &'a str) -> Result { + fn try_from(t: &str) -> Result { t.parse() } } -impl<'a> TryFrom<&'a String> for Uri { +impl TryFrom<&String> for Uri { type Error = InvalidUri; #[inline] - fn try_from(t: &'a String) -> Result { + fn try_from(t: &String) -> Result { t.parse() } } @@ -759,11 +760,11 @@ impl TryFrom for Uri { } } -impl<'a> TryFrom<&'a Uri> for Uri { +impl TryFrom<&Uri> for Uri { type Error = crate::Error; #[inline] - fn try_from(src: &'a Uri) -> Result { + fn try_from(src: &Uri) -> Result { Ok(src.clone()) } } @@ -872,10 +873,17 @@ fn parse_full(mut s: Bytes) -> Result { data: unsafe { ByteStr::from_utf8_unchecked(authority) }, }; + // When absolute, path is coered to / if empty. + let path_and_query = if s.is_empty() { + PathAndQuery::slash() + } else { + PathAndQuery::from_shared(s)? + }; + Ok(Uri { scheme: scheme.into(), authority, - path_and_query: PathAndQuery::from_shared(s)?, + path_and_query, }) } @@ -995,13 +1003,13 @@ impl PartialEq for str { } } -impl<'a> PartialEq<&'a str> for Uri { - fn eq(&self, other: &&'a str) -> bool { +impl PartialEq<&str> for Uri { + fn eq(&self, other: &&str) -> bool { self == *other } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { fn eq(&self, uri: &Uri) -> bool { uri == *self } @@ -1024,17 +1032,19 @@ impl Default for Uri { impl fmt::Display for Uri { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(scheme) = self.scheme() { - write!(f, "{}://", scheme)?; + f.write_str(scheme.as_str())?; + f.write_str("://")?; } if let Some(authority) = self.authority() { - write!(f, "{}", authority)?; + f.write_str(authority.as_str())?; } - write!(f, "{}", self.path())?; + f.write_str(self.path())?; if let Some(query) = self.query() { - write!(f, "?{}", query)?; + f.write_str("?")?; + f.write_str(query)?; } Ok(()) @@ -1060,6 +1070,10 @@ impl From for InvalidUriParts { } impl InvalidUri { + pub(crate) fn is_empty(&self) -> bool { + self.0 == ErrorKind::Empty + } + fn s(&self) -> &str { match self.0 { ErrorKind::InvalidUriChar => "invalid uri character", @@ -1070,6 +1084,7 @@ impl InvalidUri { ErrorKind::SchemeMissing => "scheme missing", ErrorKind::AuthorityMissing => "authority missing", ErrorKind::PathAndQueryMissing => "path missing", + ErrorKind::PathDoesNotStartWithSlash => "path does not start with slash", ErrorKind::TooLong => "uri too long", ErrorKind::Empty => "empty string", ErrorKind::SchemeTooLong => "scheme too long", diff --git a/src/uri/path.rs b/src/uri/path.rs index df00c415..f62a6867 100644 --- a/src/uri/path.rs +++ b/src/uri/path.rs @@ -4,7 +4,7 @@ use std::{cmp, fmt, hash, str}; use bytes::Bytes; -use super::{ErrorKind, InvalidUri}; +use super::{ErrorKind, InvalidUri, MAX_LEN}; use crate::byte_str::ByteStr; /// Represents the path component of a URI @@ -19,90 +19,23 @@ const NONE: u16 = u16::MAX; impl PathAndQuery { // Not public while `bytes` is unstable. pub(super) fn from_shared(mut src: Bytes) -> Result { - let mut query = NONE; - let mut fragment = None; - - // block for iterator borrow - { - let mut iter = src.as_ref().iter().enumerate(); - - // path ... - for (i, &b) in &mut iter { - // See https://url.spec.whatwg.org/#path-state - match b { - b'?' => { - debug_assert_eq!(query, NONE); - query = i as u16; - break; - } - b'#' => { - fragment = Some(i); - break; - } - - // This is the range of bytes that don't need to be - // percent-encoded in the path. If it should have been - // percent-encoded, then error. - #[rustfmt::skip] - 0x21 | - 0x24..=0x3B | - 0x3D | - 0x40..=0x5F | - 0x61..=0x7A | - 0x7C | - 0x7E..=0xFF => {} - - // These are code points that are supposed to be - // percent-encoded in the path but there are clients - // out there sending them as is and httparse accepts - // to parse those requests, so they are allowed here - // for parity. - // - // For reference, those are code points that are used - // to send requests with JSON directly embedded in - // the URI path. Yes, those things happen for real. - #[rustfmt::skip] - b'"' | - b'{' | b'}' => {} - - _ => return Err(ErrorKind::InvalidUriChar.into()), - } - } - - // query ... - if query != NONE { - for (i, &b) in iter { - match b { - // While queries *should* be percent-encoded, most - // bytes are actually allowed... - // See https://url.spec.whatwg.org/#query-state - // - // Allowed: 0x21 / 0x24 - 0x3B / 0x3D / 0x3F - 0x7E - #[rustfmt::skip] - 0x21 | - 0x24..=0x3B | - 0x3D | - 0x3F..=0xFF => {} - - b'#' => { - fragment = Some(i); - break; - } - - _ => return Err(ErrorKind::InvalidUriChar.into()), - } - } - } - } + let Scanned { + query, + fragment, + is_maybe_not_utf8, + } = scan_path_and_query(&src)?; if let Some(i) = fragment { - src.truncate(i); + src.truncate(i as usize); } - Ok(PathAndQuery { - data: unsafe { ByteStr::from_utf8_unchecked(src) }, - query, - }) + let data = if is_maybe_not_utf8 { + ByteStr::from_utf8(src).map_err(|_| ErrorKind::InvalidUriChar)? + } else { + unsafe { ByteStr::from_utf8_unchecked(src) } + }; + + Ok(PathAndQuery { data, query }) } /// Convert a `PathAndQuery` from a static string. @@ -124,10 +57,19 @@ impl PathAndQuery { /// assert_eq!(v.query(), Some("world")); /// ``` #[inline] - pub fn from_static(src: &'static str) -> Self { - let src = Bytes::from_static(src.as_bytes()); - - PathAndQuery::from_shared(src).unwrap() + pub const fn from_static(src: &'static str) -> Self { + match scan_path_and_query(src.as_bytes()) { + Ok(Scanned { + query, + fragment: None, + is_maybe_not_utf8: false, + }) => PathAndQuery { + data: ByteStr::from_static(src), + query, + }, + // Yes, we reject fragments and non-utf8 + _ => panic!("static str is not valid path"), + } } /// Attempt to convert a `Bytes` buffer to a `PathAndQuery`. @@ -278,18 +220,18 @@ impl PathAndQuery { } } -impl<'a> TryFrom<&'a [u8]> for PathAndQuery { +impl TryFrom<&[u8]> for PathAndQuery { type Error = InvalidUri; #[inline] - fn try_from(s: &'a [u8]) -> Result { + fn try_from(s: &[u8]) -> Result { PathAndQuery::from_shared(Bytes::copy_from_slice(s)) } } -impl<'a> TryFrom<&'a str> for PathAndQuery { +impl TryFrom<&str> for PathAndQuery { type Error = InvalidUri; #[inline] - fn try_from(s: &'a str) -> Result { + fn try_from(s: &str) -> Result { TryFrom::try_from(s.as_bytes()) } } @@ -336,11 +278,14 @@ impl fmt::Display for PathAndQuery { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { if !self.data.is_empty() { match self.data.as_bytes()[0] { - b'/' | b'*' => write!(fmt, "{}", &self.data[..]), - _ => write!(fmt, "/{}", &self.data[..]), + b'/' | b'*' => fmt.write_str(&self.data), + _ => { + fmt.write_str("/")?; + fmt.write_str(&self.data) + } } } else { - write!(fmt, "/") + fmt.write_str("/") } } } @@ -369,16 +314,16 @@ impl PartialEq for PathAndQuery { } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { #[inline] fn eq(&self, other: &PathAndQuery) -> bool { self == &other.as_str() } } -impl<'a> PartialEq<&'a str> for PathAndQuery { +impl PartialEq<&str> for PathAndQuery { #[inline] - fn eq(&self, other: &&'a str) -> bool { + fn eq(&self, other: &&str) -> bool { self.as_str() == *other } } @@ -425,14 +370,14 @@ impl PartialOrd for str { } } -impl<'a> PartialOrd<&'a str> for PathAndQuery { +impl PartialOrd<&str> for PathAndQuery { #[inline] - fn partial_cmp(&self, other: &&'a str) -> Option { + fn partial_cmp(&self, other: &&str) -> Option { self.as_str().partial_cmp(*other) } } -impl<'a> PartialOrd for &'a str { +impl PartialOrd for &str { #[inline] fn partial_cmp(&self, other: &PathAndQuery) -> Option { self.partial_cmp(&other.as_str()) @@ -453,6 +398,145 @@ impl PartialOrd for String { } } +// Scanner implementation that is `const fn`, usable by both `from_static` +// and `from_shared`. +// ===== + +struct Scanned { + query: u16, + fragment: Option, + is_maybe_not_utf8: bool, +} + +// Per-byte character classes for the path and query scanners. +const CLASS_VALID: u8 = 0; +const CLASS_QUERY: u8 = 1; +const CLASS_FRAGMENT: u8 = 2; +const CLASS_HIGH: u8 = 3; +const CLASS_INVALID: u8 = 4; + +const fn build_path_map() -> [u8; 256] { + let mut t = [CLASS_INVALID; 256]; + let mut i = 0; + while i < 256 { + // See https://url.spec.whatwg.org/#path-state + t[i] = match i as u8 { + b'?' => CLASS_QUERY, + b'#' => CLASS_FRAGMENT, + + // Bytes that don't need to be percent-encoded in the path. + 0x21 | 0x24..=0x3B | 0x3D | 0x40..=0x5F | 0x61..=0x7A | 0x7C | 0x7E => CLASS_VALID, + + // Potentially utf8, checked later. + 0x80..=0xFF => CLASS_HIGH, + + // Should be percent-encoded, but accepted for parity with clients + // that send them as-is (e.g. JSON embedded in the path). + b'"' | b'{' | b'}' => CLASS_VALID, + + _ => CLASS_INVALID, + }; + i += 1; + } + t +} + +const fn build_query_map() -> [u8; 256] { + let mut t = [CLASS_INVALID; 256]; + let mut i = 0; + while i < 256 { + // See https://url.spec.whatwg.org/#query-state + t[i] = match i as u8 { + b'#' => CLASS_FRAGMENT, + + // Allowed: 0x21 / 0x24 - 0x3B / 0x3D / 0x3F - 0x7E + 0x21 | 0x24..=0x3B | 0x3D | 0x3F..=0x7E => CLASS_VALID, + + 0x80..=0xFF => CLASS_HIGH, + + _ => CLASS_INVALID, + }; + i += 1; + } + t +} + +const PATH_MAP: [u8; 256] = build_path_map(); +const QUERY_MAP: [u8; 256] = build_query_map(); + +const fn scan_path_and_query(bytes: &[u8]) -> Result { + let mut i = 0; + let mut query = NONE; + let mut fragment = None; + + let mut is_maybe_not_utf8 = false; + + if bytes.is_empty() { + return Err(ErrorKind::Empty); + } + + if bytes.len() > MAX_LEN { + return Err(ErrorKind::TooLong); + } + + if bytes.len() == 1 && bytes[0] == b'*' { + return Ok(Scanned { + query, + fragment, + is_maybe_not_utf8: false, + }); + } + + if !matches!(bytes[0], b'/' | b'?' | b'#') { + return Err(ErrorKind::PathDoesNotStartWithSlash); + } + + while i < bytes.len() { + match PATH_MAP[bytes[i] as usize] { + CLASS_VALID => {} + CLASS_QUERY => { + debug_assert!(query == NONE); + query = i as u16; + i += 1; + break; + } + CLASS_FRAGMENT => { + fragment = Some(i as u16); + break; + } + CLASS_HIGH => { + is_maybe_not_utf8 = true; + } + _ => return Err(ErrorKind::InvalidUriChar), + } + i += 1; + } + + // query ... + if query != NONE { + while i < bytes.len() { + match QUERY_MAP[bytes[i] as usize] { + CLASS_VALID => {} + CLASS_HIGH => { + is_maybe_not_utf8 = true; + } + CLASS_FRAGMENT => { + fragment = Some(i as u16); + break; + } + _ => return Err(ErrorKind::InvalidUriChar), + } + i += 1; + } + } + + Ok(Scanned { + query, + fragment, + is_maybe_not_utf8, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -566,6 +650,51 @@ mod tests { assert_eq!(Some("pizza=🍕"), pq("/test?pizza=🍕").query()); } + #[test] + fn rejects_invalid_utf8_in_path() { + PathAndQuery::try_from(&[b'/', 0xFF][..]).expect_err("reject invalid utf8"); + } + + #[test] + fn rejects_invalid_utf8_in_query() { + PathAndQuery::try_from(&[b'/', b'a', b'?', 0xFF][..]).expect_err("reject invalid utf8"); + } + + #[test] + fn rejects_empty_string() { + PathAndQuery::try_from("").expect_err("reject empty str"); + } + + #[test] + fn requires_starting_with_slash() { + PathAndQuery::try_from("sneaky").expect_err("reject missing slash"); + } + + #[test] + fn rejects_del_in_path() { + PathAndQuery::try_from(&[b'/', 0x7F][..]).expect_err("reject DEL"); + } + + #[test] + fn rejects_del_in_query() { + PathAndQuery::try_from(&[b'/', b'a', b'?', 0x7F][..]).expect_err("reject DEL"); + } + + #[test] + fn rejects_too_long_path_and_query() { + let path = format!("/{}?query", "a".repeat(MAX_LEN)); + let err = PathAndQuery::try_from(path).expect_err("reject overly long path and query"); + assert_eq!(err.0, ErrorKind::TooLong); + } + + #[test] + fn accepts_max_length_path_and_query() { + let path = format!("/{}?", "a".repeat(MAX_LEN - 2)); + let path_and_query = PathAndQuery::try_from(path).expect("accept maximum length"); + assert_eq!(path_and_query.as_str().len(), MAX_LEN); + assert_eq!(path_and_query.query(), Some("")); + } + #[test] fn json_is_fine() { assert_eq!( diff --git a/src/uri/scheme.rs b/src/uri/scheme.rs index dbcc8c3f..bf4d59c3 100644 --- a/src/uri/scheme.rs +++ b/src/uri/scheme.rs @@ -67,10 +67,10 @@ impl Scheme { } } -impl<'a> TryFrom<&'a [u8]> for Scheme { +impl TryFrom<&[u8]> for Scheme { type Error = InvalidUri; #[inline] - fn try_from(s: &'a [u8]) -> Result { + fn try_from(s: &[u8]) -> Result { use self::Scheme2::*; match Scheme2::parse_exact(s)? { @@ -89,10 +89,10 @@ impl<'a> TryFrom<&'a [u8]> for Scheme { } } -impl<'a> TryFrom<&'a str> for Scheme { +impl TryFrom<&str> for Scheme { type Error = InvalidUri; #[inline] - fn try_from(s: &'a str) -> Result { + fn try_from(s: &str) -> Result { TryFrom::try_from(s.as_bytes()) } } diff --git a/tests/header_map.rs b/tests/header_map.rs index 9859b0a8..f6210135 100644 --- a/tests/header_map.rs +++ b/tests/header_map.rs @@ -55,12 +55,53 @@ fn with_capacity_overflow() { HeaderMap::::with_capacity(24_577); } +#[test] +fn extend_size_hint_above_capacity() { + // A `HeaderMap` may hold more values than the table can index when many + // values are appended under one name, so an exact size hint can exceed the + // largest `reserve` request. Extending must not panic in that case. + let name = HeaderName::from_static("h"); + let value = HeaderValue::from_static("0"); + let pairs: Vec<(HeaderName, HeaderValue)> = + std::iter::repeat_with(|| (name.clone(), value.clone())) + .take(24_577) + .collect(); + + let map = HeaderMap::from_iter(pairs); + assert_eq!(map.len(), 24_577); + assert_eq!(map.keys_len(), 1); +} + #[test] #[should_panic] fn reserve_overflow() { // See https://github.com/hyperium/http/issues/352 let mut headers = HeaderMap::::with_capacity(0); - headers.reserve(std::usize::MAX); // next_power_of_two overflows + headers.reserve(usize::MAX); // next_power_of_two overflows +} + +#[test] +fn reserve() { + let mut headers = HeaderMap::::default(); + assert_eq!(headers.capacity(), 0); + + let requested_cap = 8; + headers.reserve(requested_cap); + + let reserved_cap = headers.capacity(); + assert!( + reserved_cap >= requested_cap, + "requested {} capacity, but it reserved only {} entries", + requested_cap, + reserved_cap, + ); + + for i in 0..requested_cap { + let name = format!("h{i}").parse::().unwrap(); + headers.insert(name, i); + } + + assert_eq!(headers.capacity(), reserved_cap, "unexpected reallocation"); } #[test] @@ -648,3 +689,86 @@ fn ensure_miri_sharedreadonly_not_violated() { let _foo = &headers.iter().next(); } + +#[test] +fn ensure_miri_itermut_not_violated() { + let mut headers = HeaderMap::::default(); + headers.insert(HeaderName::from_static("hello"), 1u32); + headers.insert(HeaderName::from_static("zomg"), 2u32); + + let mut iter = headers.iter_mut(); + let (_, first) = iter.next().unwrap(); + let (_, second) = iter.next().unwrap(); + + *first += 10; + *second += 20; +} + +#[test] +fn ensure_miri_valueitermut_not_violated() { + let mut headers = HeaderMap::::default(); + headers.insert(HeaderName::from_static("hello"), 1u32); + headers.append(HeaderName::from_static("hello"), 2u32); + headers.append(HeaderName::from_static("hello"), 3u32); + + let mut entry = match headers.entry(HeaderName::from_static("hello")) { + Entry::Occupied(entry) => entry, + Entry::Vacant(_) => panic!(), + }; + + let mut iter = entry.iter_mut(); + let first = iter.next().unwrap(); + let second = iter.next().unwrap(); + + *first += 10; + *second += 20; +} + +#[test] +fn into_iter_drop_panic_after_yielding_extra_value_double_drops() { + use std::panic::{catch_unwind, AssertUnwindSafe}; + + struct ManuallyAllocated { + ptr: *mut u8, + panic_on_drop: bool, + } + + impl ManuallyAllocated { + fn new(byte: u8, panic_on_drop: bool) -> Self { + Self { + ptr: Box::into_raw(Box::new(byte)), + panic_on_drop, + } + } + } + + impl Drop for ManuallyAllocated { + fn drop(&mut self) { + unsafe { + drop(Box::from_raw(self.ptr)); + } + + if self.panic_on_drop { + panic!("intentional drop panic"); + } + } + } + + let mut map: HeaderMap = HeaderMap::default(); + map.append("x-first", ManuallyAllocated::new(1, false)); + map.append("x-first", ManuallyAllocated::new(2, false)); + map.insert("x-second", ManuallyAllocated::new(3, true)); + + let mut iter = map.into_iter(); + + // HeaderMap::IntoIter yields extra values with ptr::read from + // self.extra_values and relies on Drop setting self.extra_values.len() to + // zero after the iterator has been fully consumed. If a later value's Drop + // panics while IntoIter::drop is draining the iterator, that set_len(0) is + // skipped. The Vec then drops already-yielded extra value slots again. The + // safe sequence below therefore double-frees byte 2 under Miri. + drop(iter.next().unwrap()); + drop(iter.next().unwrap()); + + let _ = catch_unwind(AssertUnwindSafe(|| drop(iter))); +} diff --git a/tests/header_map_fuzz.rs b/tests/header_map_fuzz.rs index 40db0494..14f172d0 100644 --- a/tests/header_map_fuzz.rs +++ b/tests/header_map_fuzz.rs @@ -3,8 +3,8 @@ use http::*; use quickcheck::{Arbitrary, Gen, QuickCheck, TestResult}; use rand::rngs::StdRng; -use rand::seq::SliceRandom; -use rand::{Rng, SeedableRng}; +use rand::seq::IndexedRandom; +use rand::{RngExt, SeedableRng}; use std::collections::HashMap; @@ -76,12 +76,12 @@ impl Fuzz { let mut steps = vec![]; let mut expect = AltMap::default(); - let num = rng.gen_range(5..500); + let num = rng.random_range(5..500); let weight = Weight { - insert: rng.gen_range(1..10), - remove: rng.gen_range(1..10), - append: rng.gen_range(1..10), + insert: rng.random_range(1..10), + remove: rng.random_range(1..10), + append: rng.random_range(1..10), }; while steps.len() < num { @@ -111,8 +111,8 @@ impl Fuzz { } impl Arbitrary for Fuzz { - fn arbitrary(_: &mut Gen) -> Self { - Self::new(rand::thread_rng().gen()) + fn arbitrary(g: &mut Gen) -> Self { + Self::new(Arbitrary::arbitrary(g)) } } @@ -130,7 +130,7 @@ impl AltMap { fn gen_action(&mut self, weight: &Weight, rng: &mut StdRng) -> Action { let sum = weight.insert + weight.remove + weight.append; - let mut num = rng.gen_range(0..sum); + let mut num = rng.random_range(0..sum); if num < weight.insert { return self.gen_insert(rng); @@ -180,7 +180,7 @@ impl AltMap { /// Negative numbers weigh finding an existing header higher fn gen_name(&self, weight: i32, rng: &mut StdRng) -> HeaderName { - let mut existing = rng.gen_ratio(1, weight.abs() as u32); + let mut existing = rng.random_ratio(1, weight.abs() as u32); if weight < 0 { existing = !existing; @@ -202,7 +202,7 @@ impl AltMap { if self.map.is_empty() { None } else { - let n = rng.gen_range(0..self.map.len()); + let n = rng.random_range(0..self.map.len()); self.map.keys().nth(n).map(Clone::clone) } } @@ -337,7 +337,7 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { header::X_XSS_PROTECTION, ]; - if g.gen_ratio(1, 2) { + if g.random_ratio(1, 2) { STANDARD_HEADERS.choose(g).unwrap().clone() } else { let value = gen_string(g, 1, 25); diff --git a/util/src/main.rs b/util/src/main.rs index 915cf0b8..336b2347 100644 --- a/util/src/main.rs +++ b/util/src/main.rs @@ -68,7 +68,7 @@ standard_headers! { /// not to compress if a server use more than 80 % of its computational /// power. /// - /// As long as the identity value, meaning no encryption, is not explicitly + /// As long as the identity value, meaning no compression, is not explicitly /// forbidden, by an identity;q=0 or a *;q=0 without another explicitly set /// value for identity, the server must never send back a 406 Not Acceptable /// error.