Parsing HTTP headers from &HashMap<String, String> into http::HeaderMap

Question

For context, I'm very new to Rust, and I'm trying to learn the best practices and nuances of it as I go.

I've written this small function that takes a HashMap<String, String> and parses it into a http::HeaderMap.

Any feedback on style, idiomatic inconsistencies and general best practice or code smells is appreciated.

fn build_headers(headers: &HashMap<String, String>) -> HeaderMap {
    let mut header_map = HeaderMap::new();

    for (key, value) in headers {
        let key = match HeaderName::try_from(key) {
            Ok(key) => key,
            Err(_) => {
                eprintln!("Invalid header name: {}", key);
                continue
            }
        };
        let value = match value.parse() {
            Ok(v) => v,
            Err(_) => {
                eprintln!("Invalid header value: {}", value);
                continue
            }
        };
        header_map.insert(key, value);
    }

    header_map
}

Answer 1

temp vars

        let key = match ...
            Ok(key) => key,

Prefer a 2nd line of Ok(k) => k,

The following looks great:

        let value = match ...
            Ok(v) => v,

If you don't rename to k, possibly you'd also like to use value for v.

diagnostics

                eprintln!("Invalid header name: {}", key);

Consider using log::warn() for that. Then we can adjust the logging config to make this code more or less chatty in a testing or a production setting.

We can't see what other things get logged in the course of satisfying a web request. But if I were a maintenance engineer charged with chasing down a bug, I doubt that just knowing key would suffice. Typically we'll want to know details like the client's IP or session identifier, and the full URL being requested. In other words, enough information to use a tool like the Postman app in order to reproduce the error.

Consider altering the Public API you export, if "good diagnostics" is a requirement. Maybe you'd like to also return a set of "bad" headers, which we typically expect to be empty? Then we can let the caller worry about what to do. Notice that the caller hangs onto more context, which can be logged, than the OP code has available to it. Or maybe you'd like to pass in some context, so that this routine can offer better diagnostic messages?

Answer 2

There already is a generic fallible conversion implemented from &HashMap<_, _> to HeaderMap: https://github.com/hyperium/http/blob/b53194720352ef923d5fa662bc52592520e8b3ce/src/header/map.rs#L2068

Hence, your entire function is superfluous:

use http::HeaderMap;
use std::collections::HashMap;

fn main() {
    let mut hash_map = HashMap::new();
    hash_map.insert("Content-Type".to_string(), "application/json".to_string());
    let header_map: HeaderMap = (&hash_map)
        .try_into()
        .expect("Hash map should contain valid headers.");
    dbg!(header_map);
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=63e36c4049adf44ccf1adf6f807bf451