fix(websocket): mask outgoing CLOSE frames in client mode
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LLwcyHUnbApCrY33gvgoa
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
787ae610d4
commit
901da954a9
@@ -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);
|
* 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
|
* each write goes directly to the kernel send buffer, and TCP_NODELAY ensures
|
||||||
* it's transmitted immediately.
|
* it's transmitted immediately.
|
||||||
*/
|
*/
|
||||||
public void close(int code) throws IOException {
|
public void close(int code) throws IOException {
|
||||||
if (!open.compareAndSet(true, false)) return;
|
if (!open.compareAndSet(true, false)) return;
|
||||||
writeLock.lock();
|
byte[] payload = {(byte) ((code >> 8) & 0xFF), (byte) (code & 0xFF)};
|
||||||
try {
|
writeFrame(WebSocketFrame.OP_CLOSE, payload, 0, 2);
|
||||||
out.write(0x88);
|
|
||||||
out.write(0x02);
|
|
||||||
out.write((code >> 8) & 0xFF);
|
|
||||||
out.write(code & 0xFF);
|
|
||||||
} finally {
|
|
||||||
writeLock.unlock();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Session loop internals ─────────────────────────────────────────────
|
// ── Session loop internals ─────────────────────────────────────────────
|
||||||
|
|||||||
@@ -40,4 +40,26 @@ class WebSocketSessionFrameTest {
|
|||||||
|
|
||||||
assertThrows(Exception.class, () -> session.readFrame(frame));
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -90,6 +90,26 @@ class WebSocketSessionTest {
|
|||||||
assertEquals((byte) 0xE8, bytes[3]);
|
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
|
@Test
|
||||||
void closeFromPeer_extractsCloseCode() {
|
void closeFromPeer_extractsCloseCode() {
|
||||||
WebSocketSession session = new WebSocketSession(new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream(), 64);
|
WebSocketSession session = new WebSocketSession(new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream(), 64);
|
||||||
|
|||||||
Reference in New Issue
Block a user