From 901da954a974cc9de74ff47a38846dcd95cf6fe8 Mon Sep 17 00:00:00 2001 From: Zakaria El Orche Date: Fri, 14 Aug 2026 22:46:59 +0000 Subject: [PATCH] fix(websocket): mask outgoing CLOSE frames in client mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebSocketSession.close(int) hand-wrote a raw, always-unmasked 4-byte CLOSE frame, bypassing writeFrame's maskOutgoing handling that sendText/send/ sendPong already go through correctly. A client-mode session (maskOutgoing true — WS-client usage, e.g. Pathway's UpstreamWebSocketConnector relaying a proxied client's close to a backend) therefore sent an RFC-6455-invalid unmasked frame. This was latent until this same HTTP/2 branch's readFrame rewrite added the receive-side masking check RFC 6455 §5.1 requires: a strict peer now rejects the malformed frame with WebSocketProtocolException("client frame must be masked") before ever exposing it as a CLOSE, silently dropping the close instead of relaying it — reproduced end to end via Pathway's ProxyWebSocketIntegrationTest.clientCloseIsForwardedToTheBackend. close(int) now builds its 2-byte payload and calls the same writeFrame path every other outgoing frame uses, so masking (or not) follows maskOutgoing automatically. Existing server-mode close_setsClosedAndWritesFrame is unchanged (byte-for-byte identical output — no mask bit, no key). Added close_masksWhenActingAsClient (mirrors the existing sendText coverage) and a round-trip regression, readFrame_acceptsCloseFrameWrittenByAClientSession, that reproduces the actual bug: a client session's close() output fed straight into a server session's readFrame(). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011LLwcyHUnbApCrY33gvgoa --- .../flash/websocket/WebSocketSession.java | 18 +++++++-------- .../websocket/WebSocketSessionFrameTest.java | 22 +++++++++++++++++++ .../flash/websocket/WebSocketSessionTest.java | 20 +++++++++++++++++ 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/flash/src/main/java/dev/relism/flash/websocket/WebSocketSession.java b/flash/src/main/java/dev/relism/flash/websocket/WebSocketSession.java index dcf663d..6ab580a 100644 --- a/flash/src/main/java/dev/relism/flash/websocket/WebSocketSession.java +++ b/flash/src/main/java/dev/relism/flash/websocket/WebSocketSession.java @@ -133,22 +133,20 @@ public final class WebSocketSession { } /** - * Sends a CLOSE frame exactly once. + * Sends a CLOSE frame exactly once, masked exactly like any other outgoing frame when this + * session is in client mode ({@link #maskOutgoing}) — via {@link #writeFrame}, the same path + * {@link #sendText}/{@link #send}/{@link #sendPong} use, rather than hand-writing the 4 bytes + * unmasked regardless of role. A client-mode session that skipped masking here was sending an + * RFC-6455-invalid CLOSE frame that a strict peer's {@link #readFrame} correctly rejects (the + * masking check {@link #readFrame} itself enforces on the receive side). * No flush needed — {@code out} is the raw socket OutputStream (not buffered); * each write goes directly to the kernel send buffer, and TCP_NODELAY ensures * it's transmitted immediately. */ public void close(int code) throws IOException { if (!open.compareAndSet(true, false)) return; - writeLock.lock(); - try { - out.write(0x88); - out.write(0x02); - out.write((code >> 8) & 0xFF); - out.write(code & 0xFF); - } finally { - writeLock.unlock(); - } + byte[] payload = {(byte) ((code >> 8) & 0xFF), (byte) (code & 0xFF)}; + writeFrame(WebSocketFrame.OP_CLOSE, payload, 0, 2); } // ── Session loop internals ───────────────────────────────────────────── diff --git a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionFrameTest.java b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionFrameTest.java index 0b7dfaf..7a439a4 100644 --- a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionFrameTest.java +++ b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionFrameTest.java @@ -40,4 +40,26 @@ class WebSocketSessionFrameTest { assertThrows(Exception.class, () -> session.readFrame(frame)); } + + /** + * Regression for the CLOSE frame Pathway's {@code UpstreamWebSocketConnector} (a client-mode + * session, {@code maskOutgoing=true}) sends when relaying the client's own CLOSE to a backend: + * before this fix, {@link WebSocketSession#close} never masked, so a server-mode peer's own + * {@link WebSocketSession#readFrame} (which enforces masking per RFC 6455 §5.1) rejected it + * with a {@link WebSocketProtocolException}, and the close was silently lost. + */ + @Test + void readFrame_acceptsCloseFrameWrittenByAClientSession() throws Exception { + ByteArrayOutputStream wire = new ByteArrayOutputStream(); + WebSocketSession client = new WebSocketSession(new ByteArrayInputStream(new byte[0]), wire, 64, null, true); + client.close(1000); + + WebSocketSession server = new WebSocketSession(new ByteArrayInputStream(wire.toByteArray()), new ByteArrayOutputStream(), 64); + WebSocketFrame frame = new WebSocketFrame(); + + assertTrue(server.readFrame(frame)); + assertEquals(WebSocketFrame.OP_CLOSE, frame.opcode()); + server.closeFromPeer(frame); + assertEquals(1000, server.closeCode()); + } } diff --git a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java index e1d0b9c..ca6767c 100644 --- a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java +++ b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java @@ -90,6 +90,26 @@ class WebSocketSessionTest { assertEquals((byte) 0xE8, bytes[3]); } + @Test + void close_masksWhenActingAsClient() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + WebSocketSession session = new WebSocketSession( + new ByteArrayInputStream(new byte[0]), out, 64, null, true); + + session.close(1000); + + assertFalse(session.isOpen()); + assertEquals(1000, session.closeCode()); + byte[] bytes = out.toByteArray(); + assertEquals((byte) 0x88, bytes[0]); // FIN + CLOSE + assertEquals((byte) (0x80 | 2), bytes[1]); // masked bit + length 2 + // Header is opcode + length + a full 4-byte mask key (bytes 2-5) regardless of payload + // length, then the (masked) payload — mirrors sendText_masksWhenActingAsClient above. + byte m0 = bytes[2], m1 = bytes[3]; + assertEquals((byte) (0x03 ^ m0), bytes[6]); + assertEquals((byte) (0xE8 ^ m1), bytes[7]); + } + @Test void closeFromPeer_extractsCloseCode() { WebSocketSession session = new WebSocketSession(new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream(), 64);