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 TryFrom<HashMap<String, String>> for HeaderMap #555

Closed
lovasoa opened this issue Jul 1, 2019 · 10 comments
Closed

Implement TryFrom<HashMap<String, String>> for HeaderMap #555

lovasoa opened this issue Jul 1, 2019 · 10 comments

Comments

@lovasoa
Copy link

lovasoa commented Jul 1, 2019

Currently, reqwest provides no easy way to use a HashMap of strings as headers, and converting from a HashMap to a HeaderMap is not straightforward.

Converting from a String to a HeaderName can return an InvalidHeaderName, and converting from a string to a HeaderValue can return a different type of error (InvalidHeaderValue). And reqwest::Error does not implement From<_> for these error types, which makes it impossible to use the ? operator.

It would be nice to be able to write a simple

reqwest::Client::builder().default_headers(headers_hashmap.try_into()?).build()?

Instead of having to write a verbose conversion function that handles error type conversions.

The conversion implementation could even be generic, so that it would work with any IntoIterator<(ToString, ToString)>

@lawliet89
Copy link

HeaderMap implements FromIterator<(HeaderName, T)>.

You can just do something like

use std::collections::HashMap;
use std::str::FromStr;

use reqwest::header::{HeaderMap, HeaderName, HeaderValue};

fn headermap_from_hashmap<'a, I, S>(headers: I) -> HeaderMap
where
    I: Iterator<Item = (S, S)> + 'a,
    S: AsRef<str> + 'a,
{
    headers
        .map(|(name, val)| (HeaderName::from_str(name.as_ref()), HeaderValue::from_str(val.as_ref())))
        // We ignore the errors here. If you want to get a list of failed conversions, you can use Iterator::partition 
        // to help you out here
        .filter(|(k, v)| k.is_ok() && v.is_ok())
        .map(|(k, v)| (k.unwrap(), v.unwrap()))
        .collect() 
}

fn main() {
    let hashmap: HashMap<String, String> = [("x-test", "foobar"), ("x-testing", "bar")]
        .iter()
        .map(|(k, v)| (k.to_string(), v.to_string()))
        .collect();

    println!("{:#?}", headermap_from_hashmap(hashmap.iter()));
}

Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=831e89334756b899d2dc61950795789e

@lovasoa
Copy link
Author

lovasoa commented Jul 2, 2019

Yes, this is what I do currently. But this is not very ergonomic. Especially if you want to add error handling. Don't you think headers_hashmap.try_into()? would bring some real value ?

@lawliet89
Copy link

lawliet89 commented Jul 2, 2019

I agree it's rather unergonomic.

Anyway, the various Header types are defined in the http crate. You will also need to implement TryFrom for HeaderName and HeaderValue.

I think this requires a bump of the minimum Rust version to 1.34 and I am not sure what http's policy on this is. Might be better to open a discussion there.

@lovasoa
Copy link
Author

lovasoa commented Jul 2, 2019

You will also need to implement TryFrom for HeaderName and HeaderValue.

No, this is not needed, because they already implement FromStr.
However, the error type is InvalidHeaderName, for which there is no automatic conversion to reqwest::Error, which prevents users from using the ? operator. I think reqwest::Error is defined in reqwest, so we are in the right place to discuss at least this part of the problem.

@lawliet89
Copy link

Sure.

Another complication is that HeaderMap is generic over some T, so it might get hairy supporting it for arbitrary T: FromStr.

@lovasoa
Copy link
Author

lovasoa commented Jul 2, 2019

I also made a pull request to the http crate: hyperium/http#326

@lovasoa
Copy link
Author

lovasoa commented Sep 5, 2019

hyperium/http#326 is now merged

@alexkreidler
Copy link

Any updates on this?

@emesterhazy
Copy link

@alexkreidler

Looks like the work from hyperium/http#326 got removed from the http crate at some point. Looks like you can use TryFrom now.

@lovasoa Does this resolve this issue? Maybe you can close it if so?

@lovasoa
Copy link
Author

lovasoa commented Jul 13, 2021

Yes, feel free to close if this has been solved, this is an old issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants