diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index ba8cc6e6d..bef17fb08 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -7,6 +7,9 @@ * `ResponseBuilder::append_header` method which allows using typed headers. [#1869] * `TestRequest::insert_header` method which allows using typed headers. [#1869] * `ContentEncoding` implements all necessary header traits. [#1912] +* `HeaderMap::len_keys` has the behavior of the old `len` method. [#1964] +* `HeaderMap::drain` as an efficient draining iterator. [#1964] +* Implement `IntoIterator` for owned `HeaderMap`. [#1964] * `trust-dns` optional feature to enable `trust-dns-resolver` as client dns resolver. [#1969] ### Changed @@ -20,6 +23,9 @@ * `client::error::ConnectError` Resolver variant contains `Box` type [#1905] * `client::ConnectorConfig` default timeout changed to 5 seconds. [#1905] * Simplify `BlockingError` type to a struct. It's only triggered with blocking thread pool is dead. [#1957] +* `HeaderMap::len` now returns number of values instead of number of keys. [#1964] +* `HeaderMap::insert` now returns iterator of removed values. [#1964] +* `HeaderMap::remove` now returns iterator of removed values. [#1964] ### Removed * `ResponseBuilder::set`; use `ResponseBuilder::insert_header`. [#1869] @@ -30,6 +36,9 @@ * `actors` optional feature. [#1969] * `ResponseError` impl for `actix::MailboxError`. [#1969] +### Documentation +* Vastly improve docs and add examples for `HeaderMap`. [#1964] + [#1869]: https://github.com/actix/actix-web/pull/1869 [#1894]: https://github.com/actix/actix-web/pull/1894 [#1903]: https://github.com/actix/actix-web/pull/1903 @@ -37,6 +46,7 @@ [#1905]: https://github.com/actix/actix-web/pull/1905 [#1912]: https://github.com/actix/actix-web/pull/1912 [#1957]: https://github.com/actix/actix-web/pull/1957 +[#1964]: https://github.com/actix/actix-web/pull/1964 [#1969]: https://github.com/actix/actix-web/pull/1969 diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 9f62639f8..e974e61df 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -52,7 +52,6 @@ bytes = "1" bytestring = "1" cookie = { version = "0.14.1", features = ["percent-encode"] } derive_more = "0.99.5" -either = "1.5.3" encoding_rs = "0.8" futures-channel = { version = "0.3.7", default-features = false, features = ["alloc"] } futures-core = { version = "0.3.7", default-features = false, features = ["alloc"] } diff --git a/actix-http/src/client/h1proto.rs b/actix-http/src/client/h1proto.rs index 24f4207e8..92c3c0e1b 100644 --- a/actix-http/src/client/h1proto.rs +++ b/actix-http/src/client/h1proto.rs @@ -48,11 +48,11 @@ where match wrt.get_mut().split().freeze().try_into_value() { Ok(value) => match head { RequestHeadType::Owned(ref mut head) => { - head.headers.insert(HOST, value) + head.headers.insert(HOST, value); } RequestHeadType::Rc(_, ref mut extra_headers) => { let headers = extra_headers.get_or_insert(HeaderMap::new()); - headers.insert(HOST, value) + headers.insert(HOST, value); } }, Err(e) => log::error!("Can not set HOST header {}", e), diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index 9da958563..122a815d5 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -830,8 +830,8 @@ mod tests { .get_all(SET_COOKIE) .map(|v| v.to_str().unwrap().to_owned()) .collect(); - assert_eq!(val[1], "c1=cookie1"); - assert_eq!(val[0], "c2=cookie2"); + assert_eq!(val[0], "c1=cookie1"); + assert_eq!(val[1], "c2=cookie2"); } #[test] diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index bd8287b26..2932a7dce 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -144,104 +144,54 @@ pub(crate) trait MessageType: Sized { let k = key.as_str().as_bytes(); let k_len = k.len(); - match value { - Value::One(ref val) => { - let v = val.as_ref(); - let v_len = v.len(); + // TODO: drain? + for val in value.iter() { + let v = val.as_ref(); + let v_len = v.len(); - // key length + value length + colon + space + \r\n - let len = k_len + v_len + 4; + // key length + value length + colon + space + \r\n + let len = k_len + v_len + 4; - if len > remaining { - // not enough room in buffer for this header; reserve more space - - // SAFETY: all the bytes written up to position "pos" are initialized - // the written byte count and pointer advancement are kept in sync - unsafe { - dst.advance_mut(pos); - } - - pos = 0; - dst.reserve(len * 2); - remaining = dst.capacity() - dst.len(); - - // re-assign buf raw pointer since it's possible that the buffer was - // reallocated and/or resized - buf = dst.chunk_mut().as_mut_ptr(); - } - - // SAFETY: on each write, it is enough to ensure that the advancement of the - // cursor matches the number of bytes written + if len > remaining { + // SAFETY: all the bytes written up to position "pos" are initialized + // the written byte count and pointer advancement are kept in sync unsafe { - // use upper Camel-Case - if camel_case { - write_camel_case(k, from_raw_parts_mut(buf, k_len)) - } else { - write_data(k, buf, k_len) - } - - buf = buf.add(k_len); - - write_data(b": ", buf, 2); - buf = buf.add(2); - - write_data(v, buf, v_len); - buf = buf.add(v_len); - - write_data(b"\r\n", buf, 2); - buf = buf.add(2); + dst.advance_mut(pos); } - pos += len; - remaining -= len; + pos = 0; + dst.reserve(len * 2); + remaining = dst.capacity() - dst.len(); + + // re-assign buf raw pointer since it's possible that the buffer was + // reallocated and/or resized + buf = dst.chunk_mut().as_mut_ptr(); } - Value::Multi(ref vec) => { - for val in vec { - let v = val.as_ref(); - let v_len = v.len(); - let len = k_len + v_len + 4; - - if len > remaining { - // SAFETY: all the bytes written up to position "pos" are initialized - // the written byte count and pointer advancement are kept in sync - unsafe { - dst.advance_mut(pos); - } - pos = 0; - dst.reserve(len * 2); - remaining = dst.capacity() - dst.len(); - - // re-assign buf raw pointer since it's possible that the buffer was - // reallocated and/or resized - buf = dst.chunk_mut().as_mut_ptr(); - } - - // SAFETY: on each write, it is enough to ensure that the advancement of - // the cursor matches the number of bytes written - unsafe { - if camel_case { - write_camel_case(k, from_raw_parts_mut(buf, k_len)); - } else { - write_data(k, buf, k_len); - } - - buf = buf.add(k_len); - - write_data(b": ", buf, 2); - buf = buf.add(2); - - write_data(v, buf, v_len); - buf = buf.add(v_len); - - write_data(b"\r\n", buf, 2); - buf = buf.add(2); - }; - - pos += len; - remaining -= len; + // SAFETY: on each write, it is enough to ensure that the advancement of + // the cursor matches the number of bytes written + unsafe { + if camel_case { + // use Camel-Case headers + write_camel_case(k, from_raw_parts_mut(buf, k_len)); + } else { + write_data(k, buf, k_len); } - } + + buf = buf.add(k_len); + + write_data(b": ", buf, 2); + buf = buf.add(2); + + write_data(v, buf, v_len); + buf = buf.add(v_len); + + write_data(b"\r\n", buf, 2); + buf = buf.add(2); + }; + + pos += len; + remaining -= len; } }); diff --git a/actix-http/src/header/as_name.rs b/actix-http/src/header/as_name.rs new file mode 100644 index 000000000..68456e3f7 --- /dev/null +++ b/actix-http/src/header/as_name.rs @@ -0,0 +1,46 @@ +//! Helper trait for types that can be effectively borrowed as a [HeaderValue]. + +use std::{borrow::Cow, str::FromStr}; + +use http::header::{HeaderName, InvalidHeaderName}; + +pub trait AsHeaderName: Sealed {} + +pub trait Sealed { + fn try_as_name(&self) -> Result, InvalidHeaderName>; +} + +impl Sealed for HeaderName { + fn try_as_name(&self) -> Result, InvalidHeaderName> { + Ok(Cow::Borrowed(self)) + } +} +impl AsHeaderName for HeaderName {} + +impl Sealed for &HeaderName { + fn try_as_name(&self) -> Result, InvalidHeaderName> { + Ok(Cow::Borrowed(*self)) + } +} +impl AsHeaderName for &HeaderName {} + +impl Sealed for &str { + fn try_as_name(&self) -> Result, InvalidHeaderName> { + HeaderName::from_str(self).map(Cow::Owned) + } +} +impl AsHeaderName for &str {} + +impl Sealed for String { + fn try_as_name(&self) -> Result, InvalidHeaderName> { + HeaderName::from_str(self).map(Cow::Owned) + } +} +impl AsHeaderName for String {} + +impl Sealed for &String { + fn try_as_name(&self) -> Result, InvalidHeaderName> { + HeaderName::from_str(self).map(Cow::Owned) + } +} +impl AsHeaderName for &String {} diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 8f20f3e6f..106e44edb 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -1,332 +1,563 @@ -use std::{ - collections::hash_map::{self, Entry}, - convert::TryFrom, -}; +//! A multi-value [`HeaderMap`] and its iterators. + +use std::{borrow::Cow, collections::hash_map, ops}; use ahash::AHashMap; -use either::Either; use http::header::{HeaderName, HeaderValue}; use smallvec::{smallvec, SmallVec}; +use crate::header::AsHeaderName; + /// A multi-map of HTTP headers. /// -/// `HeaderMap` is a "multi-map" of [`HeaderName`] to one or more values. +/// `HeaderMap` is a "multi-map" of [`HeaderName`] to one or more [`HeaderValue`]s. +/// +/// # Examples +/// ``` +/// use actix_http::http::{header, HeaderMap, HeaderValue}; +/// +/// let mut map = HeaderMap::new(); +/// +/// map.insert(header::CONTENT_TYPE, HeaderValue::from_static("text/plain")); +/// map.insert(header::ORIGIN, HeaderValue::from_static("example.com")); +/// +/// assert!(map.contains_key(header::CONTENT_TYPE)); +/// assert!(map.contains_key(header::ORIGIN)); +/// +/// let mut removed = map.remove(header::ORIGIN); +/// assert_eq!(removed.next().unwrap(), "example.com"); +/// +/// assert!(!map.contains_key(header::ORIGIN)); +/// ``` #[derive(Debug, Clone, Default)] pub struct HeaderMap { pub(crate) inner: AHashMap, } +/// A bespoke non-empty list for HeaderMap values. #[derive(Debug, Clone)] -pub(crate) enum Value { - One(HeaderValue), - Multi(SmallVec<[HeaderValue; 4]>), +pub(crate) struct Value { + inner: SmallVec<[HeaderValue; 4]>, } impl Value { - fn first(&self) -> &HeaderValue { - match self { - Value::One(ref val) => val, - Value::Multi(ref val) => &val[0], + fn one(val: HeaderValue) -> Self { + Self { + inner: smallvec![val], } } + fn first(&self) -> &HeaderValue { + &self.inner[0] + } + fn first_mut(&mut self) -> &mut HeaderValue { - match self { - Value::One(ref mut val) => val, - Value::Multi(ref mut val) => &mut val[0], - } + &mut self.inner[0] } - fn append(&mut self, val: HeaderValue) { - match self { - Value::One(_) => { - let data = std::mem::replace(self, Value::Multi(smallvec![val])); - match data { - Value::One(val) => self.append(val), - Value::Multi(_) => unreachable!(), - } - } - Value::Multi(ref mut vec) => vec.push(val), - } + fn append(&mut self, new_val: HeaderValue) { + self.inner.push(new_val) + } +} + +impl ops::Deref for Value { + type Target = SmallVec<[HeaderValue; 4]>; + + fn deref(&self) -> &Self::Target { + &self.inner } } impl HeaderMap { /// Create an empty `HeaderMap`. /// - /// The map will be created without any capacity. This function will not - /// allocate. + /// The map will be created without any capacity; this function will not allocate. + /// + /// # Examples + /// ``` + /// # use actix_http::http::HeaderMap; + /// let map = HeaderMap::new(); + /// + /// assert!(map.is_empty()); + /// assert_eq!(0, map.capacity()); + /// ``` pub fn new() -> Self { - HeaderMap { - inner: AHashMap::default(), - } + HeaderMap::default() } /// Create an empty `HeaderMap` with the specified capacity. /// - /// The returned map will allocate internal storage in order to hold about - /// `capacity` elements without reallocating. However, this is a "best - /// effort" as there are usage patterns that could cause additional - /// allocations before `capacity` headers are stored in the map. + /// The map will be able to hold at least `capacity` elements without needing to reallocate. + /// If `capacity` is 0, the map will be created without allocating. /// - /// More capacity than requested may be allocated. - pub fn with_capacity(capacity: usize) -> HeaderMap { + /// # Examples + /// ``` + /// # use actix_http::http::HeaderMap; + /// let map = HeaderMap::with_capacity(16); + /// + /// assert!(map.is_empty()); + /// assert!(map.capacity() >= 16); + /// ``` + pub fn with_capacity(capacity: usize) -> Self { HeaderMap { - inner: AHashMap::with_capacity_and_hasher(capacity, Default::default()), + inner: AHashMap::with_capacity(capacity), } } - /// Returns the number of keys stored in the map. + /// Create new `HeaderMap` from a `http::HeaderMap`-like drain. + pub(crate) fn from_drain(mut drain: I) -> Self + where + I: Iterator, HeaderValue)>, + { + let (first_name, first_value) = match drain.next() { + None => return HeaderMap::new(), + Some((name, val)) => { + let name = name.expect("drained first item had no name"); + (name, val) + } + }; + + let (lb, ub) = drain.size_hint(); + let capacity = ub.unwrap_or(lb); + + let mut map = HeaderMap::with_capacity(capacity); + map.append(first_name.clone(), first_value); + + let (map, _) = + drain.fold((map, first_name), |(mut map, prev_name), (name, value)| { + let name = name.unwrap_or(prev_name); + map.append(name.clone(), value); + (map, name) + }); + + map + } + + /// Returns the number of values stored in the map. /// - /// This number could be be less than or equal to actual headers stored in - /// the map. + /// See also: [`len_keys`](Self::len_keys). + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// assert_eq!(map.len(), 0); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// assert_eq!(map.len(), 2); + /// + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// assert_eq!(map.len(), 3); + /// ``` pub fn len(&self) -> usize { + self.inner + .iter() + .fold(0, |acc, (_, values)| acc + values.len()) + } + + /// Returns the number of _keys_ stored in the map. + /// + /// The number of values stored will be at least this number. See also: [`Self::len`]. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// assert_eq!(map.len_keys(), 0); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// assert_eq!(map.len_keys(), 2); + /// + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// assert_eq!(map.len_keys(), 2); + /// ``` + pub fn len_keys(&self) -> usize { self.inner.len() } /// Returns true if the map contains no elements. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// assert!(map.is_empty()); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// assert!(!map.is_empty()); + /// ``` pub fn is_empty(&self) -> bool { self.inner.len() == 0 } - /// Clears the map, removing all key-value pairs. Keeps the allocated memory - /// for reuse. + /// Clears the map, removing all name-value pairs. + /// + /// Keeps the allocated memory for reuse. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// assert_eq!(map.len(), 2); + /// + /// map.clear(); + /// assert!(map.is_empty()); + /// ``` pub fn clear(&mut self) { self.inner.clear(); } - /// Returns the number of headers the map can hold without reallocating. + fn get_value(&self, key: impl AsHeaderName) -> Option<&Value> { + match key.try_as_name().ok()? { + Cow::Borrowed(name) => self.inner.get(name), + Cow::Owned(name) => self.inner.get(&name), + } + } + + /// Returns a reference to the _first_ value associated with a header name. /// - /// This number is an approximation as certain usage patterns could cause - /// additional allocations before the returned capacity is filled. + /// Returns `None` if there is no value associated with the key. + /// + /// Even when multiple values are associated with the key, the "first" one is returned but is + /// not guaranteed to be chosen with any particular order; though, the returned item will be + /// consistent for each call to `get` if the map has not changed. + /// + /// See also: [`get_all`](Self::get_all). + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// + /// let cookie = map.get(header::SET_COOKIE).unwrap(); + /// assert_eq!(cookie, "one=1"); + /// + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// assert_eq!(map.get(header::SET_COOKIE).unwrap(), "one=1"); + /// + /// assert_eq!(map.get(header::SET_COOKIE), map.get("set-cookie")); + /// assert_eq!(map.get(header::SET_COOKIE), map.get("Set-Cookie")); + /// + /// assert!(map.get(header::HOST).is_none()); + /// assert!(map.get("INVALID HEADER NAME").is_none()); + /// ``` + pub fn get(&self, key: impl AsHeaderName) -> Option<&HeaderValue> { + self.get_value(key).map(|val| val.first()) + } + + /// Returns a mutable reference to the _first_ value associated a header name. + /// + /// Returns `None` if there is no value associated with the key. + /// + /// Even when multiple values are associated with the key, the "first" one is returned but is + /// not guaranteed to be chosen with any particular order; though, the returned item will be + /// consistent for each call to `get_mut` if the map has not changed. + /// + /// See also: [`get_all`](Self::get_all). + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// + /// let mut cookie = map.get_mut(header::SET_COOKIE).unwrap(); + /// assert_eq!(cookie, "one=1"); + /// + /// *cookie = HeaderValue::from_static("three=3"); + /// assert_eq!(map.get(header::SET_COOKIE).unwrap(), "three=3"); + /// + /// assert!(map.get(header::HOST).is_none()); + /// assert!(map.get("INVALID HEADER NAME").is_none()); + /// ``` + pub fn get_mut(&mut self, key: impl AsHeaderName) -> Option<&mut HeaderValue> { + match key.try_as_name().ok()? { + Cow::Borrowed(name) => self.inner.get_mut(name).map(|v| v.first_mut()), + Cow::Owned(name) => self.inner.get_mut(&name).map(|v| v.first_mut()), + } + } + + /// Returns an iterator over all values associated with a header name. + /// + /// The returned iterator does not incur any allocations and will yield no items if there are no + /// values associated with the key. Iteration order is **not** guaranteed to be the same as + /// insertion order. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// let mut none_iter = map.get_all(header::ORIGIN); + /// assert!(none_iter.next().is_none()); + /// + /// map.insert(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// + /// let mut set_cookies_iter = map.get_all(header::SET_COOKIE); + /// assert_eq!(set_cookies_iter.next().unwrap(), "one=1"); + /// assert_eq!(set_cookies_iter.next().unwrap(), "two=2"); + /// assert!(set_cookies_iter.next().is_none()); + /// ``` + pub fn get_all(&self, key: impl AsHeaderName) -> GetAll<'_> { + GetAll::new(self.get_value(key)) + } + + // TODO: get_all_mut ? + + /// Returns `true` if the map contains a value for the specified key. + /// + /// Invalid header names will simply return false. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// assert!(!map.contains_key(header::ACCEPT)); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// assert!(map.contains_key(header::ACCEPT)); + /// ``` + pub fn contains_key(&self, key: impl AsHeaderName) -> bool { + match key.try_as_name() { + Ok(Cow::Borrowed(name)) => self.inner.contains_key(name), + Ok(Cow::Owned(name)) => self.inner.contains_key(&name), + Err(_) => false, + } + } + + /// Inserts a name-value pair into the map. + /// + /// If the map already contained this key, the new value is associated with the key and all + /// previous values are removed and returned as a `Removed` iterator. The key is not updated; + /// this matters for types that can be `==` without being identical. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.insert(header::ACCEPT, HeaderValue::from_static("text/plain")); + /// assert!(map.contains_key(header::ACCEPT)); + /// assert_eq!(map.len(), 1); + /// + /// let mut removed = map.insert(header::ACCEPT, HeaderValue::from_static("text/csv")); + /// assert_eq!(removed.next().unwrap(), "text/plain"); + /// assert!(removed.next().is_none()); + /// + /// assert_eq!(map.len(), 1); + /// ``` + pub fn insert(&mut self, key: HeaderName, val: HeaderValue) -> Removed { + let value = self.inner.insert(key, Value::one(val)); + Removed::new(value) + } + + /// Inserts a name-value pair into the map. + /// + /// If the map already contained this key, the new value is added to the list of values + /// currently associated with the key. The key is not updated; this matters for types that can + /// be `==` without being identical. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.append(header::HOST, HeaderValue::from_static("example.com")); + /// assert_eq!(map.len(), 1); + /// + /// map.append(header::ACCEPT, HeaderValue::from_static("text/csv")); + /// assert_eq!(map.len(), 2); + /// + /// map.append(header::ACCEPT, HeaderValue::from_static("text/html")); + /// assert_eq!(map.len(), 3); + /// ``` + pub fn append(&mut self, key: HeaderName, value: HeaderValue) { + match self.inner.entry(key) { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().append(value); + } + hash_map::Entry::Vacant(entry) => { + entry.insert(Value::one(value)); + } + }; + } + + /// Removes all headers for a particular header name from the map. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// map.append(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("one=2")); + /// + /// assert_eq!(map.len(), 2); + /// + /// let mut removed = map.remove(header::SET_COOKIE); + /// assert_eq!(removed.next().unwrap(), "one=1"); + /// assert_eq!(removed.next().unwrap(), "one=2"); + /// assert!(removed.next().is_none()); + /// + /// assert!(map.is_empty()); + pub fn remove(&mut self, key: impl AsHeaderName) -> Removed { + let value = match key.try_as_name() { + Ok(Cow::Borrowed(name)) => self.inner.remove(name), + Ok(Cow::Owned(name)) => self.inner.remove(&name), + Err(_) => None, + }; + + Removed::new(value) + } + + /// Returns the number of single-value headers the map can hold without needing to reallocate. + /// + /// Since this is a multi-value map, the actual capacity is much larger when considering + /// each header name can be associated with an arbitrary number of values. The effect is that + /// the size of `len` may be greater than `capacity` since it counts all the values. + /// Conversely, [`len_keys`](Self::len_keys) will never be larger than capacity. + /// + /// # Examples + /// ``` + /// # use actix_http::http::HeaderMap; + /// let map = HeaderMap::with_capacity(16); + /// + /// assert!(map.is_empty()); + /// assert!(map.capacity() >= 16); + /// ``` pub fn capacity(&self) -> usize { self.inner.capacity() } - /// Reserves capacity for at least `additional` more headers to be inserted - /// into the `HeaderMap`. + /// Reserves capacity for at least `additional` more headers to be inserted in the map. /// - /// The header map may reserve more space to avoid frequent reallocations. - /// Like with `with_capacity`, this will be a "best effort" to avoid - /// allocations until `additional` more headers are inserted. Certain usage - /// patterns could cause additional allocations before the number is - /// reached. + /// The header map may reserve more space to avoid frequent reallocations. Additional capacity + /// only considers single-value headers. + /// + /// # Panics + /// Panics if the new allocation size overflows usize. + /// + /// # Examples + /// ``` + /// # use actix_http::http::HeaderMap; + /// let mut map = HeaderMap::with_capacity(2); + /// assert!(map.capacity() >= 2); + /// + /// map.reserve(100); + /// assert!(map.capacity() >= 102); + /// + /// assert!(map.is_empty()); + /// ``` pub fn reserve(&mut self, additional: usize) { self.inner.reserve(additional) } - /// Returns a reference to the value associated with the key. + /// An iterator over all name-value pairs. /// - /// If there are multiple values associated with the key, then the first one - /// is returned. Use `get_all` to get all values associated with a given - /// key. Returns `None` if there are no values associated with the key. - pub fn get(&self, name: N) -> Option<&HeaderValue> { - self.get2(name).map(|v| v.first()) - } - - fn get2(&self, name: N) -> Option<&Value> { - match name.as_name() { - Either::Left(name) => self.inner.get(name), - Either::Right(s) => { - if let Ok(name) = HeaderName::try_from(s) { - self.inner.get(&name) - } else { - None - } - } - } - } - - /// Returns an iterator of all values associated with a key. + /// Names will be yielded for each associated value. So, if a key has 3 associated values, it + /// will be yielded 3 times. The iteration order should be considered arbitrary. /// - /// The returned view does not incur any allocations and allows iterating the values associated - /// with the key. Returns `None` if there are no values associated with the key. Iteration order - /// is not guaranteed to be the same as insertion order. - pub fn get_all(&self, name: N) -> GetAll<'_> { - GetAll { - idx: 0, - item: self.get2(name), - } - } - - /// Returns a mutable reference to the value associated with the key. + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); /// - /// If there are multiple values associated with the key, then the first one - /// is returned. Use `entry` to get all values associated with a given - /// key. Returns `None` if there are no values associated with the key. - pub fn get_mut(&mut self, name: N) -> Option<&mut HeaderValue> { - match name.as_name() { - Either::Left(name) => self.inner.get_mut(name).map(|v| v.first_mut()), - Either::Right(s) => { - if let Ok(name) = HeaderName::try_from(s) { - self.inner.get_mut(&name).map(|v| v.first_mut()) - } else { - None - } - } - } - } - - /// Returns true if the map contains a value for the specified key. - pub fn contains_key(&self, key: N) -> bool { - match key.as_name() { - Either::Left(name) => self.inner.contains_key(name), - Either::Right(s) => { - if let Ok(name) = HeaderName::try_from(s) { - self.inner.contains_key(&name) - } else { - false - } - } - } - } - - /// An iterator visiting all key-value pairs. + /// let mut iter = map.iter(); + /// assert!(iter.next().is_none()); /// - /// The iteration order is arbitrary, but consistent across platforms for - /// the same crate version. Each key will be yielded once per associated - /// value. So, if a key has 3 associated values, it will be yielded 3 times. + /// map.append(header::HOST, HeaderValue::from_static("duck.com")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// + /// let mut iter = map.iter(); + /// assert!(iter.next().is_some()); + /// assert!(iter.next().is_some()); + /// assert!(iter.next().is_some()); + /// assert!(iter.next().is_none()); + /// + /// let pairs = map.iter().collect::>(); + /// assert!(pairs.contains(&(&header::HOST, &HeaderValue::from_static("duck.com")))); + /// assert!(pairs.contains(&(&header::SET_COOKIE, &HeaderValue::from_static("one=1")))); + /// assert!(pairs.contains(&(&header::SET_COOKIE, &HeaderValue::from_static("two=2")))); + /// ``` pub fn iter(&self) -> Iter<'_> { Iter::new(self.inner.iter()) } - /// An iterator visiting all keys. + /// An iterator over all contained header names. /// - /// The iteration order is arbitrary, but consistent across platforms for - /// the same crate version. Each key will be yielded only once even if it - /// has multiple associated values. + /// Each name will only be yielded once even if it has multiple associated values. The iteration + /// order should be considered arbitrary. + /// + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); + /// + /// let mut iter = map.keys(); + /// assert!(iter.next().is_none()); + /// + /// map.append(header::HOST, HeaderValue::from_static("duck.com")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); + /// + /// let keys = map.keys().cloned().collect::>(); + /// assert_eq!(keys.len(), 2); + /// assert!(keys.contains(&header::HOST)); + /// assert!(keys.contains(&header::SET_COOKIE)); + /// ``` pub fn keys(&self) -> Keys<'_> { Keys(self.inner.keys()) } - /// Inserts a key-value pair into the map. + /// Clears the map, returning all name-value sets as an iterator. /// - /// If the map did not previously have this key present, then `None` is - /// returned. + /// Header names will only be yielded for the first value in each set. All items that are + /// yielded without a name and after an item with a name are associated with that same name. + /// The first item will always contain a name. /// - /// If the map did have this key present, the new value is associated with - /// the key and all previous values are removed. **Note** that only a single - /// one of the previous values is returned. If there are multiple values - /// that have been previously associated with the key, then the first one is - /// returned. See `insert_mult` on `OccupiedEntry` for an API that returns - /// all values. + /// Keeps the allocated memory for reuse. + /// # Examples + /// ``` + /// # use actix_http::http::{header, HeaderMap, HeaderValue}; + /// let mut map = HeaderMap::new(); /// - /// The key is not updated, though; this matters for types that can be `==` - /// without being identical. - pub fn insert(&mut self, key: HeaderName, val: HeaderValue) { - let _ = self.inner.insert(key, Value::One(val)); - } - - /// Inserts a key-value pair into the map. + /// let mut iter = map.drain(); + /// assert!(iter.next().is_none()); + /// drop(iter); /// - /// If the map did not previously have this key present, then `false` is - /// returned. + /// map.append(header::SET_COOKIE, HeaderValue::from_static("one=1")); + /// map.append(header::SET_COOKIE, HeaderValue::from_static("two=2")); /// - /// If the map did have this key present, the new value is pushed to the end - /// of the list of values currently associated with the key. The key is not - /// updated, though; this matters for types that can be `==` without being - /// identical. - pub fn append(&mut self, key: HeaderName, value: HeaderValue) { - match self.inner.entry(key) { - Entry::Occupied(mut entry) => entry.get_mut().append(value), - Entry::Vacant(entry) => { - entry.insert(Value::One(value)); - } - } - } - - /// Removes all headers for a particular header name from the map. - pub fn remove(&mut self, key: N) { - match key.as_name() { - Either::Left(name) => { - let _ = self.inner.remove(name); - } - Either::Right(s) => { - if let Ok(name) = HeaderName::try_from(s) { - let _ = self.inner.remove(&name); - } - } - } + /// let mut iter = map.drain(); + /// assert_eq!(iter.next().unwrap(), (Some(header::SET_COOKIE), HeaderValue::from_static("one=1"))); + /// assert_eq!(iter.next().unwrap(), (None, HeaderValue::from_static("two=2"))); + /// drop(iter); + /// + /// assert!(map.is_empty()); + /// ``` + pub fn drain(&mut self) -> Drain<'_> { + Drain::new(self.inner.drain()) } } -#[doc(hidden)] -pub trait AsName { - fn as_name(&self) -> Either<&HeaderName, &str>; -} - -impl AsName for HeaderName { - fn as_name(&self) -> Either<&HeaderName, &str> { - Either::Left(self) - } -} - -impl<'a> AsName for &'a HeaderName { - fn as_name(&self) -> Either<&HeaderName, &str> { - Either::Left(self) - } -} - -impl<'a> AsName for &'a str { - fn as_name(&self) -> Either<&HeaderName, &str> { - Either::Right(self) - } -} - -impl AsName for String { - fn as_name(&self) -> Either<&HeaderName, &str> { - Either::Right(self.as_str()) - } -} - -impl<'a> AsName for &'a String { - fn as_name(&self) -> Either<&HeaderName, &str> { - Either::Right(self.as_str()) - } -} - -/// Iterator for all values in a `HeaderMap` with the same name. -pub struct GetAll<'a> { - idx: usize, - item: Option<&'a Value>, -} - -impl<'a> Iterator for GetAll<'a> { - type Item = &'a HeaderValue; +/// Note that this implementation will clone a [HeaderName] for each value. +impl IntoIterator for HeaderMap { + type Item = (HeaderName, HeaderValue); + type IntoIter = IntoIter; #[inline] - fn next(&mut self) -> Option<&'a HeaderValue> { - if let Some(ref val) = self.item { - match val { - Value::One(ref val) => { - self.item.take(); - Some(val) - } - Value::Multi(ref vec) => { - if self.idx < vec.len() { - let item = Some(&vec[self.idx]); - self.idx += 1; - item - } else { - self.item.take(); - None - } - } - } - } else { - None - } - } -} - -pub struct Keys<'a>(hash_map::Keys<'a, HeaderName, Value>); - -impl<'a> Iterator for Keys<'a> { - type Item = &'a HeaderName; - - #[inline] - fn next(&mut self) -> Option<&'a HeaderName> { - self.0.next() + fn into_iter(self) -> Self::IntoIter { + IntoIter::new(self.inner.into_iter()) } } @@ -334,23 +565,116 @@ impl<'a> IntoIterator for &'a HeaderMap { type Item = (&'a HeaderName, &'a HeaderValue); type IntoIter = Iter<'a>; + #[inline] fn into_iter(self) -> Self::IntoIter { - self.iter() + Iter::new(self.inner.iter()) } } -pub struct Iter<'a> { +/// Iterator for all values with the same header name. +/// +/// See [`HeaderMap::get_all`]. +#[derive(Debug)] +pub struct GetAll<'a> { idx: usize, - current: Option<(&'a HeaderName, &'a SmallVec<[HeaderValue; 4]>)>, - iter: hash_map::Iter<'a, HeaderName, Value>, + value: Option<&'a Value>, +} + +impl<'a> GetAll<'a> { + fn new(value: Option<&'a Value>) -> Self { + Self { idx: 0, value } + } +} + +impl<'a> Iterator for GetAll<'a> { + type Item = &'a HeaderValue; + + fn next(&mut self) -> Option { + let val = self.value?; + + match val.get(self.idx) { + Some(val) => { + self.idx += 1; + Some(val) + } + None => { + // current index is none; remove value to fast-path future next calls + self.value = None; + None + } + } + } + + fn size_hint(&self) -> (usize, Option) { + match self.value { + Some(val) => (val.len(), Some(val.len())), + None => (0, Some(0)), + } + } +} + +/// Iterator for owned [`HeaderValue`]s with the same associated [`HeaderName`] returned from methods +/// on [`HeaderMap`] that remove or replace items. +#[derive(Debug)] +pub struct Removed { + inner: Option>, +} + +impl<'a> Removed { + fn new(value: Option) -> Self { + let inner = value.map(|value| value.inner.into_iter()); + Self { inner } + } +} + +impl Iterator for Removed { + type Item = HeaderValue; + + #[inline] + fn next(&mut self) -> Option { + self.inner.as_mut()?.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + match self.inner { + Some(ref iter) => iter.size_hint(), + None => (0, None), + } + } +} + +/// Iterator over all [`HeaderName`]s in the map. +#[derive(Debug)] +pub struct Keys<'a>(hash_map::Keys<'a, HeaderName, Value>); + +impl<'a> Iterator for Keys<'a> { + type Item = &'a HeaderName; + + #[inline] + fn next(&mut self) -> Option { + self.0.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } +} + +#[derive(Debug)] +pub struct Iter<'a> { + inner: hash_map::Iter<'a, HeaderName, Value>, + multi_inner: Option<(&'a HeaderName, &'a SmallVec<[HeaderValue; 4]>)>, + multi_idx: usize, } impl<'a> Iter<'a> { fn new(iter: hash_map::Iter<'a, HeaderName, Value>) -> Self { Self { - iter, - idx: 0, - current: None, + inner: iter, + multi_idx: 0, + multi_inner: None, } } } @@ -358,28 +682,272 @@ impl<'a> Iter<'a> { impl<'a> Iterator for Iter<'a> { type Item = (&'a HeaderName, &'a HeaderValue); - #[inline] - fn next(&mut self) -> Option<(&'a HeaderName, &'a HeaderValue)> { - if let Some(ref mut item) = self.current { - if self.idx < item.1.len() { - let item = (item.0, &item.1[self.idx]); - self.idx += 1; - return Some(item); - } else { - self.idx = 0; - self.current.take(); - } - } - if let Some(item) = self.iter.next() { - match item.1 { - Value::One(ref value) => Some((item.0, value)), - Value::Multi(ref vec) => { - self.current = Some((item.0, vec)); - self.next() + fn next(&mut self) -> Option { + // handle in-progress multi value lists first + if let Some((ref name, ref mut vals)) = self.multi_inner { + match vals.get(self.multi_idx) { + Some(val) => { + self.multi_idx += 1; + return Some((name, val)); + } + None => { + // no more items in value list; reset state + self.multi_idx = 0; + self.multi_inner = None; } } - } else { - None + } + + let (name, value) = self.inner.next()?; + + // set up new inner iter and recurse into it + self.multi_inner = Some((name, &value.inner)); + self.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + // take inner lower bound + // make no attempt at an upper bound + (self.inner.size_hint().0, None) + } +} + +/// Iterator over drained name-value pairs. +/// +/// Iterator items are `(Option, HeaderValue)` to avoid cloning. +#[derive(Debug)] +pub struct Drain<'a> { + inner: hash_map::Drain<'a, HeaderName, Value>, + multi_inner: Option<(Option, SmallVec<[HeaderValue; 4]>)>, + multi_idx: usize, +} + +impl<'a> Drain<'a> { + fn new(iter: hash_map::Drain<'a, HeaderName, Value>) -> Self { + Self { + inner: iter, + multi_inner: None, + multi_idx: 0, } } } + +impl<'a> Iterator for Drain<'a> { + type Item = (Option, HeaderValue); + + fn next(&mut self) -> Option { + // handle in-progress multi value iterators first + if let Some((ref mut name, ref mut vals)) = self.multi_inner { + if !vals.is_empty() { + // OPTIMIZE: array removals + return Some((name.take(), vals.remove(0))); + } else { + // no more items in value iterator; reset state + self.multi_inner = None; + self.multi_idx = 0; + } + } + + let (name, value) = self.inner.next()?; + + // set up new inner iter and recurse into it + self.multi_inner = Some((Some(name), value.inner)); + self.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + // take inner lower bound + // make no attempt at an upper bound + (self.inner.size_hint().0, None) + } +} + +/// Iterator over owned name-value pairs. +/// +/// Implementation necessarily clones header names for each value. +#[derive(Debug)] +pub struct IntoIter { + inner: hash_map::IntoIter, + multi_inner: Option<(HeaderName, smallvec::IntoIter<[HeaderValue; 4]>)>, +} + +impl IntoIter { + fn new(inner: hash_map::IntoIter) -> Self { + Self { + inner, + multi_inner: None, + } + } +} + +impl Iterator for IntoIter { + type Item = (HeaderName, HeaderValue); + + fn next(&mut self) -> Option { + // handle in-progress multi value iterators first + if let Some((ref name, ref mut vals)) = self.multi_inner { + match vals.next() { + Some(val) => { + return Some((name.clone(), val)); + } + None => { + // no more items in value iterator; reset state + self.multi_inner = None; + } + } + } + + let (name, value) = self.inner.next()?; + + // set up new inner iter and recurse into it + self.multi_inner = Some((name, value.inner.into_iter())); + self.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + // take inner lower bound + // make no attempt at an upper bound + (self.inner.size_hint().0, None) + } +} + +#[cfg(test)] +mod tests { + use http::header; + + use super::*; + + #[test] + fn create() { + let map = HeaderMap::new(); + assert_eq!(map.len(), 0); + assert_eq!(map.capacity(), 0); + + let map = HeaderMap::with_capacity(16); + assert_eq!(map.len(), 0); + assert!(map.capacity() >= 16); + } + + #[test] + fn insert() { + let mut map = HeaderMap::new(); + + map.insert(header::LOCATION, HeaderValue::from_static("/test")); + assert_eq!(map.len(), 1); + } + + #[test] + fn contains() { + let mut map = HeaderMap::new(); + assert!(!map.contains_key(header::LOCATION)); + + map.insert(header::LOCATION, HeaderValue::from_static("/test")); + assert!(map.contains_key(header::LOCATION)); + assert!(map.contains_key("Location")); + assert!(map.contains_key("Location".to_owned())); + assert!(map.contains_key("location")); + } + + #[test] + fn entries_iter() { + let mut map = HeaderMap::new(); + + map.append(header::HOST, HeaderValue::from_static("duck.com")); + map.append(header::COOKIE, HeaderValue::from_static("one=1")); + map.append(header::COOKIE, HeaderValue::from_static("two=2")); + + let mut iter = map.iter(); + assert!(iter.next().is_some()); + assert!(iter.next().is_some()); + assert!(iter.next().is_some()); + assert!(iter.next().is_none()); + + let pairs = map.iter().collect::>(); + assert!(pairs.contains(&(&header::HOST, &HeaderValue::from_static("duck.com")))); + assert!(pairs.contains(&(&header::COOKIE, &HeaderValue::from_static("one=1")))); + assert!(pairs.contains(&(&header::COOKIE, &HeaderValue::from_static("two=2")))); + } + + #[test] + fn drain_iter() { + let mut map = HeaderMap::new(); + + map.append(header::COOKIE, HeaderValue::from_static("one=1")); + map.append(header::COOKIE, HeaderValue::from_static("two=2")); + + let mut vals = vec![]; + let mut iter = map.drain(); + + let (name, val) = iter.next().unwrap(); + assert_eq!(name, Some(header::COOKIE)); + vals.push(val); + + let (name, val) = iter.next().unwrap(); + assert!(name.is_none()); + vals.push(val); + + assert!(vals.contains(&HeaderValue::from_static("one=1"))); + assert!(vals.contains(&HeaderValue::from_static("two=2"))); + + assert!(iter.next().is_none()); + drop(iter); + + assert!(map.is_empty()); + } + + #[test] + fn entries_into_iter() { + let mut map = HeaderMap::new(); + + map.append(header::HOST, HeaderValue::from_static("duck.com")); + map.append(header::COOKIE, HeaderValue::from_static("one=1")); + map.append(header::COOKIE, HeaderValue::from_static("two=2")); + + let mut iter = map.into_iter(); + assert!(iter.next().is_some()); + assert!(iter.next().is_some()); + assert!(iter.next().is_some()); + assert!(iter.next().is_none()); + } + + #[test] + fn iter_and_into_iter_same_order() { + let mut map = HeaderMap::new(); + + map.append(header::HOST, HeaderValue::from_static("duck.com")); + map.append(header::COOKIE, HeaderValue::from_static("one=1")); + map.append(header::COOKIE, HeaderValue::from_static("two=2")); + + let mut iter = map.iter(); + let mut into_iter = map.clone().into_iter(); + + assert_eq!(iter.next().map(owned_pair), into_iter.next()); + assert_eq!(iter.next().map(owned_pair), into_iter.next()); + assert_eq!(iter.next().map(owned_pair), into_iter.next()); + assert_eq!(iter.next().map(owned_pair), into_iter.next()); + } + + #[test] + fn get_all_and_remove_same_order() { + let mut map = HeaderMap::new(); + + map.append(header::COOKIE, HeaderValue::from_static("one=1")); + map.append(header::COOKIE, HeaderValue::from_static("two=2")); + + let mut vals = map.get_all(header::COOKIE); + let mut removed = map.clone().remove(header::COOKIE); + + assert_eq!(vals.next(), removed.next().as_ref()); + assert_eq!(vals.next(), removed.next().as_ref()); + assert_eq!(vals.next(), removed.next().as_ref()); + } + + fn owned_pair<'a>( + (name, val): (&'a HeaderName, &'a HeaderValue), + ) -> (HeaderName, HeaderValue) { + (name.clone(), val.clone()) + } +} diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index dc97bf5ff..9543d43b6 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -1,4 +1,4 @@ -//! Typed HTTP headers, pre-defined `HeaderName`s, traits for parsing/conversion and other +//! Typed HTTP headers, pre-defined `HeaderName`s, traits for parsing and conversion, and other //! header utility methods. use std::fmt; @@ -11,6 +11,7 @@ pub use http::header::*; use crate::error::ParseError; use crate::httpmessage::HttpMessage; +mod as_name; mod into_pair; mod into_value; mod utils; @@ -23,6 +24,7 @@ pub use self::common::*; #[doc(hidden)] pub use self::shared::*; +pub use self::as_name::AsHeaderName; pub use self::into_pair::IntoHeaderPair; pub use self::into_value::IntoHeaderValue; #[doc(hidden)] @@ -39,16 +41,14 @@ pub trait Header: IntoHeaderValue { fn parse(msg: &T) -> Result; } -#[doc(hidden)] +#[derive(Debug, Default)] pub(crate) struct Writer { buf: BytesMut, } impl Writer { fn new() -> Writer { - Writer { - buf: BytesMut::new(), - } + Writer::default() } fn take(&mut self) -> Bytes { @@ -71,12 +71,8 @@ impl fmt::Write for Writer { /// Convert `http::HeaderMap` to our `HeaderMap`. impl From for HeaderMap { - fn from(map: http::HeaderMap) -> HeaderMap { - let mut new_map = HeaderMap::with_capacity(map.capacity()); - for (h, v) in map.iter() { - new_map.append(h.clone(), v.clone()); - } - new_map + fn from(mut map: http::HeaderMap) -> HeaderMap { + HeaderMap::from_drain(map.drain()) } } diff --git a/actix-http/src/request.rs b/actix-http/src/request.rs index 1031d7dce..fbf4d3c23 100644 --- a/actix-http/src/request.rs +++ b/actix-http/src/request.rs @@ -177,13 +177,17 @@ impl

fmt::Debug for Request

{ self.method(), self.path() )?; + if let Some(q) = self.uri().query().as_ref() { writeln!(f, " query: ?{:?}", q)?; } + writeln!(f, " headers:")?; - for (key, val) in self.headers() { + + for (key, val) in self.headers().iter() { writeln!(f, " {:?}: {:?}", key, val)?; } + Ok(()) } } diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index 110514e05..113ace221 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -363,7 +363,9 @@ impl ResponseBuilder { { if let Some(parts) = parts(&mut self.head, &self.err) { match header.try_into_header_pair() { - Ok((key, value)) => parts.headers.insert(key, value), + Ok((key, value)) => { + parts.headers.insert(key, value); + } Err(e) => self.err = Some(e.into()), }; } @@ -752,9 +754,11 @@ impl<'a> From<&'a ResponseHead> for ResponseBuilder { let mut msg = BoxedResponseHead::new(head.status); msg.version = head.version; msg.reason = head.reason; - for (k, v) in &head.headers { + + for (k, v) in head.headers.iter() { msg.headers.append(k.clone(), v.clone()); } + msg.no_chunking(!head.chunked()); ResponseBuilder { @@ -893,16 +897,20 @@ mod tests { .max_age(time::Duration::days(1)) .finish(), ) - .del_cookie(&cookies[1]) + .del_cookie(&cookies[0]) .finish(); - let mut val: Vec<_> = resp + let mut val = resp .headers() .get_all(SET_COOKIE) .map(|v| v.to_str().unwrap().to_owned()) - .collect(); + .collect::>(); val.sort(); + + // the .del_cookie call assert!(val[0].starts_with("cookie1=; Max-Age=0;")); + + // the .cookie call assert_eq!( val[1], "name=value; HttpOnly; Path=/test; Domain=www.rust-lang.org; Max-Age=86400" @@ -927,9 +935,9 @@ mod tests { let mut iter = r.cookies(); let v = iter.next().unwrap(); - assert_eq!((v.name(), v.value()), ("cookie3", "val300")); - let v = iter.next().unwrap(); assert_eq!((v.name(), v.value()), ("original", "val100")); + let v = iter.next().unwrap(); + assert_eq!((v.name(), v.value()), ("cookie3", "val300")); } #[test] diff --git a/awc/src/frozen.rs b/awc/src/frozen.rs index 878f404c6..46b4063a0 100644 --- a/awc/src/frozen.rs +++ b/awc/src/frozen.rs @@ -145,7 +145,9 @@ impl FrozenSendBuilder { { match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { - Ok(value) => self.extra_headers.insert(key, value), + Ok(value) => { + self.extra_headers.insert(key, value); + } Err(e) => self.err = Some(e.into()), }, Err(e) => self.err = Some(e.into()), diff --git a/awc/src/request.rs b/awc/src/request.rs index b9a333b18..c87df9b3b 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -159,7 +159,9 @@ impl ClientRequest { H: IntoHeaderPair, { match header.try_into_header_pair() { - Ok((key, value)) => self.head.headers.insert(key, value), + Ok((key, value)) => { + self.head.headers.insert(key, value); + } Err(e) => self.err = Some(e.into()), }; @@ -232,7 +234,9 @@ impl ClientRequest { >::Error: Into, { match HeaderValue::try_from(value) { - Ok(value) => self.head.headers.insert(header::CONTENT_TYPE, value), + Ok(value) => { + self.head.headers.insert(header::CONTENT_TYPE, value); + } Err(e) => self.err = Some(e.into()), } self diff --git a/awc/src/sender.rs b/awc/src/sender.rs index 1cf863d96..ca220be71 100644 --- a/awc/src/sender.rs +++ b/awc/src/sender.rs @@ -304,7 +304,9 @@ impl RequestSender { RequestSender::Owned(head) => { if !head.headers.contains_key(&key) { match value.try_into_value() { - Ok(value) => head.headers.insert(key, value), + Ok(value) => { + head.headers.insert(key, value); + } Err(e) => return Err(e.into()), } } diff --git a/src/request.rs b/src/request.rs index a563518e0..9a5e43185 100644 --- a/src/request.rs +++ b/src/request.rs @@ -445,10 +445,10 @@ mod tests { { let cookies = req.cookies().unwrap(); assert_eq!(cookies.len(), 2); - assert_eq!(cookies[0].name(), "cookie2"); - assert_eq!(cookies[0].value(), "value2"); - assert_eq!(cookies[1].name(), "cookie1"); - assert_eq!(cookies[1].value(), "value1"); + assert_eq!(cookies[0].name(), "cookie1"); + assert_eq!(cookies[0].value(), "value1"); + assert_eq!(cookies[1].name(), "cookie2"); + assert_eq!(cookies[1].value(), "value2"); } let cookie = req.cookie("cookie1");