fix(rdp): TCP stream buffering + error surfacing for guacd pipeline
Three bugs fixed: 1. TCP stream fragmentation — guacd→browser data pipe treated each TCP chunk as a complete instruction. TCP is a stream protocol; instructions WILL be split across chunks (especially display/image data). Added instruction buffer that accumulates data and only forwards complete instructions (terminated by ';'). 2. Missing client.onerror — when guacd fails the RDP connection (NLA, auth, TLS), it sends a Guacamole error instruction. No handler was registered, so errors were silently swallowed. User saw blank canvas with no feedback. Now surfaces errors via console and gateway callback. 3. Missing client.onstatechange — no connection state tracking. Added state transition logging for diagnostics. Also improved CONNECT handshake logging to surface connection parameters (host, port, user, domain, security mode) without exposing passwords. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
36251c3caa
commit
34bea52e0b
@ -66,7 +66,10 @@ export class GuacamoleService {
|
||||
|
||||
// Phase 2: CONNECT — send values in the exact order guacd expects
|
||||
const connectInstruction = this.buildConnectInstruction(params, argNames);
|
||||
this.logger.debug(`Sending connect instruction with ${argNames.length} values`);
|
||||
this.logger.log(
|
||||
`Sending CONNECT: host=${params.hostname}:${params.port} user=${params.username} domain=${params.domain || '(none)'} ` +
|
||||
`security=${params.security || 'any'} size=${params.width}x${params.height} ignoreCert=${params.ignoreCert !== false}`,
|
||||
);
|
||||
socket.write(connectInstruction);
|
||||
|
||||
resolve(socket);
|
||||
|
||||
@ -89,18 +89,32 @@ export class RdpGateway {
|
||||
|
||||
this.clientSockets.set(client, socket);
|
||||
|
||||
// Pipe guacd → browser: wrap raw Guacamole instruction bytes in JSON envelope
|
||||
// Pipe guacd → browser: buffer TCP stream into complete Guacamole instructions.
|
||||
// TCP is a stream protocol — data events can contain partial instructions,
|
||||
// multiple instructions, or any combination. We must only forward complete
|
||||
// instructions (terminated by ';') to the browser.
|
||||
let guacMsgCount = 0;
|
||||
let tcpBuffer = '';
|
||||
socket.on('data', (data: Buffer) => {
|
||||
const instruction = data.toString('utf-8');
|
||||
tcpBuffer += data.toString('utf-8');
|
||||
|
||||
// Extract all complete instructions from the buffer
|
||||
let semicolonIdx: number;
|
||||
while ((semicolonIdx = tcpBuffer.indexOf(';')) !== -1) {
|
||||
const instruction = tcpBuffer.substring(0, semicolonIdx + 1);
|
||||
tcpBuffer = tcpBuffer.substring(semicolonIdx + 1);
|
||||
|
||||
guacMsgCount++;
|
||||
// Log first 5 instructions and any errors
|
||||
if (guacMsgCount <= 5 || instruction.includes('error')) {
|
||||
this.logger.log(`[guacd→browser #${guacMsgCount}] ${instruction.substring(0, 300)}`);
|
||||
// Log first 10 instructions and any errors for diagnostics
|
||||
if (guacMsgCount <= 10 || instruction.includes('error')) {
|
||||
this.logger.log(
|
||||
`[guacd→browser #${guacMsgCount}] ${instruction.substring(0, 300)}`,
|
||||
);
|
||||
}
|
||||
if (client.readyState === 1 /* WebSocket.OPEN */) {
|
||||
client.send(JSON.stringify({ type: 'guac', instruction }));
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
socket.on('close', () => {
|
||||
|
||||
@ -168,6 +168,28 @@ export function useRdp() {
|
||||
console.error('[RDP] Gateway error:', message)
|
||||
}
|
||||
|
||||
// Handle Guacamole-level errors (NLA failure, auth failure, etc.)
|
||||
client.onerror = (status: Guacamole.Status) => {
|
||||
const code = status.code
|
||||
const msg = status.message || 'Unknown error'
|
||||
console.error(`[RDP] Guacamole error: code=${code} message=${msg}`)
|
||||
// Surface error via gateway error callback so UI can display it
|
||||
wrapper.onGatewayError?.(`RDP connection failed: ${msg}`)
|
||||
}
|
||||
|
||||
// Track connection state transitions for diagnostics
|
||||
client.onstatechange = (state: number) => {
|
||||
const states: Record<number, string> = {
|
||||
0: 'IDLE',
|
||||
1: 'CONNECTING',
|
||||
2: 'WAITING',
|
||||
3: 'CONNECTED',
|
||||
4: 'DISCONNECTING',
|
||||
5: 'DISCONNECTED',
|
||||
}
|
||||
console.log(`[RDP] State: ${states[state] || state}`)
|
||||
}
|
||||
|
||||
// Attach Guacamole display element to container
|
||||
const displayEl = client.getDisplay().getElement()
|
||||
container.appendChild(displayEl)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user