Attempt to overwrite response headers when gorilla/websocket upgrade fails
- Lenguaje dominante
- Go
- Estrellas
- 10.8k
- Forks
- 1.3k
- Merge medio
- 2 d 36 min
- PR fusionados (30 d)
- 26
Descripción
In transport/websocket.go, when `Upgrader.Upgrade()` returns an error, [we try to send a `http.StatusBadRequest`](https://github.com/99designs/gqlgen/blob/master/graphql/handler/transport/websocket.go#L96C2-L101C3)
```go
func (t Websocket) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecutor) {
t.injectGraphQLWSSubprotocols()
ws, err := t.Upgrader.Upgrade(w, r, http.Header{}) // <-- already writes errors
if err != nil {
// error to request already written
log.Printf("unable to upgrade %T to websocket %s: ", w, err.Error())
SendErrorf(w, http.StatusBadRequest, "unable to upgrade") // <-- overwriting headers. Seems unnecessary?
return
}
```
but the [gorilla Upgrade func](https://github.com/gorilla/websocket/blob/main/server.go#L124) itself already writes HTTP errors if `Upgrader.Upgrade()` returns an error, quoting the source of v1.5.0:
``` go
// If the upgrade fails, then Upgrade replies to the client with an HTTP error
// response.
func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeader http.Header) (*Conn, error)
```
so we end up attempting to overwrite response headers every time an upgrade fails (e.g. when `CheckOrigin` returns `false`, a HTTP 403 Forbidden is written in Upgrader.Upgrade(), but we attempt to overwrite it with 400 Bad Request).
It's nothing dramatic, but my console is full of:
```
[GIN-debug] [WARNING] Headers were already written. Wanted to override status code 403 with 400
```
Guía de contribución
Evaluación
Este issue todavía no se ha evaluado.