Skip to content

Fix Recoverer upgrade detection for case-insensitive tokenized Connection header#1077

Open
andrewstellman wants to merge 2 commits intogo-chi:masterfrom
andrewstellman:fix-recoverer-upgrade-detection
Open

Fix Recoverer upgrade detection for case-insensitive tokenized Connection header#1077
andrewstellman wants to merge 2 commits intogo-chi:masterfrom
andrewstellman:fix-recoverer-upgrade-detection

Conversation

@andrewstellman
Copy link
Copy Markdown

Summary

The Recoverer middleware checks whether a request is a WebSocket upgrade before writing a 500 status after a panic. It currently uses an exact string comparison:

if r.Header.Get("Connection") != "Upgrade" {
    w.WriteHeader(http.StatusInternalServerError)
}

This misses two cases that are valid per HTTP specs:

  • Case variation: Connection: upgrade (lowercase) — HTTP headers are case-insensitive (RFC 7230 §3.2), but Header.Get returns the value as-is and the comparison is case-sensitive.
  • Token lists: Connection: keep-alive, Upgrade — the Connection header can contain multiple comma-separated tokens (RFC 7230 §6.1), but Header.Get returns the full value and the comparison requires an exact match.

In both cases, the Recoverer incorrectly writes a 500 status on a connection that is being upgraded, which can corrupt the WebSocket handshake.

Fix

Replace the exact string comparison with a helper that splits the header into comma-separated tokens and does a case-insensitive match against each one.

Tests

Four test cases in TestRecovererUpgradeConnectionDetection:

Case Connection header Expected
exact Upgrade Upgrade no 500
lowercase upgrade no 500 (was 500)
token list keep-alive, Upgrade no 500 (was 500)
no header (empty) 500

Full test suite passes (go test ./...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant