Skip to content
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

What causes "response.Write on hijacked connection" when using CompressHandler? #589

Closed
andig opened this issue Apr 17, 2020 · 6 comments
Closed
Labels

Comments

@andig
Copy link

andig commented Apr 17, 2020

I realize this is old (#212, #494). I understand

The warnings indicate that the application is calling http.ResponseWriter methods after the call to Upgrade.

Yet I'm failing to identify the cause. I have a web socket on /ws and a router HandleFunc on / for the index. There is also

router.Use(handlers.CompressHandler)

The CompressHandler causes the problem and gives the following stack trace:

runtime/debug.Stack(0x155047c, 0x108e8d0, 0xc0002307e0)
        /usr/local/Cellar/go/1.14.1/libexec/src/runtime/debug/stack.go:24 +0xab
runtime/debug.PrintStack()
        /usr/local/Cellar/go/1.14.1/libexec/src/runtime/debug/stack.go:16 +0x34
github.com/andig/evcc/api.debugLogger.Write(0x0, 0x0, 0xc0002307e0, 0x5d, 0x60, 0xc0002cf2d8, 0x108af29, 0xc0002307e0)
        /Users/andig/htdocs/evcc/api/log.go:70 +0x12c
log.(*Logger).Output(0xc0001a4ff0, 0x2, 0xc000230720, 0x5c, 0x0, 0x0)
        /usr/local/Cellar/go/1.14.1/libexec/src/log/log.go:181 +0x39e
log.(*Logger).Printf(0xc0001a4ff0, 0x1b73c74, 0x3b, 0xc0002cf518, 0x3, 0x3)
        /usr/local/Cellar/go/1.14.1/libexec/src/log/log.go:188 +0x90
net/http.(*Server).logf(0xc0001409a0, 0x1b73c74, 0x3b, 0xc0002cf518, 0x3, 0x3)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:3059 +0xa6
net/http.(*response).write(0xc0004ec9a0, 0xa, 0xc0002d4089, 0xa, 0xa, 0x0, 0x0, 0x1156659, 0xc0002cf640, 0xc0000200f0)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:1559 +0x28c
net/http.(*response).Write(0xc0004ec9a0, 0xc0002d4089, 0xa, 0xa, 0xe7791f700, 0x243b640, 0xc000020000)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:1547 +0x71
compress/gzip.(*Writer).Write(0xc0002d4000, 0x0, 0x0, 0x0, 0x2f707d0, 0x0, 0xc0000200f0)
        /usr/local/Cellar/go/1.14.1/libexec/src/compress/gzip/gzip.go:168 +0x518
compress/gzip.(*Writer).Close(0xc0002d4000, 0xc0002d4000, 0x0)
        /usr/local/Cellar/go/1.14.1/libexec/src/compress/gzip/gzip.go:237 +0x4b3
github.com/gorilla/handlers.CompressHandlerLevel.func1(0x1ec5aa0, 0xc0004ec9a0, 0xc00017e300)
        /Users/andig/go/pkg/mod/github.com/gorilla/[email protected]/compress.go:149 +0x1cd
net/http.HandlerFunc.ServeHTTP(0xc0002ea0c0, 0x1ec5aa0, 0xc0004ec9a0, 0xc00017e300)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:2012 +0x52
github.com/gorilla/mux.(*Router).ServeHTTP(0xc0001c20c0, 0x1ec5aa0, 0xc0004ec9a0, 0xc00017e100)
        /Users/andig/go/pkg/mod/github.com/gorilla/[email protected]/mux.go:210 +0x13f
net/http.serverHandler.ServeHTTP(0xc0001409a0, 0x1ec5aa0, 0xc0004ec9a0, 0xc00017e000)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:2807 +0xcf
net/http.(*conn).serve(0xc0004bab40, 0x1ec78e0, 0xc00011c180)
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:1895 +0x838
created by net/http.(*Server).Serve
        /usr/local/Cellar/go/1.14.1/libexec/src/net/http/server.go:2933 +0x5b7
http: response.Write on hijacked connection from compress/gzip.(*Writer).Write (gzip.go:168)

Versions

Go version: go version go1.14.1 darwin/amd64

The websocket conn is closed when the client goes away. The http part is only dealing with HandlerFuncs:

return func(w http.ResponseWriter, r *http.Request) {...}

...but not with connections. I'm never writing to a connection explicitly and only write bodies after writing headers. I'm totally stumped where or how I might be writing to a hijacked connection?

@andig andig added the question label Apr 17, 2020
@andig andig changed the title What causes "response.Write on hijacked connection"? What causes "response.Write on hijacked connection" when using CompressHandler? Apr 17, 2020
@ghost
Copy link

ghost commented Apr 17, 2020

To upgrade from the HTTP protocol to the WebSocket protocol, this package hijacks the network connection from the HTTP server. The response writer cannot be used after the network connection is pulled out from underneath it.

The application should not use the compression middleware on WebSocket connections.

Separate from that, the middleware should not close the gzip writer on hijacked connections.

@andig
Copy link
Author

andig commented Apr 17, 2020

@srybacki thank you very much, I really appreciate the answer! I feel my confusion- potentially that of others- might come from the terminology:

To upgrade from the HTTP protocol to the WebSocket protocol, this package hijacks the network connection from the HTTP server. The response writer cannot be used after the network connection is pulled out from underneath it.

Frankly- I don't comprehend. I'm doing:

router.HandleFunc("/ws", SocketHandler(hub))

The response writer is inside the handler:

func SocketHandler(hub *SocketHub) http.HandlerFunc {
	return func(w http.ResponseWriter, r *http.Request) {
		ServeWebsocket(hub, w, r)
	}
}

There are also the http handlers, example:

func HealthHandler(lp loadPoint) http.HandlerFunc {
	return func(w http.ResponseWriter, r *http.Request) {
		res := struct{ OK bool }{OK: true}
		log.TRACE.Println("health")

		w.WriteHeader(http.StatusOK)
		if err := json.NewEncoder(w).Encode(res); err != nil {
			log.ERROR.Printf("httpd: failed to encode JSON: %v", err)
		}
	}
}

How would I "pull (the connection) out from underneath it."? Do you mean a (shared) network connection owned by an http.Client? How would I prevent that?

What I'm trying to say is that I'm only dealing with response writers, never with connection except in case of the web socket handler which deals with client connections.

The application should not use the compression middleware on WebSocket connections.

Is that what I'm doing? I'm failing to see where I'm using a websocket connection?

Separate from that, the middleware should not close the gzip writer on hijacked connections.

Again- I'm not doing that, at least not consciously. The websocket handler is eventually Closed when the socket client goes away or cannot be written to, it does not have a handle for the gzip handler.

This is the general structure, probobly a familar (but flawed?) approach:

router := mux.NewRouter().StrictSlash(true)

// static
router.HandleFunc("/", indexHandler(links, liveAssets))

// individual handlers per folder
for _, folder := range []string{"js", "css", "webfonts", "ico"} {
	router.PathPrefix(prefix).Handler(http.StripPrefix(prefix, http.FileServer(Dir(liveAssets, prefix))))
}

// api
api := router.PathPrefix("/api").Subrouter()
api.Use(jsonHandler)
for _, r := range routes { ... }

// websocket
router.HandleFunc("/ws", SocketHandler(hub))

router.Use(handlers.CompressHandler)
router.Use(handlers.CORS(
	handlers.AllowedHeaders([]string{
		"Accept", "Accept-Language", "Content-Language", "Content-Type", "Origin",
	}),
))

It would be great to understand the root cause, thank you for bearing with me.

@ghost
Copy link

ghost commented Apr 17, 2020

The application's call to Upgrader.Upgrade hijacks the network connection from the HTTP server. The hijacked connection is the connection used by the HTTP server to receive HTTP requests from the client and send HTTP responses to the client. After the connection is upgraded from the HTTP protocol to the WebSocket protocol, the network connection cannot be used for HTTP. The net/http server panics when the application attempts to write an HTTP response to a hijacked connection. The compression middleware does this automatically on the line of code linked from my previous comment.

The application applies the compression middleware to all handlers using the function call.

 router.Use(handlers.CompressHandler)

Do not apply this middleware to the websocket handler. A rather blunt fix is to remove compression from all handlers by deleting the line of code. Because I am not familiar with the mux package, I don't have a suggestion for a more focused fix.

@andig
Copy link
Author

andig commented Apr 18, 2020

I still don't get how the pieces connect.

The application's call to Upgrader.Upgrade hijacks the network connection from the HTTP server. The hijacked connection is the connection used by the HTTP server to receive HTTP requests from the client and send HTTP responses to the client.

Is that really true? Imho what happens in the HTTP server is:

  • server creates Listener which listens to interface/port
  • listener accepts incoming connection- only at this point do you have a connection

There is no "connection used by the HTTP server per se", connections only exist at the point the client connects.

After the connection is upgraded from the HTTP protocol to the WebSocket protocol, the network connection cannot be used for HTTP. The net/http server panics when the application attempts to write an HTTP response to a hijacked connection.

That's clear. Once a websocket client connects that connection must not be used for gzip or anything else.

The compression middleware does this automatically on the line of code linked from my previous comment. ... The application applies the compression middleware to all handlers using the function call.

I think my confusion came from the fact that regular responses don't trigger the problem, gzip middleware does. The reason is clear now: gzip applies to all handlers, i.e. it cascades down.

Do not apply this middleware to the websocket handler.

Right. I MUST not apply additional handler to the router if it handles websockets on one of it's paths.

A rather blunt fix is to remove compression from all handlers by deleting the line of code. Because I am not familiar with the mux package, I don't have a suggestion for a more focused fix.

I'l check if that can be done using sub routers. Would propose to keep this open to add a suggested solution once I found one.

@andig
Copy link
Author

andig commented Apr 18, 2020

Here is what works for me. Don't add the CompressHandler to the root router:

router := mux.NewRouter().StrictSlash(true)
router.HandleFunc("/ws", SocketHandler(hub))

Even when serving from /, create a new subrouter and add handler there:

static := router.PathPrefix("/").Subrouter()
static.Use(handlers.CompressHandler)

That way, the websocket handler's response writer will not be affected by the CompressHandler

@ghost
Copy link

ghost commented Apr 18, 2020

I spent more time looking at the interaction between the interaction between the compression middleware and the websocket package. The compression middleware should step out of the way on an upgrade request. See gorilla/handlers#182.

This issue can be closed.

only at this point do you have a connection

... and the HTTP server uses this connection to receive requests and send responses to the client. This is the connection that upgrade hijacks from the HTTP server.

@andig andig closed this as completed Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant