Skip to content

Commit c72b2e2

Browse files
xerialclaude
andcommitted
feature: Harden WebSocket handshake (version check) + Node backpressure
- Add Sec-WebSocket-Version validation via a shared WebSocketHandshake.validate: missing key -> 400; version absent -> 400; version present but != 13 -> 426 (advertising Sec-WebSocket-Version: 13). Used by both manual backends (Native, Node), dedup'ing the key check too. - Node: handle write backpressure — pause the socket when socket.write() returns false, resume on 'drain' — so a slow consumer can't make us buffer unbounded. - Node writeHttpClose now copies the response's own headers (so the 426's Sec-WebSocket-Version reaches the client; Native's serialize already did). - Tests: Native + Node "unsupported version -> 426". JVM 1635 / Native 1388 / JS 1387 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e0d70d2 commit c72b2e2

6 files changed

Lines changed: 150 additions & 19 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# WebSocket hardening (Sec-WebSocket-Version + Node backpressure)
2+
3+
## Context
4+
Follow-up hardening for the WebSocket servers (Native #576, Node #577). Two gaps noted at merge:
5+
1. Neither manual backend validated `Sec-WebSocket-Version` (RFC 6455 says reply `426` advertising the
6+
supported version on a mismatch).
7+
2. Node ignored `socket.write()` backpressure (could buffer unbounded for a slow consumer).
8+
9+
## Changes
10+
- **`WebSocketHandshake` (shared)**`validate(request): Either[Response, String]`: missing/empty
11+
`Sec-WebSocket-Key` → 400; `Sec-WebSocket-Version` absent → 400, present-but-not-13 → 426 (with
12+
`Sec-WebSocket-Version: 13`); otherwise `Right(key)`. Dedups the key check and adds the version
13+
check across both manual backends. Native (`NativeHttpServer.handleWebSocketUpgrade`) and Node
14+
(`NodeWebSocket.handleUpgrade`) now call it.
15+
- **Node backpressure**`NodeWebSocketContext.writeFrame` pauses the socket when `write()` returns
16+
false; `accept` wires `'drain'``socket.resume()`. Bounds memory under a slow consumer.
17+
- **Node `writeHttpClose`** now copies the response's own headers (so the 426's `Sec-WebSocket-Version`
18+
reaches the client; Native's `serialize` already did).
19+
20+
## Verification
21+
`scalafmt`; full JVM/JS/Native suites + Netty compile. New tests: Native + Node "unsupported version
22+
→ 426 (advertising 13)". JVM 1635 / Native 1388 / JS 1387 pass.
23+
24+
## Follow-ups (remaining)
25+
Portable Native idle/read timeout; Native libcurl client bug; cross-platform WebSocket client;
26+
permessage-deflate.

uni/.js/src/main/scala/wvlet/uni/http/NodeWebSocket.scala

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ private[http] object NodeWebSocket extends LogSupport:
4747
case None =>
4848
destroyQuietly(socket)
4949
case Some(route) =>
50-
request.header(HttpHeader.SecWebSocketKey).filter(_.nonEmpty) match
51-
case None =>
52-
// A WebSocket upgrade without a Sec-WebSocket-Key is malformed (RFC 6455 §4.2.1).
53-
writeHttpClose(socket, Response.badRequest("Missing Sec-WebSocket-Key"))
54-
case Some(key) =>
50+
WebSocketHandshake.validate(request) match
51+
case Left(rejection) =>
52+
// Malformed handshake: missing key (400), or unsupported Sec-WebSocket-Version (426).
53+
writeHttpClose(socket, rejection)
54+
case Right(key) =>
5555
gateAndAccept(socket, head, key, route, request, maxFrameSize)
5656
catch
5757
case NonFatal(e) =>
@@ -147,8 +147,17 @@ private[http] object NodeWebSocket extends LogSupport:
147147
val onData: js.Function1[js.Dynamic, Unit] =
148148
(chunk: js.Dynamic) => drive(NodeBytes.toBytes(chunk))
149149
val onClose: js.Function1[js.Dynamic, Unit] = (_: js.Dynamic) => notifyClose()
150+
// Resume reading once a back-pressured write has drained (see NodeWebSocketContext.writeFrame).
151+
val onDrain: js.Function0[Unit] =
152+
() =>
153+
try
154+
socket.applyDynamic("resume")()
155+
catch
156+
case NonFatal(_) =>
157+
()
150158
socket.applyDynamic("on")("close", onClose)
151159
socket.applyDynamic("on")("error", onClose)
160+
socket.applyDynamic("on")("drain", onDrain)
152161

153162
try
154163
handler.onOpen(ctx)
@@ -213,6 +222,17 @@ private[http] object NodeWebSocket extends LogSupport:
213222
.append(" ")
214223
.append(response.status.reason)
215224
.append("\r\n")
225+
// Copy the response's own headers (e.g. Sec-WebSocket-Version on a 426), minus the ones set
226+
// explicitly below.
227+
response
228+
.headers
229+
.entries
230+
.foreach { case (name, value) =>
231+
if !name.equalsIgnoreCase(HttpHeader.Connection) &&
232+
!name.equalsIgnoreCase(HttpHeader.ContentLength)
233+
then
234+
sb.append(name).append(": ").append(value).append("\r\n")
235+
}
216236
sb.append(HttpHeader.Connection).append(": close\r\n")
217237
sb.append(HttpHeader.ContentLength).append(": ").append(body.length).append("\r\n\r\n")
218238
val head = sb.toString.getBytes(StandardCharsets.ISO_8859_1)
@@ -279,9 +299,14 @@ private[http] class NodeWebSocketContext(socket: js.Dynamic, override val reques
279299

280300
private def writeFrame(opcode: Int, payload: Array[Byte]): Unit =
281301
try
282-
socket.applyDynamic("write")(
283-
NodeBytes.toUint8Array(WebSocketFrame.encodeFrame(opcode, payload))
284-
)
302+
val accepted =
303+
socket.applyDynamic("write")(
304+
NodeBytes.toUint8Array(WebSocketFrame.encodeFrame(opcode, payload))
305+
)
306+
// Backpressure: write() returns false when the kernel send buffer is full. Pause reading so a
307+
// slow consumer can't make us buffer unbounded; the 'drain' listener (wired in accept) resumes.
308+
if !accepted.asInstanceOf[Boolean] then
309+
socket.applyDynamic("pause")()
285310
catch
286311
case NonFatal(e) =>
287312
debug(s"WebSocket write failed: ${e.getMessage}")

uni/.js/src/test/scala/wvlet/uni/http/NodeWebSocketTest.scala

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,13 @@ class NodeWebSocketTest extends UniTest:
6363
socket.applyDynamic("on")("error", onError)
6464

6565
/** Send the upgrade request; resolves with the parsed handshake response. */
66-
def connect(path: String): Future[Handshake] =
66+
def connect(path: String, version: String = "13"): Future[Handshake] =
6767
val onConnect: js.Function0[Unit] =
6868
() =>
6969
val req =
7070
s"GET ${path} HTTP/1.1\r\nHost: localhost\r\n" +
7171
s"Upgrade: websocket\r\nConnection: Upgrade\r\n" +
72-
s"Sec-WebSocket-Key: ${key}\r\nSec-WebSocket-Version: 13\r\n\r\n"
72+
s"Sec-WebSocket-Key: ${key}\r\nSec-WebSocket-Version: ${version}\r\n\r\n"
7373
socket.applyDynamic("write")(
7474
NodeBytes.toUint8Array(req.getBytes(StandardCharsets.ISO_8859_1))
7575
)
@@ -313,6 +313,23 @@ class NodeWebSocketTest extends UniTest:
313313
}
314314
}
315315

