Repository: chimurai/http-proxy-middleware
Pull request #1096 (2025-04-10 11:58:40)

fix(fixRequestBody): check readableLength

pkg-pr-new[bot]: 



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


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

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

coverage: 97.115% (-0.09%) from 97.203%
when pulling **c0a54e5faa1aead454ccf2a6173cbbe6a43bf466 on fix-request-body**
into **01934d33b157b2d49f7099a18ceec165e7502040 on master**.


Patch:
diff --git a/src/handlers/fix-request-body.ts b/src/handlers/fix-request-body.ts
index 2f3afd6..e8e1373 100644
--- a/src/handlers/fix-request-body.ts
+++ b/src/handlers/fix-request-body.ts
@@ -1,26 +1,20 @@
 import type * as http from 'http';
 import * as querystring from 'querystring';
 
-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 extends BodyParserLikeRequest = BodyParserLikeRequest>(
   proxyReq: http.ClientRequest,
   req: TReq,
-  res: http.ServerResponse<http.IncomingMessage>,
-  options: Options,
 ): void {
+  // skip fixRequestBody() when req.readableLength not 0 (bodyParser failure)
+  if (req.readableLength !== 0) {
+    return;
+  }
+
   const requestBody = req.body;
 
   if (!requestBody) {
@@ -33,22 +27,6 @@ export function fixRequestBody<TReq extends BodyParserLikeRequest = BodyParserLi
     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);
@@ -79,14 +57,3 @@ 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 f58bdd2..d125046 100644
--- a/test/unit/fix-request-body.spec.ts
+++ b/test/unit/fix-request-body.spec.ts
@@ -38,7 +38,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody(undefined), fakeProxyResponse(), {});
+    fixRequestBody(proxyRequest, createRequestWithBody(undefined));
 
     expect(proxyRequest.setHeader).not.toHaveBeenCalled();
     expect(proxyRequest.write).not.toHaveBeenCalled();
@@ -51,7 +51,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(proxyRequest, createRequestWithBody({}), fakeProxyResponse(), {});
+    fixRequestBody(proxyRequest, createRequestWithBody({}));
 
     expect(proxyRequest.setHeader).toHaveBeenCalled();
     expect(proxyRequest.write).toHaveBeenCalled();
@@ -64,12 +64,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -83,12 +78,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = handlerFormDataBodyData('multipart/form-data', {
       someField: 'some value',
@@ -112,12 +102,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = handlerFormDataBodyData('multipart/form-data', {
       someField: 'some value',
@@ -142,12 +127,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
     expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
@@ -160,12 +140,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = querystring.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -179,12 +154,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = querystring.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -198,12 +168,7 @@ describe('fixRequestBody', () => {
     jest.spyOn(proxyRequest, 'setHeader');
     jest.spyOn(proxyRequest, 'write');
 
-    fixRequestBody(
-      proxyRequest,
-      createRequestWithBody({ someField: 'some value' }),
-      fakeProxyResponse(),
-      {},
-    );
+    fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
 
     const expectedBody = JSON.stringify({ someField: 'some value' });
     expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -211,56 +176,26 @@ describe('fixRequestBody', () => {
     expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
   });
 
-  it('should return 400 and abort request on "Connection: Upgrade" header', () => {
+  it('should not fixRequestBody() when there bodyParser fails', () => {
     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(),
-    };
+    const request = {
+      get readableLength() {
+        return 4444; // simulate bodyParser failure
+      },
+    } as BodyParserLikeRequest;
 
-    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, 'write');
     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 });
+    fixRequestBody(proxyRequest, request);
 
-    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`,
-    );
+    expect(proxyResponse.end).toHaveBeenCalledTimes(0);
+    expect(proxyRequest.write).toHaveBeenCalledTimes(0);
+    expect(proxyRequest.destroy).toHaveBeenCalledTimes(0);
   });
 });