Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/development/setup-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ TEST_TIMEOUT=30000
| `DAP_TRACE` | Set to `1` to trace every DAP frame to a per-session `dap-trace-<sessionId>.ndjson` (capped at 50 MB) | Not set |
| `DAP_TRACE_FILE` | Explicit DAP trace file path (implies tracing on) | Not set |
| `MCP_SKIP_ORPHAN_REAPERS` | Set to `1` to skip the startup orphan-process scans (e.g. PID-namespaced containers where orphans are impossible) | Not set |
| `DAP_MAX_FRAME_BYTES` | Upper bound for a single DAP frame body accepted by the frame decoder | 64 MB |

## Troubleshooting Setup Issues

Expand Down
117 changes: 92 additions & 25 deletions src/proxy/dap-framing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,62 +20,117 @@ export function encodeDapMessage(message: DebugProtocol.ProtocolMessage): Buffer
return Buffer.from(`Content-Length: ${Buffer.byteLength(json, 'utf8')}${TWO_CRLF}${json}`, 'utf8');
}

export type DapFrameDecoderErrorContext = 'header' | 'json';
export type DapFrameDecoderErrorContext = 'header' | 'json' | 'overflow';

/**
* Default frame-body cap (issue #402). Generous — real DAP messages are KBs;
* the largest legitimate payloads (huge variable dumps, base64 blobs) stay
* far under this. Matches the repo's 64MB maxBuffer precedent. Override per
* decoder via options.maxContentLength or globally via DAP_MAX_FRAME_BYTES.
*/
const DEFAULT_MAX_CONTENT_LENGTH = 64 * 1024 * 1024;

/** Headers are a handful of short lines; anything past this with no separator is garbage. */
const MAX_HEADER_BYTES = 16 * 1024;

export interface DapFrameDecoderOptions {
/**
* Invoked on malformed input. 'header' means an invalid/absent
* Content-Length header was seen and the buffered payload was discarded;
* 'json' means a complete frame failed to parse and was skipped.
* 'json' means a complete frame failed to parse and was skipped;
* 'overflow' means the peer advertised a frame above maxContentLength (or
* streamed header bytes past the header allowance) and the buffer was
* discarded — same recovery contract as 'header'.
*/
onError?: (error: Error, context: DapFrameDecoderErrorContext) => void;
/** Upper bound for a single frame body (issue #402); default 64 MB or DAP_MAX_FRAME_BYTES. */
maxContentLength?: number;
}

function defaultMaxContentLength(): number {
const env = Number(process.env.DAP_MAX_FRAME_BYTES);
return Number.isFinite(env) && env > 0 ? env : DEFAULT_MAX_CONTENT_LENGTH;
}

