Expose OpenSSL::SSL::SSLContext object#303
Open
rhenium wants to merge 2 commits into
Open
Conversation
Add a new test case to confirm the current behavior. Commit fa68e64 fixed the handling of certificate identity check when connecting to an IP address, but the happy path is currently not covered by the test suite.
Add Net::HTTP#ssl_context to return the SSLContext used for HTTPS connections, and reuse the same object for future connections made by the same Net::HTTP instance. Also change existing SSL/TLS-related accessors to interact with the SSLContext directly, rather than temporarily storing values in instance variables and applying them later in Net::HTTP#start. Note that the SSLContext becomes frozen after the first connection is made using the Net::HTTP instance. Although I do not expect this to affect most users, it is a compatibility concern. [Feature #19641]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
Net::HTTP#ssl_contextto return theSSLContextused for HTTPS connections, and reuse the same object for future connections made by the sameNet::HTTPinstance.Also change existing SSL/TLS-related accessors to interact with the
SSLContextdirectly, rather than temporarily storing values in instance variables and applying them later inNet::HTTP#start.Note that the
SSLContextbecomes frozen after the first connection is made using theNet::HTTPinstance. Although I do not expect this to affect most users, it is a compatibility concern.[Feature #19641] https://bugs.ruby-lang.org/issues/19641
/cc @shouichi
An alternative to #147. Instead of accepting user-supplied
SSLContext, this PR makesNet::HTTPprepareSSLContextearlier and expose it. This avoids ambiguity when bothNet::HTTPaccessors andSSLContextaccessors are used.This doesn't address my comment in the PR #147 (comment), but I think this incompatibility may be unavoidable. It should be documented in the release note.
This resolves #139 by allowing users to do the following: