Content-type doesn't get correctly copied over when converting from `http_types::Request`

[pwoolcoc]

trafficstars

If you convert from http_types::Request to surf::Request using surf::Request::try_from(http_types::Request), the content-type doesn't get coped over correctly. Below is my Cargo.toml and src/lib.rs, with a test that duplicates the error.

# Cargo.toml
[package]
...

[dependencies]
surf = "2.0.0-alpha.2"
http-types = "1.2.0"
url = "2.1.1"

# src/lib.rs
use http_types::headers::{HeaderName, HeaderValue};
use url::Url;
use std::convert::TryFrom;

#[test]
fn content_type_change() {
    let mut http_types_request = http_types::Request::new(http_types::Method::Get, Url::parse("https://example.com/").unwrap());
    http_types_request.insert_header("Content-Type", http_types::mime::JSON);

    let surf_request = surf::Request::try_from(http_types_request).unwrap();
    let content_type = HeaderName::try_from("Content-Type").unwrap();

    let expected = HeaderValue::try_from("application/json").unwrap();
    assert_eq!(
        &expected,
        surf_request.header(&content_type).unwrap().get(0).unwrap(),
    );
}

// running 1 test
// test content_type_change ... FAILED
// 
// failures:
// 
// ---- content_type_change stdout ----
// thread 'content_type_change' panicked at 'assertion failed: `(left == right)`
//   left: `HeaderValue { inner: "application/json" }`,
//  right: `HeaderValue { inner: "application/octet-stream" }`', src/lib.rs:14:5
// note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
// 
// 
// failures:
//     content_type_change
// 
// test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
// 
// error: test failed, to rerun pass '--lib'

[peterhuene]

I just ran into this issue as well.

In fact, none of the headers are copied from the http_types::Request. I'm also not seeing a body from a Vec<u8> being copied either.

Content-Type is only getting set as a result of calling surf::Request::body in the TryFrom implementation, which is defaulting the Content-Type header to application/octet-stream.

The question is why the TryFrom implementation for surf::Request doesn't just construct using the passed in request, for example:

fn try_from(http_request: http_types::Request) -> io::Result<Self> {
    let url = http_request.url().clone();
    let req = Self {
        fut: None,
        client: Some(Client::new()),
        req: Some(http_request),
        url,
        middleware: Some(vec![]),
    };

    #[cfg(feature = "middleware-logger")]
    let req = req.middleware(crate::middleware::logger::new());

    Ok(req)
}

[peterhuene]

Also, given that currently the TryFrom implementation can't fail, perhaps we could turn this into From for ergonomics?

[yoshuawuyts]

perhaps we could turn this into From for ergonomics?

Yes, absolutely!