fix(run): forward text-frame keystrokes to pty and fix ws-pty upgrade dispatch
xterm.js's onData hands the browser a plain string, and WebSocket.send(string)
always emits a TEXT frame — pty-attach.js only forwarded BINARY frames to
pty.write(), so every keystroke was silently dropped as an unparseable JSON
control message. Now any non-control text frame reaches the pty.
Deeper root cause of the terminal never accepting input at all: the /ws
WebSocketServer used the {server, path} shorthand, whose own internal
upgrade listener calls handleUpgrade() for every upgrade on the shared
http.Server and aborts with 400 on a path mismatch — killing /ws-pty/*
upgrades before the PTY server's own listener ever ran. Switched /ws to
noServer + a manual path-checked dispatch, matching /ws-pty's pattern.
This commit is contained in:
@@ -78,6 +78,17 @@ describe("pty-attach", () => {
|
|||||||
assert.deepEqual(fakePty.__writes[0], { resize: [100, 40] });
|
assert.deepEqual(fakePty.__writes[0], { resize: [100, 40] });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("forwards a plain text WS frame (keystrokes) to pty.write", () => {
|
||||||
|
// xterm.js's onData hands the browser a plain string, and
|
||||||
|
// WebSocket.send(string) always emits a TEXT frame — so every keystroke
|
||||||
|
// arrives here as non-binary. This must reach the pty, not be dropped as
|
||||||
|
// an unparseable control message.
|
||||||
|
const ws = makeFakeWs();
|
||||||
|
ptyAttach.attach(ws, "ccam-lane-1", { cols: 80, rows: 24 });
|
||||||
|
ws.__emitter.emit("message", Buffer.from("ls -la\r"), { binary: false });
|
||||||
|
assert.deepEqual(fakePty.__writes[0], "ls -la\r");
|
||||||
|
});
|
||||||
|
|
||||||
it("sends an exit control message and closes on PTY exit", () => {
|
it("sends an exit control message and closes on PTY exit", () => {
|
||||||
const ws = makeFakeWs();
|
const ws = makeFakeWs();
|
||||||
let closed = false;
|
let closed = false;
|
||||||
|
|||||||
@@ -71,19 +71,31 @@ function attach(ws, runId, { cols, rows }) {
|
|||||||
// callback arg (newer) or via `data.binary` on some transports — this
|
// callback arg (newer) or via `data.binary` on some transports — this
|
||||||
// helper's own tests exercise the `{binary}` option shape used above.
|
// helper's own tests exercise the `{binary}` option shape used above.
|
||||||
const binary = typeof isBinary === "boolean" ? isBinary : !!(isBinary && isBinary.binary);
|
const binary = typeof isBinary === "boolean" ? isBinary : !!(isBinary && isBinary.binary);
|
||||||
|
const text = data.toString("utf8");
|
||||||
if (binary) {
|
if (binary) {
|
||||||
pty.write(data.toString("utf8"));
|
pty.write(text);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// The browser's WebSocket API sends a JS string as a text frame, and
|
||||||
|
// xterm.js's onData callback hands over plain strings — so every
|
||||||
|
// keystroke arrives here as text, not binary. Only a JSON control frame
|
||||||
|
// (matched by this same prefix check the client uses for output) is
|
||||||
|
// NOT keystroke input; everything else must reach the pty or typing
|
||||||
|
// does nothing.
|
||||||
|
if (text.startsWith('{"type"')) {
|
||||||
let msg;
|
let msg;
|
||||||
try {
|
try {
|
||||||
msg = JSON.parse(data.toString("utf8"));
|
msg = JSON.parse(text);
|
||||||
} catch {
|
} catch {
|
||||||
|
pty.write(text);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (msg && msg.type === "resize" && Number.isFinite(msg.cols) && Number.isFinite(msg.rows)) {
|
if (msg && msg.type === "resize" && Number.isFinite(msg.cols) && Number.isFinite(msg.rows)) {
|
||||||
pty.resize(msg.cols, msg.rows);
|
pty.resize(msg.cols, msg.rows);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
pty.write(text);
|
||||||
});
|
});
|
||||||
|
|
||||||
ws.on("close", () => {
|
ws.on("close", () => {
|
||||||
|
|||||||
+16
-4
@@ -9,11 +9,15 @@ const { isHostAllowed, isWebSocketAuthorized } = require("./lib/security");
|
|||||||
let wss = null;
|
let wss = null;
|
||||||
|
|
||||||
function initWebSocket(server) {
|
function initWebSocket(server) {
|
||||||
// Express middleware doesn't run on WS upgrades, so enforce the same Host
|
// `noServer: true` + a manual, path-checked `server.on("upgrade", ...)`
|
||||||
// allowlist (anti DNS-rebinding) and optional token here (GHSA-gr74-4xfh-6jw9).
|
// rather than the `{server, path}` shorthand: that shorthand's own
|
||||||
|
// internal upgrade listener calls `handleUpgrade` for EVERY upgrade on the
|
||||||
|
// shared http.Server (path filtering happens inside `handleUpgrade`,
|
||||||
|
// which `abortHandshake`s with 400 on a mismatch) — so it was answering,
|
||||||
|
// and killing, `/ws-pty/*` upgrades before the PTY server's own listener
|
||||||
|
// (registered below by `initPtyWebSocket`) ever got a chance to run.
|
||||||
wss = new WebSocketServer({
|
wss = new WebSocketServer({
|
||||||
server,
|
noServer: true,
|
||||||
path: "/ws",
|
|
||||||
maxPayload: 64 * 1024,
|
maxPayload: 64 * 1024,
|
||||||
verifyClient(info, done) {
|
verifyClient(info, done) {
|
||||||
if (!isHostAllowed(info.req.headers.host)) return done(false, 403, "host not allowed");
|
if (!isHostAllowed(info.req.headers.host)) return done(false, 403, "host not allowed");
|
||||||
@@ -22,6 +26,14 @@ function initWebSocket(server) {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
server.on("upgrade", (req, socket, head) => {
|
||||||
|
const url = new URL(req.url, "http://localhost");
|
||||||
|
if (url.pathname !== "/ws") return; // not ours — `/ws-pty/*` handles its own path.
|
||||||
|
wss.handleUpgrade(req, socket, head, (ws) => {
|
||||||
|
wss.emit("connection", ws, req);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
wss.on("connection", (ws) => {
|
wss.on("connection", (ws) => {
|
||||||
ws.isAlive = true;
|
ws.isAlive = true;
|
||||||
ws.on("pong", () => {
|
ws.on("pong", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user