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.

Sign up for GitHub

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

Jump to bottom

Add HTTPS Proxy support #47

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Add HTTPS Proxy support #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8bfc3c
Add HTTPS Proxy support
richardmarbach Nov 11, 2024
8612fa7
Add mininum net-http version
richardmarbach Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion faraday-net_http.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ Gem::Specification.new do |spec|
spec.files = Dir.glob('lib/**/*') + %w[README.md LICENSE.md]
spec.require_paths = ['lib']

spec.add_runtime_dependency 'net-http'
spec.add_runtime_dependency 'net-http', '>= 0.5.0'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version limitation creates an issue when gems are installed at runtime. The standard version is lower than the required one, leading to two different versions being loaded simultaneously. This causes the following error:

Unable to activate faraday-net_http-3.4.0, because net-http-0.4.1 conflicts with net-http (>= 0.5.0) (Gem::ConflictError)

The workaround was to keep using the previous version of faraday-net-http.

😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleolleolle this is probably due to the add_runtime_dependency here.
I believe we have 2 options to address this:

  1. Switch to add_dependency
  2. Remove the version dependency and update the code to support both

The reason we didn't explicitly depend on net-http was that this would cause compatibility issues with the "stdlib" Net::HTTP bundled with Ruby. However, I believe this was removed in Ruby 3.0 and since we support only Ruby 3.0+ it should be safe to make this dependency more concrete now 👍

Would appreciate if you could double-check my claim here, I'm mostly going by memory and a quick Google Search 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also what we state in the gemspec, so I'd go with option 1 unless I'm missing something important?

end
3 changes: 2 additions & 1 deletion lib/faraday/adapter/net_http.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ def net_http_connection(env)
if proxy
Net::HTTP.new(env[:url].hostname, port,
proxy[:uri].hostname, proxy[:uri].port,
proxy[:user], proxy[:password])
proxy[:user], proxy[:password],
proxy[:uri].scheme == 'https')
else
Net::HTTP.new(env[:url].hostname, port, nil)
end
Expand Down