hyperium / http

Rust HTTP types
Apache License 2.0
1.16k stars 289 forks source link

Non obvious upgrade path with the new by-value request builder #543

Closed sdeleuze closed 2 years ago

sdeleuze commented 2 years ago

Hi,

Related to #191, I try to upgrade my mini-http fork to http crate version 0.2 (see related commit) but the new by-value request builder combined to the fact that I need to iterate to create various headers makes the migration non trivial, at least for a Rust newcomer like me.

When I update:

for header in req.headers {
    request.header(header.name, header.value);
}

to

let headers = request.headers_mut().unwrap();
for header in req.headers {
    headers.insert(header.name, HeaderValue::from_bytes(header.value).unwrap());
}

I get the following error and I am not sure how to solve it:

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
   --> src/http_stream.rs:122:33
    |
58  |     pub fn try_build_request(&mut self) -> Result<Option<RequestHead>> {
    |                              --------- this data with an anonymous lifetime `'_`...
...
122 |             let header_bytes = &self.read_buf[..self.headers_length];
    |                                 -------------^^^^^^^^^^^^^^^^^^^^^^^
    |                                 |
    |                                 ...is used here...
...
150 |                 headers.insert(header.name, HeaderValue::from_bytes(header.value).unwrap());
    |                         ------ ...and is required to live as long as `'static` here

I am still growing my Rust skills so I may have missed something obvious, but I have the feeling that request.headers_mut() is considered as less safe than request.header() by the Rust compiler but the by-value request builder does not allow me to use request.header() in a for loop. I have already tried to clone the data inserted the the headers map, still the same issue, so I am kind of stuck.

Could you please share some guidance on how to upgrade for this use case and use that opportunity that the new API is designed for this kind of use case?

To reproduce the error:

git clone https://github.com/sdeleuze/mini_http.git
cd mini_http
git checkout http-0.2
cargo build
seanmonstar commented 2 years ago

This might be the easiest way for you:

for header in req.headers {
    request = request.header(header.name, header.value);
}

I think the error you're seeing is nothing to do with headers_mut being less safe (it's all safe), but rather that HeaderMap::insert requires the name to be either a HeaderName or a &'static str, and I'm guessing header.name is neither, but a &str.

sdeleuze commented 2 years ago

I see, thanks a lot for your feedback, much appreciated.