seanmonstar / reqwest

An easy and powerful Rust HTTP Client
https://docs.rs/reqwest
Apache License 2.0
9.88k stars 1.12k forks source link

Issue when using corporate proxy #823

Open ramsayleung opened 4 years ago

ramsayleung commented 4 years ago

Hi Sean, @frejion was using ncspot with proxy which depends on rspotify, and rspotify depends on reqwest, and rspotify fails to connect through proxy.

This came up in this issue (for reference): https://github.com/ramsayleung/rspotify/issues/83

I think perhaps it's something related with reqwest as well, this is the rspotify code which sends request by reqwest:

    fn internal_call(
        &self,
        method: Method,
        url: &str,
        payload: Option<&Value>,
    ) -> Result<String, failure::Error> {
        let mut url: Cow<str> = url.into();
        if !url.starts_with("http") {
            url = ["https://api.spotify.com/v1/", &url].concat().into();
        }

        let mut headers = HeaderMap::new();
        headers.insert(AUTHORIZATION, self.auth_headers().parse().unwrap());
        headers.insert(CONTENT_TYPE, "application/json".parse().unwrap());

        let mut response = {
            let builder = CLIENT.request(method, &url.into_owned()).headers(headers);

            // only add body if necessary
            // spotify rejects GET requests that have a body with a 400 response
            let builder = if let Some(json) = payload {
                builder.json(json)
            } else {
                builder
            };

            builder.send().unwrap()
        };

        let mut buf = String::new();
        response
            .read_to_string(&mut buf)
            .expect("failed to read response");
        if response.status().is_success() {
            Ok(buf)
        } else {
            Err(failure::Error::from(ApiError::from(&response)))
        }
    }

this is the backtrace from @frejion :

thread 'main' panicked at 'called Result::unwrap() on an Err value: reqwest::Error { kind: Request, url: "api.spotify.com/v1/me", source: hyper::Error(Connect, Os { code: 10054, kind: ConnectionReset, message: "An existing connection was forcibly closed by the remote host." }) }', src\libcore\result.rs:1165:5
stack backtrace:
0: backtrace::backtrace::dbghelp::trace
at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.40\src\backtrace/dbghelp.rs:88
1: backtrace::backtrace::trace_unsynchronized
at C:\Users\VssAdministrator.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.40\src\backtrace/mod.rs:66
2: std::sys_common::backtrace::_print_fmt
at src\libstd\sys_common/backtrace.rs:77
3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
at src\libstd\sys_common/backtrace.rs:61
4: core::fmt::write
at src\libcore\fmt/mod.rs:1028
5: std::io::Write::write_fmt
at src\libstd\io/mod.rs:1412
6: std::sys_common::backtrace::_print
at src\libstd\sys_common/backtrace.rs:65
7: std::sys_common::backtrace::print
at src\libstd\sys_common/backtrace.rs:50
8: std::panicking::default_hook::{{closure}}
at src\libstd/panicking.rs:188
9: std::panicking::default_hook
at src\libstd/panicking.rs:205
10: std::panicking::rust_panic_with_hook
at src\libstd/panicking.rs:464
11: std::panicking::continue_panic_fmt
at src\libstd/panicking.rs:373
12: rust_begin_unwind
at src\libstd/panicking.rs:302
13: core::panicking::panic_fmt
at src\libcore/panicking.rs:139
14: core::result::unwrap_failed
at src\libcore/result.rs:1165
15: core::result::Result<T,E>::unwrap
at /rustc/73528e339aae0f17a15ffa49a8ac608f50c6cf14\src\libcore/result.rs:933
16: rspotify::spotify::client::Spotify::internal_call
at C:\Users\jonas.frei.cargo\registry\src\github.com-1ecc6299db9ec823\rspotify-0.8.0\src\spotify/client.rs:163
17: rspotify::spotify::client::Spotify::get
at C:\Users\jonas.frei.cargo\registry\src\github.com-1ecc6299db9ec823\rspotify-0.8.0\src\spotify/client.rs:189
18: rspotify::spotify::client::Spotify::me
at C:\Users\jonas.frei.cargo\registry\src\github.com-1ecc6299db9ec823\rspotify-0.8.0\src\spotify/client.rs:960
19: rspotify::spotify::client::Spotify::current_user
at C:\Users\jonas.frei.cargo\registry\src\github.com-1ecc6299db9ec823\rspotify-0.8.0\src\spotify/client.rs:966

I am not sure whether it does has something to do with reqwest?

seanmonstar commented 4 years ago

It's hard to know if it's proxy related without knowing the proxy settings or the logs. So far, it just looks like the server terminated the request upon connect.

freijon commented 4 years ago

Hello Sean, I'm using CNTLM (a local proxy server) which connects to our corporate proxy which uses NTLM authentication. This normally works just fine, you just set the environment variables http_proxy and https_proxy to https://127.0.0.1:<port> and this makes basically any app NTLM aware. In the case of ncspot however, there is this issue I can't get rid of and I expect there is an issue in reqwest that prevent this from working properly.

As you can see, some network operations work just fine, like receiving an access token, so I would exclude the possibility of a network issue.

See the debug output for further information. Let me know if you need more details.

seanmonstar commented 4 years ago

reqwest doesn't support NTLM yet, see https://github.com/seanmonstar/reqwest/issues/498

freijon commented 4 years ago

Yes, but with CNTLM no app has to support NTML. CNTML is a local http proxy which translates all traffic to NTML. Like this you can make all apps NTML capable. It works just fine (normally).

seanmonstar commented 4 years ago

Those logs look old... What version of reqwest are you using?

freijon commented 4 years ago

Reqwest seems to be a sub-dependency of ncspot. The versions used are 0.10.1 and 0.9.24

seanmonstar commented 4 years ago

I can't say whether the new version of reqwest (0.10.x) has fixed it, but it has seen some adjustments to the way proxies are used. Can you upgrade?

seanmonstar commented 4 years ago

I'm going to close this as out-of-date. If updating still shows a problem, we can re-open!

freijon commented 4 years ago

The thing iis , I don't know how to build ncspot with a current version of reqwest since it's a sub-dependency... Do you have an idea?

freijon commented 4 years ago

Hello @seanmonstar The issue persists with the newest version of reqwest

seanmonstar commented 4 years ago

Can you provide trace logs again?

freijon commented 4 years ago

It seems I did it wrong the first time. Now I figured out how to utilize the newest version of 'reqwest'. Unfortunately the app doesn't compile anymore. It appears that the get() function changed fundamentally between 0.9.24 and 0.10.4 and changed the returned type. I would have to modify a lot of code of 'ncspot' to make it compatible, just for this test. I guess I would have to ask the developer of 'ncspot' to upgrade to reqwest 0.10? Is there any other way we could test this?

freijon commented 4 years ago

What I actually don't get: 'ncspot' uses an old version of reqwest, but the Authentication works fine with the proxy. However, 'rspotify' uses the current version of 'reqwest'. According to the dev of 'ncspot' the app fails because of 'rspotify'. This leads me to believe that maybe we're facing a regression. It worked fine with 0.9.24 and stopped working with 0.10.4?

seanmonstar commented 4 years ago

It appears that the get() function changed fundamentally between 0.9.24 and 0.10

Yes, but it's easy to fix. The default API is now async, and if the blocking feature is enabled in the Cargo.toml, you can use reqwest::blocking::get.

seanmonstar commented 4 years ago

As for a regression, if so we should fix it! But I'm curious about something. In 0.9.x, reqwest didn't have automatic support for system proxies, and 0.10 does (on Unix, it looks for environment variables like curl, and on Windows it looks in the system registry). How were you configuring the proxy for 0.9?

freijon commented 4 years ago

After your hint to use blocking, I managed to build the application, thanks!

As for the proxy support, I'm on Windows and set the environment variables. Where in the registry does reqwest look?

seanmonstar commented 4 years ago

I'm on Windows and set the environment variables. Where in the registry does reqwest look?

Woops, correction: even on Windows, it checks for the http_proxy and https_proxy environment variables, and if nothing is found, it looks in the registry.

So in the logs, nothing stands out to me. Though, I've thought of this possible change: in 0.9, there was a default user-agent, and in 0.10, there is not. Could the proxy be requiring a user-agent?

freijon commented 4 years ago

Okay, then everything should work. These environment variables are set and work fine otherwise. The user-agent is a possible lead. However, in the CNTLM manual I don't see any information about a mandatory user-agent. Is there an easy way to revert this change and see if it makes a difference?