bugfix: client: bound the handshake response header read

[shreemaan-abhishek]

What

The WebSocket handshake response header block was read with an unbounded
sock:receiveuntil("\r\n\r\n"), with a standing -- FIXME: check for too big response headers next to it.

A malicious or broken server can stream bytes forever without ever sending the
\r\n\r\n terminator, and the cosocket buffers all of it. That exhausts the
nginx worker's memory and takes down every request that worker is handling, not
just the WebSocket one. The read timeout does not help: nginx resets the read
timer on each read event, so a steady drip keeps the read alive indefinitely.

CWE-770, remote DoS. It needs a hostile or compromised server, or a MITM on a
plaintext ws:// connection.

Note the asymmetry: server.lua has no equivalent hole, because nginx caps
request headers itself via large_client_header_buffers. Only the client side
was unbounded.

How

recv_header() reads at most max_header_len bytes by asking the iterator for
one byte past the limit: a header block that fits comes back whole, while an
oversized one, or a peer that never terminates the block at all, comes back at
the limit and is refused. Peak buffering is bounded at max_header_len + 1
bytes.

The same helper covers the CONNECT response read on the wss-over-proxy path,
which had the identical unbounded read.

Behavior change

max_header_len defaults to 8192, mirroring nginx's own 8k header buffer, so
the protection is on without opting in. A server sending a larger handshake
header block is now refused with:

failed to receive response header: response headers too large (limit: 8192 bytes)

Set max_header_len = 0 in client:new() to restore the previous unbounded
read.

Tests

t/max_header_len.t covers the default limit, max_header_len = 0, a smaller
explicit limit, a header block that is never terminated, new() rejecting a bad
value, and a real handshake still succeeding under the default.

Notes

Upstream openresty/lua-resty-websocket#94 proposes a max_header_len option
for the same FIXME. The option name and semantics here match it, so a later
upstream sync is a no-op rather than a conflict. The default differs: upstream
leaves it opt-in at 0, which keeps the DoS reachable out of the box.

Summary by CodeRabbit

• New Features

  • Added configurable limits for WebSocket handshake response headers.
  • Headers are limited to 8,192 bytes by default; oversized or incomplete headers now fail with a size error.
  • Set max_header_len to 0 to allow unlimited header sizes, or use a smaller non-negative limit.

• Documentation

  • Updated client configuration documentation with max_header_len behavior and defaults.

• Bug Fixes

  • Added consistent header-size validation during both proxy and WebSocket handshakes.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: a3dbf7c7-825e-4680-9358-f887703c7fab

📥 Commits

Reviewing files that changed from the base of the PR and between b910a04 and bd2cdc7.

📒 Files selected for processing (3)

• README.markdown
• lib/resty/websocket/client.lua
• t/max_header_len.t

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

📝 Walkthrough

Walkthrough

The WebSocket client now limits handshake response headers to 8192 bytes by default. The max_header_len option supports custom limits and unlimited reads with 0. Tests cover validation, proxy and WebSocket handshakes, oversized headers, and unterminated headers.

Changes

Handshake header limits

Layer / File(s)	Summary
Header limit contract and reader lib/resty/websocket/client.lua	The client validates max_header_len, applies an 8192-byte default, stores the option, and reads through CRLFCRLF with bounded or unlimited buffering.
Handshake integration lib/resty/websocket/client.lua	Proxy CONNECT and WebSocket handshake response headers use the configured limit.
Behavior validation and documentation t/max_header_len.t, README.markdown	Tests cover default, custom, unlimited, unterminated, invalid, and successful handshake cases. The README documents the option.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Proxy
  participant WebSocketServer
  Client->>Proxy: Send CONNECT request
  Proxy-->>Client: Return CONNECT response headers
  Client->>WebSocketServer: Send WebSocket handshake
  WebSocketServer-->>Client: Return handshake response headers
  Client->>Client: Apply max_header_len while reading headers

Loading

Suggested reviewers: bzp2010

Merge Risk: 🔵 Low · up to bd2cd

The client now bounds handshake headers by default, reducing memory-exhaustion exposure. Proxy CONNECT handling is described as using the same limit, but its path lacks a dedicated test, leaving a low bounded regression risk.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
E2e Test Quality Review	⚠️ Warning	The new suite uses real nginx-to-cosocket flows and covers default, unlimited, invalid, oversized, and unterminated headers. However, it violates the blocking error-handling criterion: several added c…	Check and handle the return values from every added client:new(), recv_frame(), and close() call. Add an end-to-end wss-through-HTTP-proxy test that sends an oversized and an unterminated CONNECT response, and include exact-limit …

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: limiting the WebSocket client handshake response header read.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Security Check	✅ Passed	No issues found in categories 1-7. The PR changes only WebSocket client header-length handling, documentation, and tests. The new error contains only the configured numeric limit and does not log or r…

Full details: E2e Test Quality Review

Explanation

The new suite uses real nginx-to-cosocket flows and covers default, unlimited, invalid, oversized, and unterminated headers. However, it violates the blocking error-handling criterion: several added cases call client:new() without checking its (wb, err) result, and the real-handshake case ignores wb:recv_frame() and wb:close() results. The suite also does not exercise an oversized or unterminated CONNECT response on the wss proxy path, although that is a separate changed handshake path. Existing proxy tests cover only a normal response.

Resolution

Check and handle the return values from every added client:new(), recv_frame(), and close() call. Add an end-to-end wss-through-HTTP-proxy test that sends an oversized and an unterminated CONNECT response, and include exact-limit and just-over-limit boundary cases.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}