mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-05 10:18:47 +00:00
fix(docker): harden proxy with timeout, Bun compat, and test coverage
- Add 30s timeout on proxy requests to prevent indefinite hangs
- Wrap resolveLocalCliproxyPort in try/catch with default port fallback
- Simplify buildProxyBody: remove fragile content-length check that
caused Bun to fall through to req.pipe() on consumed streams
- Replace proxyRes.pipe(res) with manual streaming for Bun compatibility
(pipe hangs after writeHead in Bun runtime)
- Replace deprecated req.on('aborted') with res.on('close') cleanup
(req.on('close') fires with req.destroyed=true in Bun after body
consumption, prematurely destroying the proxy connection)
- Explicitly end proxy request for bodyless methods (GET/HEAD/OPTIONS)
instead of piping an already-consumed express stream
- Add server.closeAllConnections() in test cleanup to prevent hangs
- Add GET passthrough and 502 unreachable test cases
This commit is contained in:
@@ -19,9 +19,16 @@ export interface CliproxyLocalProxyDeps {
|
|||||||
resolveTargetPort?: () => number;
|
resolveTargetPort?: () => number;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Proxy request timeout in milliseconds (30 seconds) */
|
||||||
|
const PROXY_TIMEOUT_MS = 30_000;
|
||||||
|
|
||||||
function resolveLocalCliproxyPort(): number {
|
function resolveLocalCliproxyPort(): number {
|
||||||
const config = loadOrCreateUnifiedConfig();
|
try {
|
||||||
return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT);
|
const config = loadOrCreateUnifiedConfig();
|
||||||
|
return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT);
|
||||||
|
} catch {
|
||||||
|
return CLIPROXY_DEFAULT_PORT;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function isJsonContentType(contentType: string | string[] | undefined): boolean {
|
function isJsonContentType(contentType: string | string[] | undefined): boolean {
|
||||||
@@ -30,22 +37,17 @@ function isJsonContentType(contentType: string | string[] | undefined): boolean
|
|||||||
}
|
}
|
||||||
|
|
||||||
function buildProxyBody(req: Request): Buffer | undefined {
|
function buildProxyBody(req: Request): Buffer | undefined {
|
||||||
if (!isJsonContentType(req.headers['content-type']) || req.body === undefined) {
|
// If express.json() parsed the body (content-type is JSON and req.body is populated),
|
||||||
return undefined;
|
// re-serialize it since the original request stream was consumed by the middleware.
|
||||||
}
|
if (
|
||||||
|
!isJsonContentType(req.headers['content-type']) ||
|
||||||
const contentLengthHeader = req.headers['content-length'];
|
req.body === undefined ||
|
||||||
const contentLength = Array.isArray(contentLengthHeader)
|
req.body === null
|
||||||
? contentLengthHeader[0]
|
) {
|
||||||
: contentLengthHeader;
|
|
||||||
const hasTransferEncoding = req.headers['transfer-encoding'] !== undefined;
|
|
||||||
const parsedContentLength =
|
|
||||||
typeof contentLength === 'string' ? Number.parseInt(contentLength, 10) : NaN;
|
|
||||||
|
|
||||||
if (!hasTransferEncoding && (!Number.isFinite(parsedContentLength) || parsedContentLength <= 0)) {
|
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// express.json() sets req.body to the parsed value — re-serialize for the proxy target
|
||||||
return Buffer.from(JSON.stringify(req.body));
|
return Buffer.from(JSON.stringify(req.body));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -100,20 +102,27 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {}
|
|||||||
path: targetPath,
|
path: targetPath,
|
||||||
method: req.method,
|
method: req.method,
|
||||||
headers: buildProxyHeaders(req.headers, targetPort, bodyBuffer),
|
headers: buildProxyHeaders(req.headers, targetPort, bodyBuffer),
|
||||||
|
timeout: PROXY_TIMEOUT_MS,
|
||||||
},
|
},
|
||||||
(proxyRes) => {
|
(proxyRes) => {
|
||||||
res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers);
|
res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers);
|
||||||
proxyRes.pipe(res, { end: true });
|
// Manual streaming instead of pipe() for Bun runtime compatibility
|
||||||
|
proxyRes.on('data', (chunk: Buffer) => res.write(chunk));
|
||||||
|
proxyRes.on('end', () => res.end());
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
|
proxyReq.on('timeout', () => proxyReq.destroy());
|
||||||
|
|
||||||
proxyReq.on('error', () => {
|
proxyReq.on('error', () => {
|
||||||
if (!res.headersSent) {
|
if (!res.headersSent) {
|
||||||
res.status(502).json({ error: 'CLIProxy is not reachable' });
|
res.status(502).json({ error: 'CLIProxy is not reachable' });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
req.on('aborted', () => proxyReq.destroy());
|
// Clean up proxy connection when client disconnects.
|
||||||
|
// Only use res.on('close') — req.on('close') fires with req.destroyed=true
|
||||||
|
// in Bun after body consumption, which would prematurely kill the proxy.
|
||||||
res.on('close', () => {
|
res.on('close', () => {
|
||||||
if (!res.writableEnded) {
|
if (!res.writableEnded) {
|
||||||
proxyReq.destroy();
|
proxyReq.destroy();
|
||||||
@@ -125,6 +134,14 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {}
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For methods without a body (GET, HEAD, etc.) or when express.json()
|
||||||
|
// has already consumed the stream, end the request immediately.
|
||||||
|
const hasBody = req.method !== 'GET' && req.method !== 'HEAD' && req.method !== 'OPTIONS';
|
||||||
|
if (!hasBody) {
|
||||||
|
proxyReq.end();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
req.pipe(proxyReq, { end: true });
|
req.pipe(proxyReq, { end: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -56,6 +56,8 @@ afterEach(async () => {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Force-close keep-alive connections so server.close() doesn't hang
|
||||||
|
server.closeAllConnections();
|
||||||
await new Promise<void>((resolve) => server.close(() => resolve()));
|
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -118,4 +120,33 @@ describe('cliproxy local proxy route', () => {
|
|||||||
path: '/v0/management/test',
|
path: '/v0/management/test',
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('forwards GET requests and returns backend response', async () => {
|
||||||
|
const backend = await createBackendServer((_req, res) => {
|
||||||
|
res.writeHead(200, { 'Content-Type': 'text/html' });
|
||||||
|
res.end('<html>management panel</html>');
|
||||||
|
});
|
||||||
|
const proxy = await createProxyServer({
|
||||||
|
resolveTargetPort: () => backend.port,
|
||||||
|
enforceAccess: () => true,
|
||||||
|
});
|
||||||
|
|
||||||
|
const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/management.html`);
|
||||||
|
|
||||||
|
expect(response.status).toBe(200);
|
||||||
|
expect(await response.text()).toBe('<html>management panel</html>');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns 502 when CLIProxy is not reachable', async () => {
|
||||||
|
// Use a port with nothing listening
|
||||||
|
const proxy = await createProxyServer({
|
||||||
|
resolveTargetPort: () => 19999,
|
||||||
|
enforceAccess: () => true,
|
||||||
|
});
|
||||||
|
|
||||||
|
const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/`);
|
||||||
|
|
||||||
|
expect(response.status).toBe(502);
|
||||||
|
expect(await response.json()).toEqual({ error: 'CLIProxy is not reachable' });
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user