316+
test("WebSocket upgrade with an unsupported version is rejected with 426") {
317+
NodeServer
318+
.withPort(0)
319+
.withWebSocketRoute("/ws") { _ =>
320+
new WebSocketHandler {}
321+
}
322+
.startAndAwait { server =>
323+
val client = NodeWsClient(server.localPort)
324+
Rx.future(client.connect("/ws", version = "8"))
325+
.map { hs =>
326+
client.close()
327+
hs.status shouldBe 426
328+
hs.headers.get("sec-websocket-version") shouldBe Some("13")
329+
}
330+
}
331+
}
332+
316333
test("WebSocket upgrade can be rejected by a filter") {
317334
val deny = RxHttpFilter { (_, _) =>
318335
Rx.single(Response.forbidden)

uni/.native/src/main/scala/wvlet/uni/http/NativeHttpServer.scala

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -263,18 +263,14 @@ class NativeHttpServer(config: NativeServerConfig) extends HttpServer with LogSu
263263
* worker for the WebSocket's lifetime).
264264
*/
265265
private def handleWebSocketUpgrade(clientFd: Int, request: Request, route: WebSocketRoute): Unit =
266-
request.header(HttpHeader.SecWebSocketKey).filter(_.nonEmpty) match
267-
case None =>
268-
// A WebSocket upgrade without a Sec-WebSocket-Key is a malformed handshake (RFC 6455 §4.2.1).
266+
WebSocketHandshake.validate(request) match
267+
case Left(rejection) =>
268+
// Malformed handshake: missing key (400), or unsupported Sec-WebSocket-Version (426).
269269
NativeSocket.sendAll(
270270
clientFd,
271-
HttpResponseWriter.serialize(
272-
Response.badRequest("Missing Sec-WebSocket-Key"),
273-
keepAlive = false,
274-
includeBody = true
275-
)
271+
HttpResponseWriter.serialize(rejection, keepAlive = false, includeBody = true)
276272
)
277-
case Some(key) =>
273+
case Right(key) =>
278274
// Capture the request as threaded through the filter, so attributes a filter adds during
279275
// the handshake reach the WebSocketContext (matching the Netty backend).
280276
val upgradeRequest = AtomicReference[Request](request)

uni/.native/src/test/scala/wvlet/uni/http/NativeServerTest.scala

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,23 @@ class NativeServerTest extends UniTest:
443443
}
444444
}
445445

