-
Notifications
You must be signed in to change notification settings - Fork 521
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
apiclient: update tungstenite to v0.20.1 #3491
apiclient: update tungstenite to v0.20.1 #3491
Conversation
sources/deny.toml
Outdated
# tungstenite uses a newer version of base64, but must of the | ||
# first party crates are still in this version | ||
{ name = "base64", version = "=0.13.1" } |
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.
After the next tough
release, we can switch everything to 0.21
.
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 think in the past we had a case where apiclient processes became zombies and maybe tungstenite was to blame? Anyone remember? Should we check for zombies?
Newer versions of tungstenite implemented the v13 version of the Web Sockets protocol (described in https://www.rfc-editor.org/rfc/rfc6455). In this version, new headers are required in the upgrade request. With this update, now the upgrade request sent by apiclient include the missing headers. In the same version, the 'frame' message was added. Neither apiserver nor apiclient send this type of message, so they are ignored by apiclient. Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
14d6c7d
to
c475e12
Compare
(Forced push fixes the comments in the last revision) |
Description of changes:
Newer versions of
tungstenite
implemented the v13 version of the Web Sockets protocol (described in rfc6455). In this version, new headers are required in the upgrade request. With this update, now the upgrade request sent byapiclient
include the missing headers.In the same version, the 'frame' message was added. Neither
apiserver
norapiclient
send this type of message, so they are ignored byapiclient
.I found the required headers here. For the values I followed the RFC:
Testing done:
I built the
aws-ecs-1
andaws-ecs-2
variants, and confirmed I could exec commands through the admin container:I also confirmed that I can enter the admin container:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.