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

The link to OpenSSL::SSL::SSLContext#set_params in the README is not particularly helpful #207

Open
x-yuri opened this issue Jul 12, 2024 · 3 comments

Comments

@x-yuri
Copy link

x-yuri commented Jul 12, 2024

https://github.com/redis-rb/redis-client/blob/v0.22.2/README.md#configuration
https://www.rubydoc.info/stdlib/openssl/OpenSSL%2FSSL%2FSSLContext:set_params

Maybe it was some time ago, but these days it's not.

The link provided by the openssl gem is.

Also I noticed a mysterious (supposedly) no-op line in the code:

        context.set_params(params)
        if context.verify_mode != OpenSSL::SSL::VERIFY_NONE
          if context.respond_to?(:verify_hostname) # Missing on JRuby
            context.verify_hostname  # O.o
          end
        end

        context

introduced here.

verify_hostname looks like a read-write attribute to me. According to the documentation and the code. Can you explain what it's doing?

@x-yuri
Copy link
Author

x-yuri commented Jul 12, 2024

I see, you provided a permalink, which kind of narrows on the method, not showing the rest:

https://www.rubydoc.info/stdlib/openssl/OpenSSL%2FSSL%2FSSLContext:set_params

Which is actually confusing. I think the following link would be better:

https://www.rubydoc.info/stdlib/openssl/OpenSSL/SSL/SSLContext#set_params-instance_method

I looked for the verify_hostname attribute, didn't found it there and started looking in the code.

@casperisfine
Copy link
Collaborator

casperisfine commented Jul 15, 2024

If you think that part of the doc can be improved I'll accept a PR.

verify_hostname looks like a read-write attribute to me. According to the documentation and the code. Can you explain what it's doing?

I honestly don't remember why I did this, likely just a mistake that wasn't noticed because it's a noop.

@pboling
Copy link

pboling commented Sep 10, 2024

But it looks like it shouldn't be a noop, doesn't it? Why is there a verify mode if not to verify?

if context.verify_mode != OpenSSL::SSL::VERIFY_NONE

Perhaps the verification is happening, but at a lower level, and the entire block is not needed here?

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

No branches or pull requests

3 participants