/**
* Incremental Content-Length frame decoder. Feed it raw socket chunks via
* push(); it returns every complete protocol message contained so far and
* buffers any trailing partial frame for the next call.
*
* Accumulation is linear (issue #402): header bytes live in a small bounded
* buffer, body bytes in a chunk list concatenated once per completed frame —
* never a per-chunk Buffer.concat of the whole backlog.
*/
export class DapFrameDecoder {
private rawData = Buffer.alloc(0);
/** Bytes in the header-search phase; bounded by MAX_HEADER_BYTES + one chunk. */
private headerData: Buffer = Buffer.alloc(0);
/** Body bytes of the frame in progress, concatenated once when complete. */
private bodyChunks: Buffer[] = [];
private bodyBytes = 0;
private contentLength = -1;
private readonly onError?: DapFrameDecoderOptions['onError'];
private readonly maxContentLength: number;

constructor(options?: DapFrameDecoderOptions) {
this.onError = options?.onError;
this.maxContentLength = options?.maxContentLength ?? defaultMaxContentLength();
}

push(data: Buffer): DebugProtocol.ProtocolMessage[] {
this.rawData = Buffer.concat([this.rawData, data]);
const messages: DebugProtocol.ProtocolMessage[] = [];
let input: Buffer | null = data;

while (true) {
if (this.contentLength >= 0) {
// We have a content length, check if we have the full message
if (this.rawData.length >= this.contentLength) {
const message = this.rawData.toString('utf8', 0, this.contentLength);
this.rawData = this.rawData.slice(this.contentLength);
this.contentLength = -1;

if (message.length > 0) {
try {
messages.push(JSON.parse(message) as DebugProtocol.ProtocolMessage);
} catch (e) {
this.onError?.(e instanceof Error ? e : new Error(String(e)), 'json');
}
// Body phase: collect chunks until the advertised length is buffered
if (input && input.length > 0) {
this.bodyChunks.push(input);
this.bodyBytes += input.length;
}
input = null;
if (this.bodyBytes < this.contentLength) {
break;
}
const full = this.bodyChunks.length === 1
? this.bodyChunks[0]
: Buffer.concat(this.bodyChunks, this.bodyBytes);
const message = full.toString('utf8', 0, this.contentLength);
// Bytes past the frame belong to the next header
input = full.subarray(this.contentLength);
this.bodyChunks = [];
this.bodyBytes = 0;
this.contentLength = -1;

if (message.length > 0) {
try {
messages.push(JSON.parse(message) as DebugProtocol.ProtocolMessage);
} catch (e) {
this.onError?.(e instanceof Error ? e : new Error(String(e)), 'json');
}
continue;
}
continue;
}

// Header phase
if (input && input.length > 0) {
this.headerData = this.headerData.length === 0 ? input : Buffer.concat([this.headerData, input]);
}
input = null;

// Look for the header
const idx = this.rawData.indexOf(TWO_CRLF);
const idx = this.headerData.indexOf(TWO_CRLF);
if (idx === -1) {
if (this.headerData.length > MAX_HEADER_BYTES) {
this.onError?.(
new Error(`No header separator within ${MAX_HEADER_BYTES} bytes; discarding payload`),
'overflow'
);
this.reset();
}
// No complete header yet
break;
}

const header = this.rawData.toString('utf8', 0, idx);
const header = this.headerData.toString('utf8', 0, idx);
const lines = header.split('\r\n');
let parsedLength: number | null = null;

Expand All @@ -90,27 +145,39 @@ export class DapFrameDecoder {
}
}

// Remove header from buffer
this.rawData = this.rawData.slice(idx + TWO_CRLF.length);
// Remove header from buffer; the remainder starts the body (or next header)
const remainder = this.headerData.subarray(idx + TWO_CRLF.length);
this.headerData = Buffer.alloc(0);

if (parsedLength === null || parsedLength <= 0 || !Number.isFinite(parsedLength)) {
this.onError?.(
new Error('Invalid Content-Length header encountered; discarding payload'),
'header'
);
this.contentLength = -1;
this.rawData = Buffer.alloc(0);
this.reset();
continue;
}

if (parsedLength > this.maxContentLength) {
this.onError?.(
new Error(`Content-Length ${parsedLength} exceeds cap ${this.maxContentLength}; discarding payload`),
'overflow'
);
this.reset();
continue;
}

this.contentLength = parsedLength;
input = remainder;
}

return messages;
}

reset(): void {
this.rawData = Buffer.alloc(0);
this.headerData = Buffer.alloc(0);
this.bodyChunks = [];
this.bodyBytes = 0;
this.contentLength = -1;
}
}
6 changes: 4 additions & 2 deletions src/proxy/dap-mirror-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,10 @@ export class MirrorClientConnection {
this.decoder = new DapFrameDecoder({
onError: (error, context) => {
this.logger.warn(`[DapMirror] Malformed frame from mirror client (${context}): ${error.message}`);
if (context === 'header') {
// Framing is unrecoverable once the byte stream is corrupt.
if (context === 'header' || context === 'overflow') {
// Framing is unrecoverable once the byte stream is corrupt, and a
// client advertising an over-cap frame is equally untrustworthy
// (issue #402).
this.close();
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/proxy/minimal-dap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ export class MinimalDapClient extends EventEmitter {
private socket: Socket | null = null;
private decoder = new DapFrameDecoder({
onError: (error, context) => {
if (context === 'header') {
logger.warn('[MinimalDapClient] Invalid Content-Length header encountered; discarding payload');
if (context === 'header' || context === 'overflow') {
// Same recovery contract: the decoder discarded the buffer (issue #402)
logger.warn(`[MinimalDapClient] ${error.message}`);
} else {
logger.error('[MinimalDapClient] Error parsing message:', error);
}
Expand Down
62 changes: 62 additions & 0 deletions tests/unit/proxy/dap-framing.property.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,68 @@ describe('DapFrameDecoder malformed-input recovery', () => {
expect(errors).toEqual(['json']);
});

it('rejects a Content-Length above the cap with an overflow error and recovers (issue #402)', () => {
const errors: string[] = [];
const decoder = new DapFrameDecoder({
onError: (_err, context) => errors.push(context),
maxContentLength: 1024
});

// A hostile/buggy peer advertises a frame the decoder must never buffer
const evil = Buffer.from('Content-Length: 999999999\r\n\r\npartial body...', 'utf8');
expect(decoder.push(evil)).toEqual([]);
expect(errors).toEqual(['overflow']);

// The overflow discarded the buffer; a fresh valid frame decodes normally
const msg = { seq: 4, type: 'event', event: 'output', body: { text: 'ok' } };
expect(decoder.push(frame(msg))).toEqual([msg]);
});

it('accepts a frame exactly at the cap boundary (issue #402)', () => {
const bodyText = '{"type":"event"}';
const decoder = new DapFrameDecoder({
maxContentLength: Buffer.byteLength(bodyText, 'utf8')
});

const exact = Buffer.from(
`Content-Length: ${Buffer.byteLength(bodyText, 'utf8')}\r\n\r\n${bodyText}`,
'utf8'
);
expect(decoder.push(exact)).toEqual([{ type: 'event' }]);
});

it('bounds header-search accumulation when no header separator ever arrives (issue #402)', () => {
const errors: string[] = [];
const decoder = new DapFrameDecoder({
onError: (_err, context) => errors.push(context)
});

// A garbage stream with no \r\n\r\n used to buffer without bound
const garbage = Buffer.alloc(20 * 1024, 0x78); // 20 KB of 'x'
expect(decoder.push(garbage)).toEqual([]);
expect(errors).toEqual(['overflow']);

// Recovery after the discard
const msg = { seq: 5, type: 'event', event: 'output', body: {} };
expect(decoder.push(frame(msg))).toEqual([msg]);
});

it('reassembles a large frame delivered in many small chunks (issue #402)', () => {
// The old implementation Buffer.concat'ed per chunk — O(N^2) on exactly
// this shape. This pins correctness; the linear accumulation is the fix.
const big = { seq: 6, type: 'event', event: 'output', body: { text: 'y'.repeat(256 * 1024) } };
const encoded = frame(big);
const decoder = new DapFrameDecoder();

const received: unknown[] = [];
const CHUNK = 1024;
for (let offset = 0; offset < encoded.length; offset += CHUNK) {
received.push(...decoder.push(encoded.subarray(offset, offset + CHUNK)));
}

expect(received).toEqual([big]);
});

it('reset() drops any partial frame in progress', () => {
const decoder = new DapFrameDecoder();
const msg = { seq: 3, type: 'event', event: 'continued', body: {} };
Expand Down
12 changes: 6 additions & 6 deletions tests/unit/proxy/minimal-dap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ describe('MinimalDapClient', () => {
'[MinimalDapClient] Invalid Content-Length header encountered; discarding payload'
);
expect(protocolSpy).not.toHaveBeenCalled();
expect(
(client as unknown as { decoder: { rawData: Buffer } }).decoder.rawData.length
).toBe(0);
const decoder = (client as unknown as { decoder: { headerData: Buffer; bodyBytes: number } }).decoder;
expect(decoder.headerData.length).toBe(0);
expect(decoder.bodyBytes).toBe(0);
protocolSpy.mockRestore();
});

Expand All @@ -325,9 +325,9 @@ describe('MinimalDapClient', () => {
2,
'[MinimalDapClient] Invalid Content-Length header encountered; discarding payload'
);
expect(
(client as unknown as { decoder: { rawData: Buffer } }).decoder.rawData.length
).toBe(0);
const decoder = (client as unknown as { decoder: { headerData: Buffer; bodyBytes: number } }).decoder;
expect(decoder.headerData.length).toBe(0);
expect(decoder.bodyBytes).toBe(0);
});

it('should handle incomplete message body', async () => {
Expand Down
Loading