Skip to content

fix: generate proper Sec-WebSocket-Key for WebSocket handshake#1228

Open
Tonkpils wants to merge 1 commit intogo-rod:mainfrom
Tonkpils:fix/sec-websocket-key
Open

fix: generate proper Sec-WebSocket-Key for WebSocket handshake#1228
Tonkpils wants to merge 1 commit intogo-rod:mainfrom
Tonkpils:fix/sec-websocket-key

Conversation

@Tonkpils
Copy link
Copy Markdown

@Tonkpils Tonkpils commented Mar 15, 2026

The handshake method sets Sec-WebSocket-Key to the literal string "nil", which causes 400 Bad Request on WebSocket servers that validate the key per RFC 6455 §4.1 — notably Browserless.io. #1092 (comment)

What changed

  • Generate a proper key random 16-byte nonce, base64-encoded, instead of "nil"
  • Fix header override comparison the existing k == "Sec-WebSocket-Key" check uses exact case, but Go's http.Header canonicalizes keys (lowercase w in Websocket). Switched to strings.EqualFold so user-provided header overrides actually work.
  • Add test TestSecWebSocketKeyIsValid verifies the generated key is valid base64 (16 bytes decoded) and not "nil", and that consecutive keys are unique.

Context

Ran into this connecting to Browserless.io from a Go service using Rod. The literal "nil" key doesn't pass their handshake validation. Generating a real key per the RFC spec fixes it.

The handshake method set Sec-WebSocket-Key to the literal string "nil",
which causes 400 Bad Request on servers that validate the key per
RFC 6455 (e.g. Browserless.io).

- Generate a random 16-byte base64-encoded key instead of "nil"
- Fix header override comparison to use case-insensitive match
  (Go canonicalizes to Sec-Websocket-Key, breaking exact match)
- Add test verifying the key is valid base64, not "nil"
Comment thread lib/cdp/websocket.go
defaultSecKey := "nil"
keyBytes := make([]byte, 16)
_, _ = rand.Read(keyBytes)
defaultSecKey := base64.StdEncoding.EncodeToString(keyBytes)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could set directly secKey here instead of using 2 different vars.

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.

2 participants