Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement HttpTryFrom<HashMap<String,String>> for HeaderMap #326

Merged
merged 8 commits into from Jul 25, 2019
51 changes: 47 additions & 4 deletions src/header/map.rs
@@ -1,5 +1,8 @@
use super::HeaderValue;
use super::name::{HeaderName, HdrName, InvalidHeaderName};
use convert::{HttpTryFrom, HttpTryInto};
use Error;
use sealed::Sealed;

use std::{fmt, mem, ops, ptr, vec};
use std::collections::hash_map::RandomState;
Expand All @@ -23,10 +26,11 @@ pub use self::into_header_name::IntoHeaderName;
/// ```
/// # use http::HeaderMap;
/// # use http::header::{CONTENT_LENGTH, HOST, LOCATION};
/// let mut headers = HeaderMap::new();
///
/// headers.insert(HOST, "example.com".parse().unwrap());
/// headers.insert(CONTENT_LENGTH, "123".parse().unwrap());
/// # use http::HttpTryFrom;
seanmonstar marked this conversation as resolved.
Show resolved Hide resolved
/// let mut headers = HeaderMap::try_from(vec![
/// (HOST, "example.com"),
/// (CONTENT_LENGTH, "123")
/// ]).unwrap();
///
/// assert!(headers.contains_key(HOST));
/// assert!(!headers.contains_key(LOCATION));
Expand Down Expand Up @@ -1720,6 +1724,45 @@ impl<T> FromIterator<(HeaderName, T)> for HeaderMap<T>
}
}

impl<T> Sealed for HeaderMap<T> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be an identity impl, impl<T> HttpTryFrom<HeaderMap<T>> for HeaderMap<T>. Would that conflict with the generic one below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, HeaderMap<T> is itself IntoIterator<Item=(HttpTryFrom<HeaderName>, HttpTryFrom<HeaderValue>)>, so HeaderMap::try_from(header_map) works as expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually one would expect TryFrom<Me> for Me to be basically free, but because that conflicts, it would be surprising that it essentially clones a new HeaderMap....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed sub-optimal, and can fixed later when rust adds support for specialization.
But until then, I think the usability benefits far outweigh this downside. The standard library itself has the same issue in several places, where an implementation is slower than it could be in order to be more generic.


/// Convert a collection of tuples into a HeaderMap
///
/// # Examples
///
/// ```
/// # use http::{HttpTryFrom, Result, header::HeaderMap};
/// # use std::collections::HashMap;
/// let mut headers_hashmap: HashMap<String, String> = vec![
/// ("X-Custom-Header".to_string(), "my value".to_string()),
/// ].iter().cloned().collect();
///
/// let good_headers: Result<HeaderMap> = HeaderMap::try_from(&headers_hashmap);
/// assert!(good_headers.is_ok());
///
/// headers_hashmap.insert("\r".into(), "\0".into());
/// let bad_headers: Result<HeaderMap> = HeaderMap::try_from(&headers_hashmap);
/// assert!(bad_headers.is_err());
/// ```
impl<COLLECTION, K, V> HttpTryFrom<COLLECTION> for HeaderMap<HeaderValue>
seanmonstar marked this conversation as resolved.
Show resolved Hide resolved
where
COLLECTION: IntoIterator<Item=(K, V)>,
HeaderName: HttpTryFrom<K>,
HeaderValue: HttpTryFrom<V>
{
type Error = Error;

fn try_from(c: COLLECTION) -> Result<Self, Self::Error> {
c.into_iter()
.map(|(k, v)| -> ::Result<(HeaderName, HeaderValue)> {
let name : HeaderName = k.http_try_into()?;
let value : HeaderValue = v.http_try_into()?;
Ok((name, value))
})
.collect()
}
}

impl<T> Extend<(Option<HeaderName>, T)> for HeaderMap<T> {
/// Extend a `HeaderMap` with the contents of another `HeaderMap`.
///
Expand Down
8 changes: 8 additions & 0 deletions src/header/name.rs
Expand Up @@ -1772,6 +1772,14 @@ impl<'a> HttpTryFrom<&'a str> for HeaderName {
}
}

impl<'a> HttpTryFrom<&'a String> for HeaderName {
type Error = InvalidHeaderName;
#[inline]
fn try_from(s: &'a String) -> Result<Self, Self::Error> {
Self::from_bytes(s.as_bytes())
}
}

impl<'a> HttpTryFrom<&'a [u8]> for HeaderName {
type Error = InvalidHeaderName;
#[inline]
Expand Down
8 changes: 8 additions & 0 deletions src/header/value.rs
Expand Up @@ -521,6 +521,14 @@ impl<'a> HttpTryFrom<&'a str> for HeaderValue {
}
}

impl<'a> HttpTryFrom<&'a String> for HeaderValue {
type Error = InvalidHeaderValue;
#[inline]
fn try_from(s: &'a String) -> Result<Self, Self::Error> {
Self::from_bytes(s.as_bytes())
}
}

impl<'a> HttpTryFrom<&'a [u8]> for HeaderValue {
type Error = InvalidHeaderValue;

Expand Down