Repository: chimurai/http-proxy-middleware
Pull request #1092 (2025-04-09 18:18:31)

fix(fixRequestBody): handle invalid request

pkg-pr-new[bot]: 



```
npm i https://pkg.pr.new/http-proxy-middleware@1092
```


_commit: <a href="https://github.com/chimurai/http-proxy-middleware/runs/40122055935"><code>ffefd91</code></a>_

coveralls: 
[![Coverage Status](https://coveralls.io/builds/73152527/badge)](https://coveralls.io/builds/73152527)

coverage: 97.203% (+0.1%) from 97.101%
when pulling **ffefd91344246923be8ff7172e0cef65feb4de5c on fix-request-body-handle-invalid-requests**
into **020976044d113fc0bcbbaf995e91d05e2829a145 on master**.


Patch:
diff --git a/src/handlers/fix-request-body.ts b/src/handlers/fix-request-body.ts
index 6943b27..2f3afd6 100644
--- a/src/handlers/fix-request-body.ts
+++ b/src/handlers/fix-request-body.ts
@@ -1,16 +1,27 @@
 import type * as http from 'http';
 import * as querystring from 'querystring';
 
-export type BodyParserLikeRequest = http.IncomingMessage & { body: any };
+import type { Options } from '../types';
+import { getLogger } from '../logger';
+
+export type BodyParserLikeRequest = http.IncomingMessage & { body?: any };
+
+type HandleBadRequestArgs = {
+  proxyReq: http.ClientRequest;
+  req: BodyParserLikeRequest;
+  res: http.ServerResponse<http.IncomingMessage>;
+};
 
 /**
  * Fix proxied body if bodyParser is involved.
  */
-export function fixRequestBody<TReq = http.IncomingMessage>(
+export function fixRequestBody<TReq extends BodyParserLikeRequest = BodyParserLikeRequest>(
   proxyReq: http.ClientRequest,
   req: TReq,
+  res: http.ServerResponse<http.IncomingMessage>,
+  options: Options,
 ): void {
-  const requestBody = (req as unknown as BodyParserLikeRequest).body;
+  const requestBody = req.body;
 
   if (!requestBody) {
     return;
@@ -22,6 +33,22 @@ export function fixRequestBody<TReq = http.IncomingMessage>(
     return;
   }
 
+  const logger = getLogger(options);
+
+  // Handle bad request when unexpected "Connect: Upgrade" header is provided
+  if (/upgrade/gi.test(proxyReq.getHeader('Connection') as string)) {
+    handleBadRequest({ proxyReq, req, res });
+    logger.error(`[HPM] HPM_UNEXPECTED_CONNECTION_UPGRADE_HEADER. Aborted request: ${req.url}`);
+    return;
+  }
+
+  // Handle bad request when invalid request body is provided
+  if (hasInvalidKeys(requestBody)) {
+    handleBadRequest({ proxyReq, req, res });
+    logger.error(`[HPM] HPM_INVALID_REQUEST_DATA. Aborted request: ${req.url}`);
+    return;
+  }
+
   const writeBody = (bodyData: string) => {
     proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData));
     proxyReq.write(bodyData);
@@ -52,3 +79,14 @@ function handlerFormDataBodyData(contentType: string, data: any) {
   }
   return str;
 }
+
+function hasInvalidKeys(obj) {
+  return Object.keys(obj).some((key) => /[\n\r]/.test(key));
+}
+
+function handleBadRequest({ proxyReq, req, res }: HandleBadRequestArgs) {
+  res.writeHead(400);
+  res.end('Bad Request');
+  proxyReq.destroy();
+  req.destroy();
+}


Test patch:
diff --git a/test/unit/fix-request-body.spec.ts b/test/unit/fix-request-body.spec.ts
index 0c81546..f58bdd2 100644
--- a/test/unit/fix-request-body.spec.ts
+++ b/test/unit/fix-request-body.spec.ts
@@ -1,5 +1,5 @@
 import { Socket } from 'net';
-import { ClientRequest, IncomingMessage } from 'http';
+import { ClientRequest, IncomingMessage, ServerResponse } from 'http';
 import * as querystring from 'querystring';
 import { fixRequestBody, BodyParserLikeRequest } from '../../src/handlers/fix-request-body';
 
@@ -10,8 +10,14 @@ const fakeProxyRequest = (): ClientRequest => {
   return proxyRequest;
 };
 
+const fakeProxyResponse = (): ServerResponse<IncomingMessage> => {
+  const res = new ServerResponse(new IncomingMessage(new Socket()));
+  return res;
+};
+
 const createRequestWithBody = (body: unknown): BodyParserLikeRequest => {
   const req = new IncomingMessage(new Socket()) as BodyParserLikeRequest;
+  req.url = '/test_path';
   req.body = body;
   return req;
 };
@@ -32,7 +38,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody(undefined));
+    fixRequestBody(proxyRequest, createRequestWithBody(undefined), fakeProxyResponse(), {});
 
     expect(proxyRequest.setHeader).not.toHaveBeenCalled();
     expect(proxyRequest.write).not.toHaveBeenCalled();
@@ -45,7 +51,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({}));
+    fixRequestBody(proxyRequest, createRequestWithBody({}), fakeProxyResponse(), {});
 
     expect(proxyRequest.setHeader).toHaveBeenCalled();
     expect(proxyRequest.write).toHaveBeenCalled();
@@ -58,7 +64,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -72,7 +83,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = handlerFormDataBodyData('multipart/form-data', {
       someField: 'some value',
@@ -96,7 +112,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = handlerFormDataBodyData('multipart/form-data', {
       someField: 'some value',
@@ -121,7 +142,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
     expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
@@ -134,7 +160,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = querystring.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -148,7 +179,12 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = querystring.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -162,11 +198,69 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
+    fixRequestBody(
+      proxyRequest,
+      createRequestWithBody({ someField: 'some value' }),
+      fakeProxyResponse(),
+      {},
+    );
 
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
     expect(proxyRequest.write).toHaveBeenCalledTimes(1);
     expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
   });
+
+  it('should return 400 and abort request on "Connection: Upgrade" header', () => {
+    const proxyRequest = fakeProxyRequest();
+    const request = createRequestWithBody({ someField: 'some value' });
+    const proxyResponse = fakeProxyResponse();
+    proxyRequest.setHeader('connection', 'upgrade');
+    proxyRequest.setHeader('content-type', 'application/x-www-form-urlencoded');
+
+    jest.spyOn(proxyRequest, 'destroy');
+    jest.spyOn(request, 'destroy');
+    jest.spyOn(proxyResponse, 'writeHead');
+    jest.spyOn(proxyResponse, 'end');
+
+    const logger = {
+      error: jest.fn(),
+    };
+
+    fixRequestBody(proxyRequest, request, proxyResponse, { logger });
+
+    expect(proxyResponse.writeHead).toHaveBeenCalledWith(400);
+    expect(proxyResponse.end).toHaveBeenCalledTimes(1);
+    expect(proxyRequest.destroy).toHaveBeenCalledTimes(1);
+    expect(request.destroy).toHaveBeenCalledTimes(1);
+    expect(logger.error).toHaveBeenCalledWith(
+      `[HPM] HPM_UNEXPECTED_CONNECTION_UPGRADE_HEADER. Aborted request: /test_path`,
+    );
+  });
+
+  it('should return 400 and abort request on invalid request data', () => {
+    const proxyRequest = fakeProxyRequest();
+    const request = createRequestWithBody({ 'INVALID \n\r DATA': '' });
+    const proxyResponse = fakeProxyResponse();
+    proxyRequest.setHeader('content-type', 'application/x-www-form-urlencoded');
+
+    jest.spyOn(proxyRequest, 'destroy');
+    jest.spyOn(request, 'destroy');
+    jest.spyOn(proxyResponse, 'writeHead');
+    jest.spyOn(proxyResponse, 'end');
+
+    const logger = {
+      error: jest.fn(),
+    };
+
+    fixRequestBody(proxyRequest, request, proxyResponse, { logger });
+
+    expect(proxyResponse.writeHead).toHaveBeenCalledWith(400);
+    expect(proxyResponse.end).toHaveBeenCalledTimes(1);
+    expect(proxyRequest.destroy).toHaveBeenCalledTimes(1);
+    expect(request.destroy).toHaveBeenCalledTimes(1);
+    expect(logger.error).toHaveBeenCalledWith(
+      `[HPM] HPM_INVALID_REQUEST_DATA. Aborted request: /test_path`,
+    );
+  });
 });