branderson/dotcom-arcade
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: improve CGI executor reliability and resource cleanup

Browse Source 
- Clear force-kill timeout on process exit to prevent leaks
- Support LF-only line endings in CGI output parsing
- Add proper timeout handling for request body collection
- Save/restore env vars in tests to prevent pollution
This commit is contained in:
Darkraise
2026-02-05 18:17:05 +07:00
parent 8cde8a98eb
commit 492c049499
3 changed files with 106 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
game-service/src/cgi/cgi-executor.test.ts
+63 -4
View File
@@ -41,13 +41,32 @@ describe('CgiExecutor', () => {
let executor: CgiExecutor;
// Store original env vars to restore after tests
const originalEnv: Record<string, string | undefined> = {};
beforeEach(() => {
vi.clearAllMocks();
executor = new CgiExecutor(mockConfig);
// Save original sensitive env vars
originalEnv.JWT_SECRET = process.env.JWT_SECRET;
originalEnv.DATABASE_URL = process.env.DATABASE_URL;
});
afterEach(() => {
vi.restoreAllMocks();
// Restore original env vars (cleanup sensitive test data)
if (originalEnv.JWT_SECRET === undefined) {
delete process.env.JWT_SECRET;
} else {
process.env.JWT_SECRET = originalEnv.JWT_SECRET;
}
if (originalEnv.DATABASE_URL === undefined) {
delete process.env.DATABASE_URL;
} else {
process.env.DATABASE_URL = originalEnv.DATABASE_URL;
}
});
describe('validateBinary', () => {
@@ -210,10 +229,7 @@ describe('CgiExecutor', () => {
// Verify sensitive variables are NOT passed
expect(capturedEnv.JWT_SECRET).toBeUndefined();
expect(capturedEnv.DATABASE_URL).toBeUndefined();
// Cleanup
delete process.env.JWT_SECRET;
delete process.env.DATABASE_URL;
// Cleanup handled by afterEach
});
});
@@ -375,6 +391,49 @@ describe('CgiExecutor', () => {
expect(response.statusCode).toBe(404);
expect(response.body.toString()).toBe('<h1>Not Found</h1>');
});
it('should parse CGI output with LF-only line endings', async () => {
const { spawn } = await import('child_process');
vi.mocked(spawn).mockImplementation(() => {
const mockProcess: any = {
stdin: { write: vi.fn(), end: vi.fn() },
stdout: {
on: vi.fn((event: string, callback: (data: Buffer) => void) => {
if (event === 'data') {
// Some PHP configurations use LF (\n) instead of CRLF (\r\n)
callback(
Buffer.from(
'Content-Type: text/html\nX-LF-Header: works\n\n<html>LF output</html>'
)
);
}
}),
},
stderr: { on: vi.fn() },
on: vi.fn((event: string, callback: any) => {
if (event === 'close') {
setTimeout(() => callback(0, null), 10);
}
}),
kill: vi.fn(),
};
return mockProcess;
});
const request: CgiRequest = {
method: 'GET',
url: new URL('http://example.com/lftest.php'),
headers: {},
};
const response = await executor.execute('/var/www/htdocs/example.com/lftest.php', request);
expect(response.statusCode).toBe(200);
expect(response.headers['Content-Type']).toBe('text/html');
expect(response.headers['X-LF-Header']).toBe('works');
expect(response.body.toString()).toBe('<html>LF output</html>');
});
});
describe('timeout handling', () => {
game-service/src/cgi/cgi-executor.ts
+23 -5
View File
@@ -1,4 +1,4 @@
import { spawn, ChildProcess } from 'child_process';
import { spawn } from 'child_process';
import path from 'path';
import fs from 'fs/promises';
import { logger } from '../utils/logger';
@@ -121,6 +121,7 @@ export class CgiExecutor {
let stderrSize = 0;
let timedOut = false;
let processExited = false;
let forceKillHandle: NodeJS.Timeout | null = null;
// Set up timeout
const timeoutHandle = setTimeout(() => {
@@ -129,7 +130,7 @@ export class CgiExecutor {
proc.kill('SIGTERM');
// Force kill after 5 seconds if still running
setTimeout(() => {
forceKillHandle = setTimeout(() => {
if (!processExited) {
proc.kill('SIGKILL');
}
@@ -161,6 +162,9 @@ export class CgiExecutor {
proc.on('close', (code, signal) => {
processExited = true;
clearTimeout(timeoutHandle);
if (forceKillHandle) {
clearTimeout(forceKillHandle);
}
// Log stderr if present
if (stderr.length > 0) {
@@ -200,6 +204,9 @@ export class CgiExecutor {
proc.on('error', (error) => {
processExited = true;
clearTimeout(timeoutHandle);
if (forceKillHandle) {
clearTimeout(forceKillHandle);
}
logger.error(`[CGI] Process error: ${error.message}`);
reject(new Error(`Failed to execute CGI script: ${error.message}`));
});
@@ -324,11 +331,22 @@ export class CgiExecutor {
* Body content here...
*
* The Status header determines the HTTP response code.
* Note: Some PHP configurations may use \n instead of \r\n.
*/
private parseCgiOutput(output: Buffer): CgiResponse {
// Find the header/body separator (empty line)
// Try CRLF first (standard), then LF-only (some PHP configs)
const outputStr = output.toString('binary');
const separatorIndex = outputStr.indexOf('\r\n\r\n');
let separatorIndex = outputStr.indexOf('\r\n\r\n');
let separatorLength = 4;
let lineEnding = '\r\n';
// Fallback to LF-only if CRLF not found
if (separatorIndex === -1) {
separatorIndex = outputStr.indexOf('\n\n');
separatorLength = 2;
lineEnding = '\n';
}
if (separatorIndex === -1) {
// No headers found, treat entire output as body
@@ -342,14 +360,14 @@ export class CgiExecutor {
// Split headers and body
const headerSection = outputStr.substring(0, separatorIndex);
const bodyStart = separatorIndex + 4; // Skip \r\n\r\n
const bodyStart = separatorIndex + separatorLength;
const body = output.slice(bodyStart);
// Parse headers
const headers: Record<string, string> = {};
let statusCode = 200;
const headerLines = headerSection.split('\r\n');
const headerLines = headerSection.split(lineEnding);
for (const line of headerLines) {
const colonIndex = line.indexOf(':');
if (colonIndex === -1) continue;
game-service/src/proxy-request-handler.ts
+20 -10
View File
@@ -321,16 +321,29 @@ export class ProxyRequestHandler {
}
/**
* Collect request body with size limit
* Collect request body with size limit and timeout
*/
private collectRequestBody(req: IncomingMessage): Promise<Buffer | undefined> {
return new Promise((resolve, reject) => {
const chunks: Buffer[] = [];
let totalSize = 0;
let completed = false;
// Timeout for body collection (30 seconds)
const timeoutHandle = setTimeout(() => {
if (!completed) {
completed = true;
req.destroy(new Error('Request body collection timed out'));
reject(new Error('Request body collection timed out'));
}
}, 30000);
req.on('data', (chunk: Buffer) => {
if (completed) return;
totalSize += chunk.length;
if (totalSize > MAX_REQUEST_BODY_SIZE) {
completed = true;
clearTimeout(timeoutHandle);
req.destroy(new Error('Request body too large'));
reject(new Error('Request body too large'));
return;
@@ -339,6 +352,9 @@ export class ProxyRequestHandler {
});
req.on('end', () => {
if (completed) return;
completed = true;
clearTimeout(timeoutHandle);
if (chunks.length === 0) {
resolve(undefined);
} else {
@@ -347,17 +363,11 @@ export class ProxyRequestHandler {
});
req.on('error', (error) => {
if (completed) return;
completed = true;
clearTimeout(timeoutHandle);
reject(error);
});
// Timeout for body collection
setTimeout(() => {
if (chunks.length > 0 && totalSize > 0) {
resolve(Buffer.concat(chunks));
} else {
resolve(undefined);
}
}, 30000); // 30 second timeout
});
}
}