-
Notifications
You must be signed in to change notification settings - Fork 154
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
tls_available support #1127
tls_available support #1127
Conversation
if (tlsFirst && sslContext == null) { | ||
throw new IllegalStateException("SSL context required for tls handshake first"); | ||
} | ||
|
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.
I did not used to check this before, but if there is no SSL Context, we can't do any tls anyway, so it's a fast fail essentially.
*/ | ||
public boolean isTLSRequired() { | ||
return tlsFirst || this.sslContext != null; | ||
return sslContext != 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.
reflects checking tls first and ssl context in the builder
@@ -67,7 +68,8 @@ public ServerInfo(String json) { | |||
headersSupported = readBoolean(jv, HEADERS); | |||
authRequired = readBoolean(jv, AUTH_REQUIRED); | |||
nonce = readBytes(jv, NONCE); | |||
tlsRequired = readBoolean(jv, TLS); | |||
tlsRequired = readBoolean(jv, TLS_REQUIRED); | |||
tlsAvailable = readBoolean(jv, TLS_AVAILABLE); |
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.
71: Better variable name
72: read the value from the json
} | ||
} | ||
|
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.
This whole block is simplified.
- getOptions returns the options variable, just use the variable directly. There is no lazy loading or anything.
- No need to check 2.9.19 because websocket never does upgrade, it was just extraneous
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.
LGTM
Add support for server setting of
tls_available