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

Not sending ALPN when using socks5 proxy #2118

Closed
cxw620 opened this issue Jan 30, 2024 · 2 comments · Fixed by #2164
Closed

Not sending ALPN when using socks5 proxy #2118

cxw620 opened this issue Jan 30, 2024 · 2 comments · Fixed by #2164

Comments

@cxw620
Copy link
Contributor

cxw620 commented Jan 30, 2024

I met with a weird issue. When using socks5 proxy, reqwest will use HTTP/1.1 during request, though HTTP/2 when using http/https proxy. Through Charles and verbose log I found that no ALPN were sent when using socks5 proxy.

I checked source code and noticed such codes:

reqwest/src/connect.rs

Lines 148 to 150 in ddf7f24

let mut tls_proxy = tls.clone();
tls_proxy.alpn_protocols.clear();
(Arc::new(tls), Arc::new(tls_proxy))

reqwest/src/connect.rs

Lines 205 to 225 in ddf7f24

#[cfg(feature = "__rustls")]
Inner::RustlsTls { tls_proxy, .. } => {
if dst.scheme() == Some(&Scheme::HTTPS) {
use std::convert::TryFrom;
use tokio_rustls::TlsConnector as RustlsConnector;
let tls = tls_proxy.clone();
let host = dst.host().ok_or("no host in url")?.to_string();
let conn = socks::connect(proxy, dst, dns).await?;
let server_name = rustls::ServerName::try_from(host.as_str())
.map_err(|_| "Invalid Server Name")?;
let io = RustlsConnector::from(tls)
.connect(server_name, conn)
.await?;
return Ok(Conn {
inner: self.verbose.wrap(RustlsTlsConn { inner: io }),
is_proxy: false,
tls_info: false,
});
}
}

I think here should clone tls instead of tls_proxy since tls_proxy.alpn_protocols.clear() introduced in #466 fixing #459.
image

I simply verified my thoughts with testing, but I'm still uncertain, so I submit this issue.

@abacabadabacaba
Copy link

I encountered the same issue. Looking at the code, it looks like a bug.

This "feature" was introduced by #466. It seems that before it, it tried to use ALPN, but not properly activating h2 backend for some reason.

@cxw620

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants