Skip to content

Add HTTPS Proxy support - #47

Merged
olleolleolle merged 2 commits into
lostisland:mainfrom
richardmarbach:add_tls_proxy_support
Nov 11, 2024
Merged

Add HTTPS Proxy support#47
olleolleolle merged 2 commits into
lostisland:mainfrom
richardmarbach:add_tls_proxy_support

Conversation

@richardmarbach

@richardmarbach richardmarbach commented Nov 11, 2024

Copy link
Copy Markdown
Contributor

net-http version 0.5.0 finally introduced TLS proxy support: ruby/net-http#55
https://github.com/ruby/net-http/releases/tag/v0.5.0

Since this change is breaking for older versions of net-http, I can also implement this in a backward compatible way.

@olleolleolle

Copy link
Copy Markdown
Member

@richardmarbach Cool feature! To support this, perhaps we can make this gem depend on net-http version 0.5.0+?

SSL proxies only supported in net-http 0.5.0 or above
@richardmarbach

Copy link
Copy Markdown
Contributor Author

Hey @olleolleolle, thanks for checking so quickly! I've added the minimum runtime dependency for net-http

@olleolleolle olleolleolle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, @richardmarbach!

@olleolleolle
olleolleolle merged commit 724af4d into lostisland:main Nov 11, 2024
@richardmarbach
richardmarbach deleted the add_tls_proxy_support branch November 12, 2024 07:49
Comment thread faraday-net_http.gemspec
spec.require_paths = ['lib']

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

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Switch to add_dependency

I don't think that makes a difference because https://guides.rubygems.org/specification-reference/#add_dependency states:

Also known as: add_runtime_dependency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants