Change signature of insertion/appending of headers

[halvko]

Currently the insert and append functions on Headers panics on illegal header value encoding. This commit exposes that failure mode to the library users by wrapping the return in a result instead.

This change exposes a possible optimization in places where insert/append is called with values known to be safe by creating unsafe insertion/appending functions. This commit does not implement any such extensions.

Fixes #383

[halvko]

@yoshuawuyts Do you have any thoughts about this? I can see you are assigned on the original issue.

[halvko]

I'm not sure what to do about hyperium_http::hyperium_headers_to_headers, if this should return a result, if we should panic! on errors, or if this (as the rest of the function) should rely on unsafe.

Panicing was the previous behaviour, it just wasn't explicit, but currently I've added a todo!, hoping someone more knowledgable about hyperium(?) has a more well-formed opinion about what the correct behavior should be.

[yoshuawuyts]

@halvko this is probably a long time coming, but we should enable the conversion between hyperium/http and http-types to be done using TryFrom. Failure to convert between the two should surface as errors, and we shouldn't panic at any point.

[halvko]

@yoshuawuyts my local clippy doesn't output the warnings I see in CI, do you know what I have to do to reproduce locally?

[yoshuawuyts]

@halvko I forget whether we're running lints on nightly; but otherwise check that you've got a recent version of Rust and Clippy installed?

[halvko]

Seems to warn correctly when compiling with the nightly compiler. A lot of the warnings are also present on upstream, I'm not sure if I should start by making a PR fixing those?

[halvko]

By running a diff on clippys output, I saw that this pull request doesn't introduce any new warnings:

1c1
<    Compiling http-types v3.0.0 (/home/erikfc/code/rust/open-source/http-types)
---
>     Checking http-types v3.0.0 (/home/erikfc/code/rust/open-source/http-types)
120,136d119
< warning: redundant closure
<   --> src/hyperium_http.rs:71:60
<    |
< 71 |         http::header::HeaderName::from_bytes(name).map_err(|e| Error::new_adhoc(e))
<    |                                                            ^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `Error::new_adhoc`
<    |
<    = note: `#[warn(clippy::redundant_closure)]` on by default
<    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
< 
< warning: redundant closure
<   --> src/hyperium_http.rs:89:62
<    |
< 89 |         http::header::HeaderValue::from_bytes(value).map_err(|e| Error::new_adhoc(e))
<    |                                                              ^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `Error::new_adhoc`
<    |
<    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
< 
146,147c129,130
< warning: `http-types` (lib) generated 12 warnings
< error: could not compile `http-types` due to 2 previous errors; 12 warnings emitted
---
> warning: `http-types` (lib) generated 10 warnings
> error: could not compile `http-types` due to 2 previous errors; 10 warnings emitted
149c132
< warning: `http-types` (lib test) generated 14 warnings (12 duplicates)
---
> warning: `http-types` (lib test) generated 12 warnings (10 duplicates)

[halvko]

Now exactly the same warnings is generated by upstream and this PR (except for 2 line numbers which have changed:

1c1
<    Compiling http-types v3.0.0 (/home/erikfc/code/rust/open-source/http-types)
---
>     Checking http-types v3.0.0 (/home/erikfc/code/rust/open-source/http-types)
76,89d75
< warning: field is never read: `inner`
<    --> src/body.rs:635:13
<     |
< 635 |             inner: String,
<     |             ^^^^^^^^^^^^^
<     |
<     = note: `#[warn(dead_code)]` on by default
< 
< warning: field is never read: `inner`
<    --> src/body.rs:647:13
<     |
< 647 |             inner: String,
<     |             ^^^^^^^^^^^^^
< 
118a105,118
> 
> warning: field is never read: `inner`
>    --> src/body.rs:635:13
>     |
> 635 |             inner: String,
>     |             ^^^^^^^^^^^^^
>     |
>     = note: `#[warn(dead_code)]` on by default
> 
> warning: field is never read: `inner`
>    --> src/body.rs:647:13
>     |
> 647 |             inner: String,
>     |             ^^^^^^^^^^^^^