From 610a9933ac0a7ec19ee349ff7c6490b605a739ed Mon Sep 17 00:00:00 2001 From: Serhii Shpak Date: Mon, 6 Jul 2026 23:46:59 +0300 Subject: [PATCH] fix(PP-3449): stop request inspector from swallowing proxied PUT/POST bodies The Request Inspector (added in e53fa90) broke request bodies for proxied PUT/POST requests, causing the backend to answer 408 Request Timeout: - plugin.ts mounted the inspector Express app (with global express.json/ urlencoded body parsers) for all paths before the proxy, so JSON bodies were consumed before http-proxy could pipe them to the backend. Route only /@api/* and /@pp-dev/inspector into it, matching cli.ts. - request-capture.middleware.ts attached a req 'data' listener, which switches the stream into flowing mode; buffered body chunks were emitted on process.nextTick, before the async proxy middleware attached its pipe, and were lost. Patch req.emit instead: chunks are observed only when a downstream consumer actually reads the stream, leaving its state intact. Add regression tests covering stream neutrality, late (next-tick) consumers receiving the full body, and capture still recording consumed bodies. --- src/lib/request-capture.middleware.ts | 33 +++--- src/plugin.ts | 17 ++- .../lib/request-capture.middleware.spec.ts | 100 ++++++++++++++++++ 3 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 tests/unit/lib/request-capture.middleware.spec.ts diff --git a/src/lib/request-capture.middleware.ts b/src/lib/request-capture.middleware.ts index 7446e05..ca398b0 100644 --- a/src/lib/request-capture.middleware.ts +++ b/src/lib/request-capture.middleware.ts @@ -21,27 +21,34 @@ export function createRequestCaptureMiddleware(store: RequestStore, captureLimit const id = store.allocateId(); const startTime = Date.now(); - // Capture request body by tapping into data events without consuming the stream + // Capture the request body by patching req.emit rather than attaching a 'data' + // listener: a listener would switch the stream into flowing mode, and buffered + // chunks would be emitted (and lost) before a downstream consumer — e.g. the + // proxy — attaches its own reader. Patching emit observes chunks only when + // something downstream actually reads the stream, leaving its state untouched. const reqChunks: Buffer[] = []; let reqSize = 0; let reqTruncated = false; - req.on('data', (chunk: Buffer | string) => { - if (reqTruncated) { - return; - } + const origReqEmit = req.emit.bind(req); - const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + req.emit = ((event: string | symbol, ...args: unknown[]): boolean => { + if (event === 'data' && !reqTruncated) { + const chunk = args[0]; + const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk)); - reqSize += buf.byteLength; + reqSize += buf.byteLength; - if (reqSize <= captureLimit) { - reqChunks.push(buf); - } else { - reqTruncated = true; - reqChunks.length = 0; // free memory for partial chunks + if (reqSize <= captureLimit) { + reqChunks.push(buf); + } else { + reqTruncated = true; + reqChunks.length = 0; // free memory for partial chunks + } } - }); + + return origReqEmit(event as never, ...(args as never[])); + }) as typeof req.emit; // Capture response body by wrapping write/end const resChunks: Buffer[] = []; diff --git a/src/plugin.ts b/src/plugin.ts index 4821682..6d62900 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -13,7 +13,7 @@ import { createInternalServer } from './lib/internal.middleware.js'; import { getTokenErrorInfo } from './lib/helpers/index.js'; import { RequestStore } from './lib/request-store.js'; import { createRequestCaptureMiddleware } from './lib/request-capture.middleware.js'; -import { registerInspectorRoutes } from './lib/request-inspector.js'; +import { registerInspectorRoutes, INSPECTOR_PATH } from './lib/request-inspector.js'; // ─── Public config types ────────────────────────────────────────────────────── @@ -355,7 +355,20 @@ function vitePPDev(options: NormalizedVitePPDevOptions): Plugin { const internalServer = createInternalServer(); registerInspectorRoutes(internalServer, reqStore, inspectorCaptureLimit); - server.middlewares.use(internalServer); + + // Only route internal pp-dev paths into the Express app. Mounting it for all + // paths would run its global express.json()/urlencoded() body parsers, which + // consume the request stream before the proxy can pipe it to the backend + // (the backend then waits for a body that never arrives and replies 408). + server.middlewares.use((req, res, next) => { + if (req.url?.startsWith('/@api/') || req.url?.startsWith(INSPECTOR_PATH)) { + internalServer(req as never, res as never, next); + + return; + } + + next(); + }); } if (backendBaseURL) { diff --git a/tests/unit/lib/request-capture.middleware.spec.ts b/tests/unit/lib/request-capture.middleware.spec.ts new file mode 100644 index 0000000..7a47bab --- /dev/null +++ b/tests/unit/lib/request-capture.middleware.spec.ts @@ -0,0 +1,100 @@ +import { describe, it, expect } from 'vitest'; +import { PassThrough } from 'node:stream'; +import type { IncomingMessage, ServerResponse } from 'node:http'; +import { createRequestCaptureMiddleware } from '../../../src/lib/request-capture.middleware.js'; +import { RequestStore } from '../../../src/lib/request-store.js'; + +/** + * Regression tests for the "capture middleware eats proxied request bodies" bug. + * + * The middleware previously attached a `req.on('data')` listener, which switched + * the request stream into flowing mode. Buffered body chunks were emitted on + * process.nextTick — before the (async) proxy middleware attached its pipe to the + * backend — so PUT/POST bodies were lost and the backend answered 408 after + * waiting for a body that never arrived. Capture must observe the stream without + * changing its state: the body belongs to the downstream consumer. + */ + +function makeReq(method = 'PUT', url = '/api/resource'): IncomingMessage { + const req = new PassThrough() as unknown as IncomingMessage; + + req.method = method; + req.url = url; + req.headers = { 'content-type': 'application/json' }; + + return req; +} + +function makeRes(): ServerResponse { + const res = new PassThrough() as unknown as ServerResponse; + + res.statusCode = 200; + (res as any).getHeaders = () => ({}); + + return res; +} + +describe('createRequestCaptureMiddleware — stream neutrality', () => { + it('does not switch the request stream into flowing mode', () => { + const store = new RequestStore(1024 * 1024); + const middleware = createRequestCaptureMiddleware(store); + const req = makeReq(); + + middleware(req, makeRes(), () => {}); + + // A 'data' listener would set readableFlowing to true; the stream must stay paused. + expect((req as unknown as PassThrough).readableFlowing).not.toBe(true); + }); + + it('delivers the full body to a late (next-tick) downstream consumer', async () => { + const store = new RequestStore(1024 * 1024); + const middleware = createRequestCaptureMiddleware(store); + const req = makeReq(); + const body = JSON.stringify({ name: 'test', value: 42 }); + + // Body is already buffered before any consumer attaches — the 408 scenario. + (req as unknown as PassThrough).end(body); + + middleware(req, makeRes(), () => {}); + + // The proxy attaches its pipe asynchronously (http-proxy-middleware is async). + await new Promise((resolve) => setImmediate(resolve)); + + const received: Buffer[] = []; + const sink = new PassThrough(); + + sink.on('data', (chunk: Buffer) => received.push(chunk)); + (req as unknown as PassThrough).pipe(sink); + + await new Promise((resolve) => sink.on('end', resolve)); + + expect(Buffer.concat(received).toString()).toBe(body); + }); + + it('still captures the request body once a consumer reads the stream', async () => { + const store = new RequestStore(1024 * 1024); + const middleware = createRequestCaptureMiddleware(store); + const req = makeReq(); + const res = makeRes(); + const body = JSON.stringify({ hello: 'world' }); + + middleware(req, res, () => {}); + + (req as unknown as PassThrough).end(body); + + // Downstream consumer drains the stream (as the proxy or a body parser would). + (req as unknown as PassThrough).resume(); + await new Promise((resolve) => (req as unknown as PassThrough).on('end', resolve)); + + // finalize() runs on res.end + res.end(); + + const entries = store.list({}); + + expect(entries).toHaveLength(1); + + const entry = store.get(entries[0].id); + + expect(entry?.requestBody?.toString()).toBe(body); + }); +});