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
WebSocket-specific INFO
#4255
WebSocket-specific INFO
#4255
Conversation
This fixes #4252 by ensuring that `tls_available`, `tls_required`, `host` and `port` are populated based on the WebSocket listener rather than standard listeners. Signed-off-by: Neil Twigg <neil@nats.io>
3947b01
to
afdc247
Compare
With this PR when connecting over WebSockets:
|
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
PR #4255 added code in generateClientInfoJSON to set the proper info Host/Port/TLSAvailable/TLSRequired fields to send to clients. However, this was requiring a lock but more importantly was computing the listener's host/port everytime, which is not necessary since this is immutable because we don't support the change during a config reload. Also, the TLSRequired field was set based on the server TLSConfig's InsecureSkipVerify value, which is irrelevant for a server. The mere presence of a TLSConfig (c.srv.websocket.tls being true) is enough. I have modified the TestWSReloadTLSConfig test to verify that the tls block cannot be removed and no_tls set to true, which means that tls value can't change. I also added check for the info's Host/Port/TLSAvailable/TLSRequired values. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
PR #4255 added code in generateClientInfoJSON to set the proper info Host/Port/TLSAvailable/TLSRequired fields to send to clients. However, this was requiring a lock but more importantly was computing the listener's host/port everytime, which is not necessary since this is immutable because we don't support the change during a config reload. Also, the TLSRequired field was set based on the server TLSConfig's InsecureSkipVerify value, which is irrelevant for a server. The mere presence of a TLSConfig (c.srv.websocket.tls being true) is enough. I have modified the TestWSReloadTLSConfig test to verify that the tls block cannot be removed and no_tls set to true, which means that tls value can't change. I also added check for the info's Host/Port/TLSAvailable/TLSRequired values. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This fixes #4252 by ensuring that
tls_available
,tls_required
,host
andport
are populated based on the WebSocket listener rather than standard listeners.Signed-off-by: Neil Twigg neil@nats.io