actix-web icon indicating copy to clipboard operation
actix-web copied to clipboard

Remove Option from RequestHeadType::Rc

Open asonix opened this issue 3 years ago • 2 comments

When working with middlewares, I needed to destructure into the ConnectRequest and RequestHeadType to add new headers into the request. If RequestHeadType::Rc defaulted to having an empty headermap instead of an Option<HeaderMap> it would be possible to implement a headers_mut() method on ConnectRequest for easy header additions.

I realize that providing a headers_mut() for the RequestHeadType::Rc case might be misleading, since it won't be possible to remove existing headers, but maybe with a bit of documentation that could be fine

asonix avatar Dec 01 '21 01:12 asonix

I marked this issue wrong, the type lives in actix-http which is already stable. So...

Proposal is to just add an append_header method to RequestHeadType similar to other builders:

impl RequestHeadType {
    /// Append a header, keeping any that were set with an equivalent field name.
    ///
    /// Invalid header pairs are ignored but log a warning.
    pub fn append_header(&mut self, header: impl TryIntoHeaderPair) {
        let (name, value) = match header.try_into_pair() {
            Ok(pair) => pair,
            Err(_err) => {
                log::error!("invalid header could not be appended to client request");
                return;
            }
        };

        let headers = match self {
            RequestHeadType::Owned(head) => head.headers_mut(),
            RequestHeadType::Rc(_, Some(headers)) => headers,
            RequestHeadType::Rc(_, headers) => {
                *headers = Some(HeaderMap::new());
                headers.as_mut().unwrap()
            }
        };

        headers.append(name, value);
    }
}

robjtede avatar Mar 08 '22 00:03 robjtede

fwiw the

*thing = Some(_);
thing.as_mut().unwrap()

pattern is exactly what https://doc.rust-lang.org/std/option/enum.Option.html#method.replace is for

Otherwise I like the append_header proposal :D

edit: linked wrong method:

  • https://doc.rust-lang.org/std/option/enum.Option.html#method.insert

asonix avatar Mar 08 '22 13:03 asonix