We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
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
keep-alive
Branch/Environment/Version
Describe the bug Connection header is deleted and not upgraded even if Upgrade is present, but with other Connection like keep-alive:
Upgrade
Connection
GET /ws HTTP/1.1 Host: <...> Accept-Encoding: gzip, deflate, br, zstd Sec-WebSocket-Version: 13 Sec-WebSocket-Extensions: permessage-deflate Sec-WebSocket-Key: X62lCXELOHFcBBG72P2S2Q== Connection: Upgrade, keep-alive Upgrade: websocket
This notably affects Firefox users when trying to dial the tyk gateway.
Reproduction steps Steps to reproduce the behavior:
Test case
Added a test at gateway/gateway_test.go.
gateway/gateway_test.go
func TestWebsockets(t *testing.T) { ts := StartTest(nil) defer ts.Close() globalConf := ts.Gw.GetConfig() globalConf.HttpServerOptions.EnableWebSockets = true ts.Gw.SetConfig(globalConf) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.Proxy.ListenPath = "/" }) baseURL := strings.Replace(ts.URL, "http://", "ws://", -1) url, err := url.Parse(baseURL) if err != nil { t.Fatalf("cannot parse url: %v", err) } conn, err := net.Dial("tcp", url.Host) if err != nil { t.Fatalf("cannot make connection: %v", err) } defer conn.Close() req := fmt.Sprintf(`GET %s/ws HTTP/1.1 Host: %s Accept-Encoding: gzip, deflate, br, zstd Sec-WebSocket-Version: 13 Sec-WebSocket-Extensions: permessage-deflate Sec-WebSocket-Key: X62lCXELOHFcBBG72P2S2Q== Connection: Upgrade, keep-alive Upgrade: websocket `, baseURL, url.Host) req = strings.Replace(req, "\n", "\r\n", -1) _, err = conn.Write([]byte(req)) if err != nil { t.Fatalf("cannot write request: %v", err) } buf, err := bufio.NewReader(conn).ReadString('\n') if err != nil { t.Fatalf("cannot read response: %v", err) } if !strings.Contains(buf, "HTTP/1.1 101 Switching Protocols") { t.Error("Unexpected response:", buf) } _, _ = ts.Run(t, test.TestCase{ Method: "GET", Path: "/abc", Code: http.StatusOK, }) }
Via Firefox
Firefox send a Connection: Upgrade, keep-alive when trying to connect to a websocket (GraphQL).
Connection: Upgrade, keep-alive
Actual behavior
Test panic. By applying a debug at
tyk/gateway/testutil.go
Lines 461 to 464 in 2a2a984
+ b, _ := httputil.DumpRequest(req, false) + fmt.Println(string(b)) conn, err := upgrader.Upgrade(w, req, nil) if err != nil { http.Error(w, fmt.Sprintf("cannot upgrade: %v", err), http.StatusInternalServerError) + fmt.Println("cannot upgrade:", err) }
It prints:
time="Aug 06 12:44:59" level=info msg="starting test" time="Aug 06 12:44:59" level=info msg="Rich plugins are disabled" prefix=coprocess GET /ws HTTP/1.1 Host: 127.0.0.1:16500 Accept-Encoding: gzip, deflate, br, zstd Sec-Websocket-Extensions: permessage-deflate Sec-Websocket-Key: X62lCXELOHFcBBG72P2S2Q== Sec-Websocket-Version: 13 User-Agent: Tyk/v5.5.0-dev X-Forwarded-For: 127.0.0.1 cannot upgrade: websocket: the client is not using the websocket protocol: 'upgrade' token not found in 'Connection' header panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x46911ff] goroutine 182 [running]: github.com/gorilla/websocket.(*Conn).NextReader(0x0) github.com/gorilla/[email protected]/conn.go:1000 +0x3f github.com/gorilla/websocket.(*Conn).ReadMessage(0x0) github.com/gorilla/[email protected]/conn.go:1093 +0x65 github.com/TykTechnologies/tyk/gateway.(*Test).testHttpHandler.func1.1() tyk/gateway/testutil.go:472 +0x55 created by github.com/TykTechnologies/tyk/gateway.(*Test).testHttpHandler.func1 in goroutine 180 tyk/gateway/testutil.go:470 +0x4f6 Process 15407 has exited with status 2
Connection header has been filtered and the connection is not upgraded (conn is nil), causing a panic in the test case.
Expected behavior
Connection should be upgraded and the header should be passed.
Cause
tyk/internal/httputil/streaming.go
Lines 25 to 37 in 2a2a984
Detections of "upgrade" in "Connection" header is too strict (!=) and should be more flexible (not contains).
!=
not contains
Possible solutions
Using nhooyr.io's implementation style:
func IsUpgrade(req *http.Request) (string, bool) { if !headerContainsTokenIgnoreCase(req.Header, headerConnection, "Upgrade") { return "", false } upgrade := strings.ToLower(strings.TrimSpace(req.Header.Get(headerUpgrade))) if upgrade != "" { return upgrade, true } return "", false } func headerContainsTokenIgnoreCase(h http.Header, key, token string) bool { for _, t := range headerTokens(h, key) { if strings.EqualFold(t, token) { return true } } return false } func headerTokens(h http.Header, key string) []string { key = textproto.CanonicalMIMEHeaderKey(key) var tokens []string for _, v := range h[key] { v = strings.TrimSpace(v) for _, t := range strings.Split(v, ",") { t = strings.TrimSpace(t) tokens = append(tokens, t) } } return tokens }
Nhooyr implementation seems pretty standard while gorilla/websocket seems "home-made".
If you could please fix this as this literally block all firefox users in using WS (including GraphQL subscriptions). Thank you 🙏 .
The text was updated successfully, but these errors were encountered:
+1, having the same issue here
Sorry, something went wrong.
Successfully merging a pull request may close this issue.
Branch/Environment/Version
Describe the bug
Connection header is deleted and not upgraded even if
Upgrade
is present, but with otherConnection
likekeep-alive
:This notably affects Firefox users when trying to dial the tyk gateway.
Reproduction steps
Steps to reproduce the behavior:
Test case
Added a test at
gateway/gateway_test.go
.Via Firefox
Firefox send a
Connection: Upgrade, keep-alive
when trying to connect to a websocket (GraphQL).Actual behavior
Test panic. By applying a debug at
tyk/gateway/testutil.go
Lines 461 to 464 in 2a2a984
It prints:
Connection header has been filtered and the connection is not upgraded (conn is nil), causing a panic in the test case.
Expected behavior
Connection should be upgraded and the header should be passed.
Cause
tyk/internal/httputil/streaming.go
Lines 25 to 37 in 2a2a984
Detections of "upgrade" in "Connection" header is too strict (
!=
) and should be more flexible (not contains
).Possible solutions
Using nhooyr.io's implementation style:
Nhooyr implementation seems pretty standard while gorilla/websocket seems "home-made".
If you could please fix this as this literally block all firefox users in using WS (including GraphQL subscriptions). Thank you 🙏 .
The text was updated successfully, but these errors were encountered: