middlewear.Recover causes undefined behaviour if a connection has been upgraded.

[tracplus-hpaterson]

The middlewear.Recover method attempts to write an HTTP 500 Internal Server Error to a response as part of the error recovery process. This will cause apparently undefined behavior if the connection has been hijacked.

In our use case, we have upgraded our connection to a WebSocket using golang.org/x/net/websocket/websocket.Handler to hijack the connection. A bug in our WebSocket handler, specifically a null pointer error, cause Chi to call Recover. The attempt to write to the upgraded connection after hijack caused apparently undefined behaviour: Internal variables were written down the WebSocket to the client, and the Go runtime reported a generic panic, reducing the diagnosting use of the Recover method.

Can we suggest the Recover method test if the connection has been upgraded or hijacked, and alter it's behavior appropriate - for example not trying to write to the HTTP connection, and possible providing an error over the WebSocket?

[pkieltyka]

Great find and idea. I won’t have time for this one for a while but feel free to PR if you’re interested

…

On Sep 23, 2021, at 1:05 AM, H Paterson ***@***.***> wrote:  The middlewear.Recover method attempts to write an HTTP 500 Internal Server Error to a response as part of the error recover process. This will cause apparently undefined behavior if the connection has been hijacked. In our use case, we have upgraded our connection to a WebSocket using golang.org/x/net/websocket/websocket.Handler to hijack the connection. A bug in our WebSocket handler, specifically a null pointer error, cause Chi to call Recover. The attempt to write to the upgraded connection after hijack caused apparently undefined behaviour: Internal variables were written down the WebSocket to the client, and the Go runtime reported a generic panic, reducing the diagnosting use of the Recover method. Can we suggest the Recover method test if the connection has been upgraded or hijacked, and alter it's behavior appropriate - for example not trying to write to the HTTP connection, and possible providing an error over the WebSocket? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[fatelei]

does this issue have been resovled？if not，it can assign to me

[pkieltyka]

@fatelei thank you :)