fix: resolve 14 memory leaks and bugs across native host and extension
CI / Build & Test (push) Successful in 2m47s
CI / Build & Test (push) Successful in 2m47s
Critical: - ssh-manager: release ports/wsServer/holesail on ptySpawn failure - rdp-manager: add 30s timeout to holesailInst.ready() to prevent infinite hang High: - ssh-manager: cancel password-watch timers on PTY exit/error - ssh-manager: cap outputSoFar to 4096 chars to prevent unbounded growth - connect-proxy: add 64KB header buffer cap to prevent OOM Medium: - startup: assign tunnelsRestoredPromise before resolve() to fix race - https-proxy: register upstream error handler before connect callback - connect-proxy: register upstreamSocket error handler before connect callback - https-proxy: destroy socket on backend stream error (was sending truncated 200) - logs: debounce broadcastLogs to prevent IPC storm on every log call Low: - rdp-manager: cap VNC outputBuffer entry count (not just bytes) - https-proxy: track connections in FakeHttpServer.connections Set instead of private _connections API - tab-lifecycle: clean up subscribedTabs/dashboardTabs on tab navigation - dashboard/events: guard setupEvents() against duplicate listener registration
This commit is contained in:
@@ -21,8 +21,17 @@ function debugLog(...args) {
|
|||||||
log('[debug]', ...args);
|
log('[debug]', ...args);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let _broadcastPending = false;
|
||||||
function broadcastLogs() {
|
function broadcastLogs() {
|
||||||
|
if (_broadcastPending) return;
|
||||||
|
_broadcastPending = true;
|
||||||
|
// Batch rapid log calls into a single IPC message to avoid sending a full
|
||||||
|
// copy of the 500-entry array on every individual log() call.
|
||||||
|
setTimeout(() => {
|
||||||
|
_broadcastPending = false;
|
||||||
|
const snapshot = logs.slice(0);
|
||||||
for (const tabId of dashboardTabs) {
|
for (const tabId of dashboardTabs) {
|
||||||
browser.tabs.sendMessage(tabId, { type: 'holesail-logs', logs: logs.slice(0) }).catch(() => {});
|
browser.tabs.sendMessage(tabId, { type: 'holesail-logs', logs: snapshot }).catch(() => {});
|
||||||
}
|
}
|
||||||
|
}, 100);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,3 +20,13 @@ browser.tabs.onRemoved.addListener((tabId) => {
|
|||||||
subscribedTabs.delete(tabId);
|
subscribedTabs.delete(tabId);
|
||||||
dashboardTabs.delete(tabId);
|
dashboardTabs.delete(tabId);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Clean up subscription Sets when a tab navigates away. Without this, a tab
|
||||||
|
// that subscribes and then navigates to a non-extension page stays in the Sets
|
||||||
|
// until it is closed, causing silent sendMessage failures on every broadcast.
|
||||||
|
browser.tabs.onUpdated.addListener((tabId, changeInfo) => {
|
||||||
|
if (changeInfo.url) {
|
||||||
|
subscribedTabs.delete(tabId);
|
||||||
|
dashboardTabs.delete(tabId);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -2,6 +2,12 @@
|
|||||||
// Depends on: all page modules
|
// Depends on: all page modules
|
||||||
|
|
||||||
function setupEvents() {
|
function setupEvents() {
|
||||||
|
// Guard against duplicate listener registration if setupEvents() is called
|
||||||
|
// more than once (e.g. after a hot-reload). Without this, anonymous listeners
|
||||||
|
// accumulate and each toggle fires N times per click after N calls.
|
||||||
|
if (document._holesailEventsSetup) return;
|
||||||
|
document._holesailEventsSetup = true;
|
||||||
|
|
||||||
// Toggle switches
|
// Toggle switches
|
||||||
document.querySelectorAll('.toggle').forEach(toggle => {
|
document.querySelectorAll('.toggle').forEach(toggle => {
|
||||||
toggle.addEventListener('click', () => toggle.classList.toggle('active'));
|
toggle.addEventListener('click', () => toggle.classList.toggle('active'));
|
||||||
|
|||||||
@@ -93,18 +93,9 @@ function start(port, upstreamPort, callback) {
|
|||||||
clientSocket.destroy(err);
|
clientSocket.destroy(err);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
upstreamSocket = tcp.connect(upstream, UPSTREAM_HOST, (err) => {
|
// Register error handler synchronously before the connect callback can
|
||||||
if (err) {
|
// fire, so an immediate ECONNREFUSED is never an unhandled error event.
|
||||||
debugLog('upstream connect failed:', err.message, 'upstream=', UPSTREAM_HOST + ':' + upstream);
|
upstreamSocket = tcp.connect(upstream, UPSTREAM_HOST);
|
||||||
clientSocket.destroy(err);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
debugLog('tunnel established connectTarget=', connectTarget, 'upstream=', UPSTREAM_HOST + ':' + upstream);
|
|
||||||
flushPending();
|
|
||||||
clientSocket.removeAllListeners('data');
|
|
||||||
clientSocket.pipe(upstreamSocket);
|
|
||||||
upstreamSocket.pipe(clientSocket);
|
|
||||||
});
|
|
||||||
upstreamSocket.on('error', (err) => {
|
upstreamSocket.on('error', (err) => {
|
||||||
debugLog('upstream socket error:', err.message);
|
debugLog('upstream socket error:', err.message);
|
||||||
clientSocket.destroy(err);
|
clientSocket.destroy(err);
|
||||||
@@ -112,6 +103,13 @@ function start(port, upstreamPort, callback) {
|
|||||||
clientSocket.on('error', () => {
|
clientSocket.on('error', () => {
|
||||||
if (upstreamSocket) upstreamSocket.destroy();
|
if (upstreamSocket) upstreamSocket.destroy();
|
||||||
});
|
});
|
||||||
|
upstreamSocket.on('connect', () => {
|
||||||
|
debugLog('tunnel established connectTarget=', connectTarget, 'upstream=', UPSTREAM_HOST + ':' + upstream);
|
||||||
|
flushPending();
|
||||||
|
clientSocket.removeAllListeners('data');
|
||||||
|
clientSocket.pipe(upstreamSocket);
|
||||||
|
upstreamSocket.pipe(clientSocket);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -122,6 +120,12 @@ function start(port, upstreamPort, callback) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
buffer = Buffer.concat([buffer, chunk]);
|
buffer = Buffer.concat([buffer, chunk]);
|
||||||
|
// Reject oversized headers to prevent OOM from malicious/buggy clients.
|
||||||
|
if (buffer.length > 65536) {
|
||||||
|
debugLog('header too large (' + buffer.length + ' bytes) — destroying client socket');
|
||||||
|
clientSocket.destroy();
|
||||||
|
return;
|
||||||
|
}
|
||||||
tryTunnel();
|
tryTunnel();
|
||||||
});
|
});
|
||||||
clientSocket.on('error', (err) => {
|
clientSocket.on('error', (err) => {
|
||||||
|
|||||||
@@ -92,9 +92,11 @@ function initStartup(holesailManager, certificateAuthority, httpsProxy, connectP
|
|||||||
}
|
}
|
||||||
|
|
||||||
log('Proxies startup complete: HTTPS', savedProxyPort, 'CONNECT', connectProxy.getPort() ?? 'FAILED');
|
log('Proxies startup complete: HTTPS', savedProxyPort, 'CONNECT', connectProxy.getPort() ?? 'FAILED');
|
||||||
resolve();
|
// Assign tunnelsRestoredPromise BEFORE resolving proxiesReadyPromise so
|
||||||
|
// that any getState call awaiting proxiesReadyPromise immediately sees the
|
||||||
|
// non-null promise rather than the one-microtask window where it is null.
|
||||||
tunnelsRestoredPromise = restorePersistedTunnels(holesailManager, restored).catch((e) => log('Restore tunnels failed:', e.message));
|
tunnelsRestoredPromise = restorePersistedTunnels(holesailManager, restored).catch((e) => log('Restore tunnels failed:', e.message));
|
||||||
|
resolve();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
+19
-10
@@ -315,7 +315,9 @@ h1{color:#c0392b}code{background:#f4f4f4;padding:2px 6px;border-radius:3px;font-
|
|||||||
}
|
}
|
||||||
proxyRes.on('data', (chunk) => !timedOut && res.write(chunk));
|
proxyRes.on('data', (chunk) => !timedOut && res.write(chunk));
|
||||||
proxyRes.on('end', () => !timedOut && !res.writableEnded && res.end());
|
proxyRes.on('end', () => !timedOut && !res.writableEnded && res.end());
|
||||||
proxyRes.on('error', () => !res.writableEnded && res.end());
|
// Destroy the client socket on a mid-stream backend error so the browser
|
||||||
|
// receives a connection reset rather than a silently truncated 200 body.
|
||||||
|
proxyRes.on('error', () => { try { if (res.socket) res.socket.destroy(); } catch (_) {} });
|
||||||
});
|
});
|
||||||
proxyReq.on('error', (err) => {
|
proxyReq.on('error', (err) => {
|
||||||
clearTimeout(timeoutId);
|
clearTimeout(timeoutId);
|
||||||
@@ -355,7 +357,12 @@ function onUpgrade (req, socket, head) {
|
|||||||
try { bareTcp = require('bare-tcp'); } catch (_) {}
|
try { bareTcp = require('bare-tcp'); } catch (_) {}
|
||||||
if (!bareTcp) { socket.write('HTTP/1.1 502 Bad Gateway\r\n\r\n'); socket.destroy(); return; }
|
if (!bareTcp) { socket.write('HTTP/1.1 502 Bad Gateway\r\n\r\n'); socket.destroy(); return; }
|
||||||
|
|
||||||
const upstream = bareTcp.connect(targetPort, targetHost, () => {
|
// Register error handler synchronously before the connect callback can fire,
|
||||||
|
// so an immediate ECONNREFUSED is never an unhandled error event.
|
||||||
|
const upstream = bareTcp.connect(targetPort, targetHost);
|
||||||
|
upstream.on('error', () => { try { socket.write('HTTP/1.1 502 Bad Gateway\r\n\r\n'); socket.destroy(); } catch (_) {} });
|
||||||
|
socket.on('error', () => { try { upstream.destroy(); } catch (_) {} });
|
||||||
|
upstream.on('connect', () => {
|
||||||
const headers = Object.entries(req.headers).map(([k, v]) => k + ': ' + v).join('\r\n');
|
const headers = Object.entries(req.headers).map(([k, v]) => k + ': ' + v).join('\r\n');
|
||||||
const requestLine = (req.method || 'GET') + ' ' + (req.url || '/') + ' HTTP/1.1\r\n';
|
const requestLine = (req.method || 'GET') + ' ' + (req.url || '/') + ' HTTP/1.1\r\n';
|
||||||
upstream.write(requestLine + headers + '\r\n\r\n');
|
upstream.write(requestLine + headers + '\r\n\r\n');
|
||||||
@@ -363,8 +370,6 @@ function onUpgrade (req, socket, head) {
|
|||||||
socket.pipe(upstream);
|
socket.pipe(upstream);
|
||||||
upstream.pipe(socket);
|
upstream.pipe(socket);
|
||||||
});
|
});
|
||||||
upstream.on('error', () => { try { socket.write('HTTP/1.1 502 Bad Gateway\r\n\r\n'); socket.destroy(); } catch (_) {} });
|
|
||||||
socket.on('error', () => { try { upstream.destroy(); } catch (_) {} });
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -515,7 +520,11 @@ function start (port, certsDirOrCA, callback, _baseDomains) {
|
|||||||
// Attach the TCP server to the fake server so stop() can close it
|
// Attach the TCP server to the fake server so stop() can close it
|
||||||
fakeServer._tcpServer = tcpServer;
|
fakeServer._tcpServer = tcpServer;
|
||||||
|
|
||||||
tcpServer.on('connection', handleRawConnection);
|
tcpServer.on('connection', (sock) => {
|
||||||
|
fakeServer.connections.add(sock);
|
||||||
|
sock.once('close', () => fakeServer.connections.delete(sock));
|
||||||
|
handleRawConnection(sock);
|
||||||
|
});
|
||||||
|
|
||||||
tcpServer.on('error', (err) => {
|
tcpServer.on('error', (err) => {
|
||||||
if (err.code === 'EADDRINUSE') {
|
if (err.code === 'EADDRINUSE') {
|
||||||
@@ -552,13 +561,13 @@ function stop (callback) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Destroy all raw TCP connections so close() fires immediately
|
// Destroy all tracked raw TCP connections so close() fires immediately.
|
||||||
const rawConns = tcpServer._connections;
|
// We use fakeServer.connections (populated in the 'connection' handler above)
|
||||||
if (rawConns && typeof rawConns[Symbol.iterator] === 'function') {
|
// instead of the undocumented tcpServer._connections internal.
|
||||||
for (const socket of rawConns) {
|
for (const socket of fakeServer.connections) {
|
||||||
try { socket.destroy(); } catch (_) {}
|
try { socket.destroy(); } catch (_) {}
|
||||||
}
|
}
|
||||||
}
|
fakeServer.connections.clear();
|
||||||
|
|
||||||
tcpServer.close(() => {
|
tcpServer.close(() => {
|
||||||
if (callback) callback();
|
if (callback) callback();
|
||||||
|
|||||||
@@ -90,6 +90,7 @@ async function startVncSession(sessionId, holesailInst, tunnelPort, wsPort, labe
|
|||||||
// Buffer data arriving from VNC server before the browser WS connects
|
// Buffer data arriving from VNC server before the browser WS connects
|
||||||
const outputBuffer = [];
|
const outputBuffer = [];
|
||||||
const MAX_BUFFER = 512 * 1024;
|
const MAX_BUFFER = 512 * 1024;
|
||||||
|
const MAX_ENTRIES = 1000;
|
||||||
let bufferedBytes = 0;
|
let bufferedBytes = 0;
|
||||||
let activeWsConn = null;
|
let activeWsConn = null;
|
||||||
let tcpSocket = null;
|
let tcpSocket = null;
|
||||||
@@ -102,7 +103,8 @@ async function startVncSession(sessionId, holesailInst, tunnelPort, wsPort, labe
|
|||||||
} else {
|
} else {
|
||||||
outputBuffer.push(buf);
|
outputBuffer.push(buf);
|
||||||
bufferedBytes += buf.length;
|
bufferedBytes += buf.length;
|
||||||
while (bufferedBytes > MAX_BUFFER && outputBuffer.length > 0) {
|
// Evict oldest entries when either the byte cap or the entry count cap is exceeded.
|
||||||
|
while ((bufferedBytes > MAX_BUFFER || outputBuffer.length > MAX_ENTRIES) && outputBuffer.length > 0) {
|
||||||
bufferedBytes -= outputBuffer.shift().length;
|
bufferedBytes -= outputBuffer.shift().length;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -369,9 +371,14 @@ async function startSession(payload) {
|
|||||||
let holesailInst = null;
|
let holesailInst = null;
|
||||||
try {
|
try {
|
||||||
holesailInst = new Holesail({ client: true, key: hsUrl, host: '127.0.0.1', port: tunnelPort });
|
holesailInst = new Holesail({ client: true, key: hsUrl, host: '127.0.0.1', port: tunnelPort });
|
||||||
await holesailInst.ready();
|
const TUNNEL_READY_TIMEOUT_MS = 30000;
|
||||||
|
await Promise.race([
|
||||||
|
holesailInst.ready(),
|
||||||
|
new Promise((_, reject) => setTimeout(() => reject(new Error('Tunnel ready timeout after ' + TUNNEL_READY_TIMEOUT_MS + 'ms')), TUNNEL_READY_TIMEOUT_MS))
|
||||||
|
]);
|
||||||
log('tunnel ready on 127.0.0.1:' + tunnelPort);
|
log('tunnel ready on 127.0.0.1:' + tunnelPort);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
if (holesailInst) try { holesailInst.close(); } catch (_) {}
|
||||||
releaseTunnelPort(tunnelPort);
|
releaseTunnelPort(tunnelPort);
|
||||||
releaseWsPort(wsPort);
|
releaseWsPort(wsPort);
|
||||||
return { ok: false, error: 'Tunnel failed: ' + e.message };
|
return { ok: false, error: 'Tunnel failed: ' + e.message };
|
||||||
|
|||||||
@@ -286,6 +286,9 @@ async function startSession(payload) {
|
|||||||
if (activeWsConn) {
|
if (activeWsConn) {
|
||||||
try { activeWsConn.write(Buffer.from('\r\n[Failed to spawn ssh: ' + e.message + ']\r\n')); } catch (_) {}
|
try { activeWsConn.write(Buffer.from('\r\n[Failed to spawn ssh: ' + e.message + ']\r\n')); } catch (_) {}
|
||||||
}
|
}
|
||||||
|
// Release all resources — ports, WS server, and Holesail tunnel — so they
|
||||||
|
// are not leaked for the lifetime of the process.
|
||||||
|
stopSession({ sessionId }).catch(() => {});
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -404,7 +407,9 @@ async function startSession(payload) {
|
|||||||
if (passwordDelivered || collectingPassword) return;
|
if (passwordDelivered || collectingPassword) return;
|
||||||
|
|
||||||
const str = Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk);
|
const str = Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk);
|
||||||
outputSoFar += str;
|
// Keep only the tail so LOGIN_RE can match recent output without
|
||||||
|
// accumulating the entire session transcript in memory.
|
||||||
|
outputSoFar = (outputSoFar + str).slice(-4096);
|
||||||
seenOutput = true;
|
seenOutput = true;
|
||||||
|
|
||||||
// If the accumulated output looks like post-login content, we are in —
|
// If the accumulated output looks like post-login content, we are in —
|
||||||
@@ -423,6 +428,11 @@ async function startSession(payload) {
|
|||||||
// If SSH produces absolutely no output within 2 s (e.g. very slow tunnel)
|
// If SSH produces absolutely no output within 2 s (e.g. very slow tunnel)
|
||||||
// fall through to the password prompt anyway.
|
// fall through to the password prompt anyway.
|
||||||
fallbackTimer = setTimeout(startPasswordCollection, 2000);
|
fallbackTimer = setTimeout(startPasswordCollection, 2000);
|
||||||
|
|
||||||
|
// If the PTY exits or errors before auth completes, cancel the timers so
|
||||||
|
// they don't fire against a dead session.
|
||||||
|
pty.once('exit', cancelPasswordWatch);
|
||||||
|
pty.once('error', cancelPasswordWatch);
|
||||||
}
|
}
|
||||||
|
|
||||||
// PTY output → WebSocket (live, no buffering needed — browser is already open)
|
// PTY output → WebSocket (live, no buffering needed — browser is already open)
|
||||||
|
|||||||
Reference in New Issue
Block a user