446+
test("WebSocket upgrade with an unsupported version is rejected with 426") {
447+
NativeServer
448+
.withPort(0)
449+
.withWebSocketRoute("/ws") { _ =>
450+
new WebSocketHandler {}
451+
}
452+
.start { server =>
453+
val resp = request(
454+
server.localPort,
455+
"GET /ws HTTP/1.1\r\nHost: localhost\r\nUpgrade: websocket\r\nConnection: Upgrade\r\n" +
456+
"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\nSec-WebSocket-Version: 8\r\n\r\n"
457+
)
458+
resp.status shouldBe 426
459+
resp.headers.get("sec-websocket-version") shouldBe Some("13")
460+
}
461+
}
462+
446463
test("WebSocket upgrade can be rejected by a filter") {
447464
val deny = RxHttpFilter { (_, _) =>
448465
wvlet.uni.rx.Rx.single(Response.forbidden)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package wvlet.uni.http
15+
16+
/**
17+
* Shared validation of a WebSocket upgrade request, used by the backends that perform the
18+
* handshake by hand (Native, Node.js). Centralizes the `Sec-WebSocket-Key` and
19+
* `Sec-WebSocket-Version` checks so both backends reject malformed handshakes identically.
20+
*/
21+
private[http] object WebSocketHandshake:
22+
23+
/** The only WebSocket protocol version this server speaks (RFC 6455). */
24+
final val SupportedVersion = "13"
25+
26+
/**
27+
* Validate the upgrade headers. `Right(key)` carries the `Sec-WebSocket-Key` when the handshake
28+
* is acceptable; `Left(response)` is the rejection to send:
29+
* - 400 if `Sec-WebSocket-Key` is missing/empty or `Sec-WebSocket-Version` is absent,
30+
* - 426 (advertising `Sec-WebSocket-Version: 13`) if the requested version is not 13.
31+
*/
32+
def validate(request: Request): Either[Response, String] =
33+
request.header(HttpHeader.SecWebSocketKey).filter(_.nonEmpty) match
34+
case None =>
35+
Left(Response.badRequest("Missing Sec-WebSocket-Key"))
36+
case Some(key) =>
37+
request.header(HttpHeader.SecWebSocketVersion).map(_.trim) match
38+
case Some(SupportedVersion) =>
39+
Right(key)
40+
case Some(_) =>
41+
// Unsupported version: tell the client which version we speak (RFC 6455 §4.2.2 / 4.4).
42+
Left(
43+
Response(HttpStatus.UpgradeRequired_426)
44+
.addHeader(HttpHeader.SecWebSocketVersion, SupportedVersion)
45+
.withTextContent("Unsupported Sec-WebSocket-Version")
46+
)
47+
case None =>
48+
Left(Response.badRequest("Missing Sec-WebSocket-Version"))
49+
50+
end WebSocketHandshake

0 commit comments

Comments
 (0)