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.

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

Fix HTTPS proxy support #48

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

richardmarbach
Copy link
Contributor

Hey, I'd like to apologise for the oversight in #47. I miscounted the arguments to the existing Net::HTTP constructor, as I had only tested directly with the Net::HTTP object. I tested this change directly with the adapter, and it's correct now.

The nil argument is a "Proxy Filter". I think it's safe to disable as Faraday doesn't seem to support proxy filters.

Sorry again for the oversight

@olleolleolle
Copy link
Member

Is it possible to add tests which would fail with the previous implementation?

Ensure proxy settings are set correctly.
Currently proxy_use_ssl is not exposed as a public method by net-http, so we need to check the instance variable.
@richardmarbach
Copy link
Contributor Author

richardmarbach commented Nov 13, 2024

@olleolleolle I've added some specs to check that both the HTTP and HTTPS options are set correctly. The specs also fails with the previous implementation

@olleolleolle olleolleolle merged commit e2ab196 into lostisland:main Nov 13, 2024
@richardmarbach richardmarbach deleted the fix_tls_proxy_support branch November 13, 2024 09:06
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.

2 participants