Add support for ciphers in SSLOptions setting #44
Conversation
|
@bellvat Thanks, that's a nice addition. Can you think of some way to prove or test this code path? https://github.com/lostisland/faraday-http/blob/master/spec/faraday/http/adapter_spec.rb#L72 has some test code which may give you ideas. |
|
Hello @olleolleolle, I ensured that adding ciphers still allows the request to be successful. I tried to spy on OpenSSL::SSL::SSLContext, to see that it sets ciphers param, but it then it prevents actual OpenSSL behavior. I think calling the real OpenSSL is more valuable and saw the previous tests take this approach, so I chose to stick with this. |
| params = {} | ||
| %i[ | ||
| ca_file ca_path cert_store verify_depth | ||
| ca_file ca_path cert_store ciphers verify_depth |
There was a problem hiding this comment.
Q: If I remove the ciphers word from this, does the test fail?
There was a problem hiding this comment.
No, the test will pass.
I will try another way to make the test fail without adding the ciphers word. 🫡
There was a problem hiding this comment.
I have updated the test to address this in the latest commit. Thanks!
- without adding ciphers option/keyword, the test will now fail
olleolleolle
left a comment
olleolleolle
left a comment
There was a problem hiding this comment.
Thanks, this improves things!
|
Hello @olleolleolle, CI is failing on Ruby 2.7. I will investigate and open a new PR to fix this. |
2 participants
This PR is related to this open issue, which aims to support ciphers SSLOptions setting across adapters.