-
Notifications
You must be signed in to change notification settings - Fork 312
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
Compressed messages fail in Safari #218
Comments
Received an email report about this as well, will look into it. For now you can disable compression. |
Might be https://bugs.webkit.org/show_bug.cgi?id=202401 as reported in the email I got. |
Please let me know if this occurs on messages smaller than 4092 bytes. |
Yes, this also occurs with much smaller messages. A message with 186 bytes uncompressed size already causes the error. Messages with 30-40 bytes work well - maybe because they won't be compressed? |
Hmm, weird. Could you try the technology preview and see if that fixes it as the patches landed in Safari several months ago. |
Hmm actually fixes should be in 13.1 as it released just a week ago. |
Subscription error: client.subscribeRead - error reading data: failed to get reader: failed to read frame header: EOF coder/websocket#218
Best to just disable compression for now until I figure out what's going on here. |
To clarify I mean I'm going to turn off compression by default. |
Confirmed reproduction btw with the new chat example. It's proven handy already. |
@nhooyr we are facing a similar issue with Safari version
We get below error is mentioned Safari,
If we disable compression, it works fine. Is there any suggestion for using it with compression enabled? |
Now safari are using I'm suffering this issue only under k8s + istio + gin. While running the same code on my macbook, this problem disappeared. The log shows the error comes from deeply in |
/reopen |
Sorry I don't currently have the time to help debug, in the middle of closing a home, working a new job and moving. But I've reopened this issue to investigate at some future date. |
The data that we send over WebSockets is encrypted and thus not compressible. Additionally, Safari has a broken implementation of compression (see coder/websocket#218) that makes enabling it actively harmful. Fixes tailscale/corp#6943 Signed-off-by: Mihai Parparita <mihai@tailscale.com>
The data that we send over WebSockets is encrypted and thus not compressible. Additionally, Safari has a broken implementation of compression (see coder/websocket#218) that makes enabling it actively harmful. Fixes tailscale/corp#6943 Signed-off-by: Mihai Parparita <mihai@tailscale.com>
You can disable this conditionally only for Safari: acceptOptions := &websocket.AcceptOptions{
// ...
}
userAgentLower := strings.ToLower(r.Header.Get("User-Agent"))
isSafari := strings.Contains(userAgentLower, "safari") && !strings.Contains(userAgentLower, "chrome") && !strings.Contains(userAgentLower, "android")
if isSafari {
acceptOptions.CompressionMode = websocket.CompressionDisabled
}
conn, err := websocket.Accept(w, r, acceptOptions) See: poki/netlib#28 |
(also snuck in the disabling of websocket compression, see coder/websocket#218 (comment))
Currently all lobbies stay in the database even if they are empty. This change introduces a simple timer that deletes day old empty lobbies every 30 minutes. This is needed for better lobby listing (filters, private etc) in the future. Also snuck in the disabling of websocket compression, see coder/websocket#218 (comment)
Going to close as the commit in dev exists to disable it. |
Summary: It's unreliable Long answer: See coder/websocket#218 Signed-off-by: Yonle <yonle@lecturify.net>
When Safari 13.1 for MacOS receives a compressed text message, it prints
WebSocket connection to 'ws://localhost:8080/ws' failed: Could not decode a text frame as UTF-8
on the Javascript console and closes the WebSocket connection.This is probably a bug in Safari. But this issue might help others to disable compression as a workaround. They have to pass
AcceptOptions
toAccept()
:The text was updated successfully, but these errors were encountered: