Skip to content

Fix SOCKS support for 'http://' URLs - #492

Merged
lovelydinosaur merged 2 commits into
masterfrom
socks-for-http
Jan 31, 2022
Merged

Fix SOCKS support for 'http://' URLs#492
lovelydinosaur merged 2 commits into
masterfrom
socks-for-http

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Contributor

Closes#491

The SOCKS proxy code was missing the check for "is this an https URL or not" at the point of setting up SSL with the remote endpoint.

Compare this...

# Upgrade the stream to SSL
ssl_context= (
default_ssl_context()
ifself._ssl_contextisNone
elseself._ssl_context
)
alpn_protocols= ["http/1.1", "h2"] ifself._http2else ["http/1.1"]
ssl_context.set_alpn_protocols(alpn_protocols)
kwargs= {
"ssl_context": ssl_context,
"server_hostname": self._remote_origin.host.decode("ascii"),
"timeout": timeout,
}
asyncwithTrace("connection.start_tls", request, kwargs) astrace:
stream=awaitstream.start_tls(**kwargs)
trace.return_value=stream

To the equivalent within a regular connection...

ifself._origin.scheme==b"https":
ssl_context= (
default_ssl_context()
ifself._ssl_contextisNone
elseself._ssl_context
)
alpn_protocols= ["http/1.1", "h2"] ifself._http2else ["http/1.1"]
ssl_context.set_alpn_protocols(alpn_protocols)
kwargs= {
"ssl_context": ssl_context,
"server_hostname": self._origin.host.decode("ascii"),
"timeout": timeout,
}
asyncwithTrace("connection.start_tls", request, kwargs) astrace:
stream=awaitstream.start_tls(**kwargs)
trace.return_value=stream

Bit silly really. Fix is easy-peasy.

@lovelydinosaurlovelydinosaur added the bug Something isn't working label Jan 28, 2022
@lovelydinosaur
lovelydinosaur requested a review from a teamJanuary 28, 2022 11:15
@lovelydinosaur
lovelydinosaur merged commit 1924b12 into masterJan 31, 2022
@lovelydinosaur
lovelydinosaur deleted the socks-for-http branch January 31, 2022 11:38
@lovelydinosaurlovelydinosaur mentioned this pull request Feb 1, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't working

Development

Successfully merging this pull request may close these issues.

2 participants

@lovelydinosaur@aminalaee