Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http1.1 upgrade doesn't work with rustls #2343

Closed
Yarn opened this issue Jul 4, 2024 · 4 comments
Closed

http1.1 upgrade doesn't work with rustls #2343

Yarn opened this issue Jul 4, 2024 · 4 comments

Comments

@Yarn
Copy link

Yarn commented Jul 4, 2024

I had a similar issue that was fixed at some point in hyper that I initially fixed by vendoring the openssl crate. In that case I'm pretty sure the issue was that the request was http2 even when http1 was explicitly set in the request.

I suspect this is a similar issue but I'm not sure. I can dig into it more deeply and work on a minimal reproduction of the issue if needed.

@ovnicraft
Copy link

@Yarn can you share a guide to reproduce it, would be helpful.

@Yarn
Copy link
Author

Yarn commented Jul 14, 2024

with the native-tls feature

[src/main.rs:29:5] res.version() = HTTP/1.1

with rustls-tls

[src/main.rs:29:5] res.version() = HTTP/2.0
thread 'main' panicked at src/main.rs:35:9:
no upgrade 404 []
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Cargo.toml

[package]
name = "ws_connect"
version = "0.1.0"
edition = "2021"

[dependencies]
futures = "0.3"
tokio = { version = "1", features = ["time", "rt", "rt-multi-thread", "macros"] }

# reqwest = { version = "0.11.23", features = ["native-tls"], default-features=false }
reqwest = { version = "0.11.23", features = ["rustls-tls"], default-features=false }

main.rs

use reqwest::StatusCode;
use reqwest::header::{UPGRADE, CONNECTION};

#[tokio::main]
async fn main() {
    let client = reqwest::ClientBuilder::new()
        .timeout(std::time::Duration::from_secs(10))
        .build().unwrap();
    
    let url = "https://gateway.discord.gg/?v=9&encoding=json";
    
    let req = client.get(url)
        .version(reqwest::Version::HTTP_11)
        
        .header(UPGRADE, "websocket")
        .header(CONNECTION, "Upgrade")
        .header("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==")
        .header("Sec-WebSocket-Version", "13")
        
        .build()
        .unwrap();
    
    let fut = client
        .execute(req);
    
    let res: reqwest::Response = fut.await.unwrap();
    
    dbg!(res.version());
    
    if res.status() != StatusCode::SWITCHING_PROTOCOLS {
        let status = res.status();
        let body: Vec<u8> = res.bytes()
            .await.unwrap().to_vec();
        panic!("no upgrade {:?} {:?}", status, body);
    }
    
    let upgraded = res
        .upgrade()
        .await.expect("on upgrade error");
}

@cxw620
Copy link
Contributor

cxw620 commented Aug 2, 2024

#2116 may be related to this issue.

It's hard to fix this issue perfectly, but you may set http_version_pref to http1 when create Client, like this:

// ...
    let client = reqwest::ClientBuilder::new()
        .http1_only()
        .timeout(std::time::Duration::from_secs(10))
        .build().unwrap();
// ...

@seanmonstar
Copy link
Owner

Yea, ALPN is forcing the connection to be HTTP/2, before the version of the request is taken into consideration. The solution right above this comment should work.

@seanmonstar seanmonstar closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants