mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-15 17:35:29 +02:00
fix(browse): sidebar Terminal — drop the duplicate WS subprotocol header, stop doubling CJK IME input
The terminal client passed the auth token as the WS subprotocol AND echoed it in a second header, which some Chromium builds reject; and composition events double-sent CJK input (each IME commit arrived once from the composition handler and once from the data handler). One auth path, one input path; also fixes the terminal-agent test that failed on clean main. Contributed by @mindsurf0176 (PR #2515). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
4def6f6c7e
commit
9265d27fa1
@@ -227,6 +227,45 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () =
|
||||
expect(resp.headers.get('sec-websocket-protocol')).toBe(`gstack-pty.${token}`);
|
||||
});
|
||||
|
||||
test('upgrade response contains exactly ONE Sec-WebSocket-Protocol header', async () => {
|
||||
// RFC 6455: the server MUST select at most one subprotocol. Bun >= 1.3
|
||||
// auto-echoes the first offered protocol in server.upgrade(), so a
|
||||
// manual echo on top of that produced TWO Sec-WebSocket-Protocol
|
||||
// headers — and strict clients (Chromium, python websockets) reject the
|
||||
// handshake, leaving the sidebar terminal permanently disconnected.
|
||||
//
|
||||
// Headers.get() normalizes duplicates away, so this test handshakes
|
||||
// over a raw socket and counts header lines in the response head.
|
||||
const token = 'dup-proto-token-must-be-at-least-seventeen-chars';
|
||||
await grantToken(token);
|
||||
|
||||
const head = await new Promise<string>((resolve, reject) => {
|
||||
const req =
|
||||
'GET /ws HTTP/1.1\r\n' +
|
||||
`Host: 127.0.0.1:${agentPort}\r\n` +
|
||||
'Connection: Upgrade\r\n' +
|
||||
'Upgrade: websocket\r\n' +
|
||||
'Sec-WebSocket-Version: 13\r\n' +
|
||||
'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n' +
|
||||
`Sec-WebSocket-Protocol: gstack-pty.${token}\r\n` +
|
||||
'Origin: chrome-extension://test-extension-id\r\n' +
|
||||
'\r\n';
|
||||
let buf = '';
|
||||
const socket = require('net').connect(agentPort, '127.0.0.1', () => socket.write(req));
|
||||
socket.setTimeout(5000, () => { socket.destroy(); reject(new Error('handshake timeout')); });
|
||||
socket.on('data', (chunk: Buffer) => {
|
||||
buf += chunk.toString('utf8');
|
||||
const end = buf.indexOf('\r\n\r\n');
|
||||
if (end !== -1) { socket.destroy(); resolve(buf.slice(0, end)); }
|
||||
});
|
||||
socket.on('error', reject);
|
||||
});
|
||||
|
||||
expect(head).toContain('101');
|
||||
const protoLines = head.split('\r\n').filter(l => l.toLowerCase().startsWith('sec-websocket-protocol:'));
|
||||
expect(protoLines).toEqual([`Sec-WebSocket-Protocol: gstack-pty.${token}`]);
|
||||
});
|
||||
|
||||
test('Sec-WebSocket-Protocol auth: rejects unknown token even with valid Origin', async () => {
|
||||
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, {
|
||||
headers: {
|
||||
|
||||
@@ -131,15 +131,18 @@ describe('Source-level guard: terminal-agent', () => {
|
||||
expect(wsHandler).toContain('validTokens.has');
|
||||
});
|
||||
|
||||
test('Sec-WebSocket-Protocol auth: strips gstack-pty. prefix and echoes back', () => {
|
||||
test('Sec-WebSocket-Protocol auth: strips gstack-pty. prefix, no manual echo', () => {
|
||||
const wsHandler = AGENT_SRC.slice(AGENT_SRC.indexOf("if (url.pathname === '/ws')"));
|
||||
// Browsers send `Sec-WebSocket-Protocol: gstack-pty.<token>`. The agent
|
||||
// must strip the prefix before checking validTokens, AND echo the
|
||||
// protocol back in the upgrade response — without the echo, the
|
||||
// browser closes the connection immediately.
|
||||
// must strip the prefix before checking validTokens. The protocol echo
|
||||
// is Bun's job: Bun >= 1.3 auto-echoes the first offered protocol in the
|
||||
// 101 response. A manual echo on top produced a DUPLICATE
|
||||
// Sec-WebSocket-Protocol header, which strict clients (Chromium, python
|
||||
// websockets) reject per RFC 6455 — the sidebar terminal could never
|
||||
// connect. Pin the invariant: no manual echo in the upgrade call.
|
||||
expect(wsHandler).toContain("'gstack-pty.'");
|
||||
expect(wsHandler).toContain('Sec-WebSocket-Protocol');
|
||||
expect(wsHandler).toContain('acceptedProtocol');
|
||||
expect(wsHandler).toContain('sec-websocket-protocol');
|
||||
expect(wsHandler).not.toContain("headers: { 'Sec-WebSocket-Protocol'");
|
||||
});
|
||||
|
||||
test('lazy spawn: claude PTY is spawned in message handler, not on upgrade', () => {
|
||||
@@ -155,6 +158,8 @@ describe('Source-level guard: terminal-agent', () => {
|
||||
expect(upgradeBlock).not.toContain('spawnClaude(');
|
||||
expect(upgradeBlock).not.toContain('maybeSpawnPty(');
|
||||
// Spawn must be invoked from the message handler (lazy on first byte).
|
||||
// v1.44 routes both spawn triggers (explicit {type:"start"} text frame
|
||||
// and the lazy binary-frame path) through the maybeSpawnPty helper.
|
||||
const messageHandler = AGENT_SRC.slice(AGENT_SRC.indexOf('message(ws, raw)'));
|
||||
expect(messageHandler).toContain('maybeSpawnPty(');
|
||||
expect(messageHandler).toContain('!session.spawned');
|
||||
|
||||
Reference in New Issue
Block a user