-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add support for SSL Context per connection #180
Conversation
Job #180 is now in scope, role is |
assign @yegor256 |
@rultor merge |
@joris-lambrechts @yegor256 Oops, I failed. You can see the full log here (spent 6min)
|
@joris-lambrechts there are some failures in the build. Are you sure it works? |
@joris-lambrechts are you going to fix the branch or we close this? |
final int connect, final int read, | ||
final SSLContext context) throws IOException { | ||
final SSLContext resolvedContext; | ||
if (context == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joris-lambrechts On the other hand, it seems to me now that in TrustedWire
(which should disable SSL verification) we should disregard incoming context
completely and replace it with one which comes from context()
method.
Also it appears that the whole feature of injection SSLContext
could be done with Wire
instead of adding a method to Request
.
request.through(SecureWire.class, sslcontext).fetch()
But the decision is @yegor256's
@joris-lambrechts @yegor256 |
Closing for being old. |
Job |
I opted to add another parameter to the
Wire.send
method although the list of parameters is too high. I ran the same test from the issue with theTrustedWire
and got the same result as previous without it so that's good enough for me.Any feedback is welcome.
#178