Initialise TLS sockets directly if no socks5 proxy is configured #163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In cases where no proxy is configured, first establishing a TCP connection and then upgrading the connection to TLS can cause errors on some TLS servers. This commit splits the logic in such a way that configurations without proxies (presumably the majority!) will connect directly using `ssl:connect`, whilst proxy connections will still establish a socket first and then upgrade. This fixes cmullaparthi#160.
This was referenced Jul 13, 2018
|
Thanks Vincent, looks good to me. |
|
Thanks for the quick response! :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In cases where no proxy is configured, first establishing a TCP
connection and then upgrading the connection to TLS can cause errors
on some TLS servers.
This commit splits the logic in such a way that configurations without
proxies (presumably the majority!) will connect directly using
ssl:connect, whilst proxy connections will still establish a socketfirst and then upgrade.
This fixes #160.
Note: I've verified that the issue described in #160 doesn't occur with this change. However, when running tests locally, some of them seem to fail. I can't see a way in which those failures are related right now, but I'm about 2 hours behind schedule on going to bed so it'll have to wait if it occurs in CI, too :-)