Use binary frame for websocket attach endpoint#30460
Merged
estesp merged 1 commit intomoby:masterfrom Feb 2, 2017
Merged
Conversation
Contributor
|
ping @docker/core-engine-maintainers |
Member
|
@yongtang can you add a note to the API change history? https://github.com/docker/docker/blob/master/docs/api/version-history.md IIUC, users of this API endpoint may have to make changes for this, so it's important to mention this change |
Contributor
|
Will sending a binary frame actually break older clients? |
Member
|
@stevvooe apparently it does; #28176 (comment) |
yongtang
force-pushed
the
28176-attach-binary-frame-websocket
branch
from
78920d3 to
2edc4c5
Compare
This fix tries to address the issue raised in 28176 where
text frame was used in websocket attach endpoint. In case
the data send out contains non utf8 data, the connection
will be closed in certain browsers, e.g., Safari.
This fix address the issue by change `PayloadType` to `BinaryFrame`.
This fix is tested manually with Safari. The docker daemon is inside a Linux Virtual Machine.
Create a container with:
```
docker run -itd --name websocket busybox sh -c "while true; do echo -e 'he\\xc3\\x28o'; sleep 5; done"
```
Use the following url (172.16.66.128:2375 is the tcp address of the daemon):
```
file:///websocket.html?url=ws://172.16.66.128:2375/v1.25/containers/websocket/attach/ws?logs=1&stderr=1&stdout=1&stream=1&stdin=1
```
and the following html:
```
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Websocket</title>
<script type="text/javascript">
function DockerWebSocket() {
if ("WebSocket" in window) {
console.log("WebSocket is supported by Browser...")
// Remove '?url=' prefix
url = window.location.search.replace(/^(\?url=)/,"");
console.log("URL ["+url+"]...");
var ws = new WebSocket(url);
ws.onopen = function() {
console.log("Connection is opened...");
};
ws.onclose = function() {
console.log("Connection is closed...");
};
ws.onmessage = function (e) {
if (typeof e.data === "string") {
alert("WebSocket received text message ["+e.data+"]!")
} else {
console.log("Message is received...")
var blobReader = new FileReader();
blobReader.onload = function(event) {
console.log(JSON.stringify(blobReader.result))
};
blobReader.readAsText(e.data)
console.log("Message complete...")
}
};
} else {
alert("WebSocket is not supported by Browser!");
}
}
</script>
</head>
<body>
<div>
<a href="javascript:DockerWebSocket()">Run DockerWebSocket</a>
</div>
</body>
</html>
```
This fix fixes 28176.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang
force-pushed
the
28176-attach-binary-frame-websocket
branch
from
2edc4c5 to
e82dcaa
Compare
Member
Author
|
@thaJeztah the PR has been updated with API change history docs. Please take a look. @stevvooe It depends on the client library. Golang is fine. But for JavaScript client has to use |
ghost
mentioned this pull request
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
7 participants
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.
- What I did
This fix tries to address the issue raised in #28176 where text frame was used in websocket attach endpoint. In case the data send out contains non utf8 data, the connection will be closed in certain browsers, e.g., Safari.
- How I did it
This fix address the issue by change
PayloadTypetoBinaryFrame.- How to verify it
This fix is tested manually with Safari. The docker daemon is inside a Linux Virtual Machine.
Create a container with:
Use the following url (172.16.66.128:2375 is the tcp address of the daemon):
and the following websocket.html:
The following error is returned without fix:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)
This fix fixes #28176.
Signed-off-by: Yong Tang yong.tang.github@outlook.com