http-rs / http-client

Types and traits for http clients
Apache License 2.0
114 stars 49 forks source link

h1 client fails to resolve localhost #79

Closed Jonathas-Conceicao closed 3 years ago

Jonathas-Conceicao commented 3 years ago

I've made the following reproducible sample code using mockito (though the problems happens happens with other servers too) that gives me the Connection refused (os error 111) error:

#[async_std::main]
async fn main() {
    use http_client::http_types::*;
    use http_client::HttpClient;

    let client = http_client::h1::H1Client::new();
    let _mock_guard = mockito::mock("GET", "/report")
        .with_status(200)
        .expect_at_least(2)
        .create();
    // Skips the initial "http://127.0.0.1:" to get only the port number
    let mock_port = &mockito::server_url()[17..];

    let url = &format!("http://0.0.0.0:{}/report", mock_port);
    let req = Request::new(Method::Get, Url::parse(url).unwrap());
    dbg!(client.send(req.clone()).await.unwrap());

    let url = &format!("http://localhost:{}/report", mock_port);
    let req = Request::new(Method::Get, Url::parse(url).unwrap());
    dbg!(client.send(req.clone()).await.unwrap());
}
[package]
name = "test-h1"
version = "0.1.0"
authors = ["Jonathas-Conceicao <jonathas.conceicao@ossystems.com.br>"]
edition = "2018"

[dependencies]
async-std = { version = "1", features = ["attributes"] }
http-client = { version = "6", default-features = false, features = ["h1_client"] }
mockito = "0.29"

The same example does work as expected if I swap to curl_client feature with the NativeClient.

Running a wget to the running mockito server game this output:

$ wget -O- http://localhost:1234/report
--2021-03-08 16:13:12--  http://localhost:1234/report
Resolving localhost (localhost)... ::1, 127.0.0.1
Connecting to localhost (localhost)|::1|:1234... failed: Connection refused.
Connecting to localhost (localhost)|127.0.0.1|:1234... connected.
HTTP request sent, awaiting response... 200 OK

Maybe the h1-client is not trying to resolve the hostname to an ipv4 after it fails to resolve to an ipv6?

Fishrock123 commented 3 years ago

Does this work with http-client pinned to =6.0.0 and async-h1 pinned to =2.0.0 (or =2.1.2)?

Jonathas-Conceicao commented 3 years ago

I had to use patch since these versions were yanked from crates.io, but I think I got it right. Unfortunately neither the async-h1 versions worked. This is what I've tryed on the Cargo.toml

[dependencies]
async-std = { version = "1", features = ["attributes"] }
http-client = { version = "=6.0.0", default-features = false, features = ["h1_client"] }
async-h1 = "=2.1.2"
mockito = "0.29"
testcontainers = "0.12"

[patch.crates-io]
async-h1 = { git = 'https://github.com/http-rs/async-h1', tag = "v2.1.2" }
Fishrock123 commented 3 years ago

To be clear: http_client::native does not work with default-features = false, features = ["h1_client"] (And should not even be exposed?)

otavio commented 3 years ago

I debugged this issue a little and found that the code https://github.com/http-rs/http-client/blob/db0025ddff76eea20d5d5541b63a6d4899c72e2b/src/h1/mod.rs#L139-L144 returns the list of addresses. On the other hand, however, https://github.com/http-rs/http-client/blob/db0025ddff76eea20d5d5541b63a6d4899c72e2b/src/h1/mod.rs#L169 returns on the first error instead of trying to connect to the next available address.

otavio commented 3 years ago

A integration test for this issue could be done using:

#[atest]
async fn fallback_to_ipv4() {
    let client = DefaultClient::new();
    let _mock_guard = mock("GET", "/")
        .with_status(200)
        .expect_at_least(2)
        .create();

    // Kips the initial "http://127.0.0.1:" to get only the port number
    let mock_port = &mockito::server_url()[17..];

    let url = &format!("http://localhost:{}", mock_port);
    let req = Request::new(http_types::Method::Get, Url::parse(url).unwrap());
    client.send(req.clone()).await.unwrap();
}
Jonathas-Conceicao commented 3 years ago

To be clear: http_client::native does not work with default-features = false, features = ["h1_client"] (And should not even be exposed?)

Looks like I mixed up the examples while I was reporting the issue, I meant to post one using http_client::h1::H1Client there. I've edited the original mensagem to fix this.