coder / websocket

Minimal and idiomatic WebSocket library for Go
ISC License
3.81k stars 286 forks source link

WebSocket Dial Handshake 403 When Connecting to Server Written Using x/net/websocket #461

Open Jyosua opened 1 month ago

Jyosua commented 1 month ago

We have a simplistic example websocket server written using this boilerplate in the labstack echo cookbook: https://echo.labstack.com/docs/cookbook/websocket#server

It's currently using x/net/websocket. On the client side that connects to this example server, I tried to swap out our x/net/websocket implementation for nhooyr/websocket, but when websocket.Dial is called, the handshake fails with this 403 error. I'm not sure why this is, as the default CORS for echo is to allow everything and if I swap back to the dialer in x/net/websocket, it works fine. Just in case, I've tried to provide a custom HTTP client with a custom transport that sets InsecureSkipVerify: true via the DialOptions, but this also did not work.

Jyosua commented 1 month ago

This turned out to be because we weren't sending any Origin header at all. I had originally followed the example in the readme, here:

ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()

c, _, err := websocket.Dial(ctx, "ws://localhost:8080", nil)
if err != nil {
    // ...
}
defer c.CloseNow()

err = wsjson.Write(ctx, c, "hi")
if err != nil {
    // ...
}

c.Close(websocket.StatusNormalClosure, "")

However, when written like this, no Origin header is sent. On the server side we're using labstack/echo, which has CORS configured to accept any Origin:

 e.Use(middleware.CORSWithConfig(middleware.CORSConfig{
        AllowOrigins: []string{"*"},
        AllowMethods: []string{http.MethodGet, http.MethodPut, http.MethodPost, http.MethodDelete},
    }))

Despite setting this, this results in a 403 during the handshake due to the lack of Origin header. The reason this didn't happen with x/net/websocket is because the Dial function in that library requires the origin be specified. So, given that, I feel like either a documentation change should be made to mention the possibility of this, or that specifying the origin be made mandatory to avoid the issue.

I also opened an issue on the echo side, here

aldas commented 1 month ago

I think your problem is with x/net/websocket library and with this https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L101 handshake method and that 403 originates from here https://github.com/golang/net/blob/765c7e89b3bdd76bfc210acddd3ca73931eb8d1d/websocket/server.go#L33

Jyosua commented 1 month ago

That does seem to be the source of the issue. Since this library is designed to be easy to use in place of the official x/net/websocket library, given that that one is not really maintained, I think it would be worth warning about this pitfall in the readme, as interop between the libraries may be a common usecase.