net_http adapter: Fix to avoid calling configure_ssl for HTTP connections#38
Merged
Merged
Conversation
`env[:ssl]` is an instance of `Faraday::SSLOptions`, and as a result, `configure_ssl` is always executed, even when the connection is to HTTP. Fixes lostisland#37
Contributor
Author
|
I've found that the bug also exists in previous versions such as 1.0.1. I'm not sure about the maintenance policy of this library, but I think it would be beneficial to backport the fix to those versions as well. What do you think? |
| net_http_connection(env).tap do |http| | ||
| http.use_ssl = env[:url].scheme == 'https' if http.respond_to?(:use_ssl=) | ||
| configure_ssl(http, env[:ssl]) | ||
| if env[:url].scheme == 'https' && env[:ssl] |
Member
There was a problem hiding this comment.
Minor: What if this is even smaller? Making the smallest check first.
Suggested change
| if env[:url].scheme == 'https' && env[:ssl] | |
| if env[:ssl] && env[:url].scheme == 'https' |
configure_sslconfigure_ssl
configure_sslconfigure_ssl when using a HTTP connection
configure_ssl when using a HTTP connectionconfigure_ssl for HTTP connections
Member
|
@ma2gedev If you can make a backport, I think I can make a release. |
Member
|
Right, now https://rubygems.org/gems/faraday-net_http/versions/3.1.1 exists. Since CI was stopped, you had no way of seeing the lint issue which was easy to fix. I made the CI start. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
env[:ssl]is an instance ofFaraday::SSLOptions, and as a result,configure_sslis always executed, even when the connection is to HTTP. This MR fixes the code so that SSL configuration is only performed when necessary, similar to other adapters such as faraday-httpclient and faraday-patron.Details can be found in #37 .