Skip to content

Commit bd3c124

Browse files
chimuraichimurai
authored
fix(fixRequestBody): handle invalid request (#1092)
1 parent 0209760 commit bd3c124

File tree

2 files changed

+145
-13
lines changed

2 files changed

+145
-13
lines changed

‎src/handlers/fix-request-body.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
11
import type * as http from 'http';
22
import * as querystring from 'querystring';
33

4-
export type BodyParserLikeRequest = http.IncomingMessage & { body: any };
4+
import type { Options } from '../types';
5+
import { getLogger } from '../logger';
6+
7+
export type BodyParserLikeRequest = http.IncomingMessage & { body?: any };
8+
9+
type HandleBadRequestArgs = {
10+
proxyReq: http.ClientRequest;
11+
req: BodyParserLikeRequest;
12+
res: http.ServerResponse<http.IncomingMessage>;
13+
};
514

615
/**
716
* Fix proxied body if bodyParser is involved.
817
*/
9-
export function fixRequestBody<TReq = http.IncomingMessage>(
18+
export function fixRequestBody<TReq extends BodyParserLikeRequest = BodyParserLikeRequest>(
1019
proxyReq: http.ClientRequest,
1120
req: TReq,
21+
res: http.ServerResponse<http.IncomingMessage>,
22+
options: Options,
1223
): void {
13-
const requestBody = (req as unknown as BodyParserLikeRequest).body;
24+
const requestBody = req.body;
1425

1526
if (!requestBody) {
1627
return;
@@ -22,6 +33,22 @@ export function fixRequestBody<TReq = http.IncomingMessage>(
2233
return;
2334
}
2435

36+
const logger = getLogger(options);
37+
38+
// Handle bad request when unexpected "Connect: Upgrade" header is provided
39+
if (/upgrade/gi.test(proxyReq.getHeader('Connection') as string)) {
40+
handleBadRequest({ proxyReq, req, res });
41+
logger.error(`[HPM] HPM_UNEXPECTED_CONNECTION_UPGRADE_HEADER. Aborted request: ${req.url}`);
42+
return;
43+
}
44+
45+
// Handle bad request when invalid request body is provided
46+
if (hasInvalidKeys(requestBody)) {
47+
handleBadRequest({ proxyReq, req, res });
48+
logger.error(`[HPM] HPM_INVALID_REQUEST_DATA. Aborted request: ${req.url}`);
49+
return;
50+
}
51+
2552
const writeBody = (bodyData: string) => {
2653
proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData));
2754
proxyReq.write(bodyData);
@@ -52,3 +79,14 @@ function handlerFormDataBodyData(contentType: string, data: any) {
5279
}
5380
return str;
5481
}
82+
83+
function hasInvalidKeys(obj) {
84+
return Object.keys(obj).some((key) => /[\n\r]/.test(key));
85+
}
86+
87+
function handleBadRequest({ proxyReq, req, res }: HandleBadRequestArgs) {
88+
res.writeHead(400);
89+
res.end('Bad Request');
90+
proxyReq.destroy();
91+
req.destroy();
92+
}

‎test/unit/fix-request-body.spec.ts

Lines changed: 104 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Socket } from 'net';
2-
import { ClientRequest, IncomingMessage } from 'http';
2+
import { ClientRequest, IncomingMessage, ServerResponse } from 'http';
33
import * as querystring from 'querystring';
44
import { fixRequestBody, BodyParserLikeRequest } from '../../src/handlers/fix-request-body';
55

@@ -10,8 +10,14 @@ const fakeProxyRequest = (): ClientRequest => {
1010
return proxyRequest;
1111
};
1212

13+
const fakeProxyResponse = (): ServerResponse<IncomingMessage> => {
14+
const res = new ServerResponse(new IncomingMessage(new Socket()));
15+
return res;
16+
};
17+
1318
const createRequestWithBody = (body: unknown): BodyParserLikeRequest => {
1419
const req = new IncomingMessage(new Socket()) as BodyParserLikeRequest;
20+
req.url = '/test_path';
1521
req.body = body;
1622
return req;
1723
};
@@ -32,7 +38,7 @@ describe('fixRequestBody', () => {
3238
jest.spyOn(proxyRequest, 'setHeader');
3339
jest.spyOn(proxyRequest, 'write');
3440

35-
fixRequestBody(proxyRequest, createRequestWithBody(undefined));
41+
fixRequestBody(proxyRequest, createRequestWithBody(undefined), fakeProxyResponse(), {});
3642

3743
expect(proxyRequest.setHeader).not.toHaveBeenCalled();
3844
expect(proxyRequest.write).not.toHaveBeenCalled();
@@ -45,7 +51,7 @@ describe('fixRequestBody', () => {
4551
jest.spyOn(proxyRequest, 'setHeader');
4652
jest.spyOn(proxyRequest, 'write');
4753

48-
fixRequestBody(proxyRequest, createRequestWithBody({}));
54+
fixRequestBody(proxyRequest, createRequestWithBody({}), fakeProxyResponse(), {});
4955

5056
expect(proxyRequest.setHeader).toHaveBeenCalled();
5157
expect(proxyRequest.write).toHaveBeenCalled();
@@ -58,7 +64,12 @@ describe('fixRequestBody', () => {
5864
jest.spyOn(proxyRequest, 'setHeader');
5965
jest.spyOn(proxyRequest, 'write');
6066

61-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
67+
fixRequestBody(
68+
proxyRequest,
69+
createRequestWithBody({ someField: 'some value' }),
70+
fakeProxyResponse(),
71+
{},
72+
);
6273

6374
const expectedBody = JSON.stringify({ someField: 'some value' });
6475
expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -72,7 +83,12 @@ describe('fixRequestBody', () => {
7283
jest.spyOn(proxyRequest, 'setHeader');
7384
jest.spyOn(proxyRequest, 'write');
7485

75-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
86+
fixRequestBody(
87+
proxyRequest,
88+
createRequestWithBody({ someField: 'some value' }),
89+
fakeProxyResponse(),
90+
{},
91+
);
7692

7793
const expectedBody = handlerFormDataBodyData('multipart/form-data', {
7894
someField: 'some value',
@@ -96,7 +112,12 @@ describe('fixRequestBody', () => {
96112
jest.spyOn(proxyRequest, 'setHeader');
97113
jest.spyOn(proxyRequest, 'write');
98114

99-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
115+
fixRequestBody(
116+
proxyRequest,
117+
createRequestWithBody({ someField: 'some value' }),
118+
fakeProxyResponse(),
119+
{},
120+
);
100121

101122
const expectedBody = handlerFormDataBodyData('multipart/form-data', {
102123
someField: 'some value',
@@ -121,7 +142,12 @@ describe('fixRequestBody', () => {
121142
jest.spyOn(proxyRequest, 'setHeader');
122143
jest.spyOn(proxyRequest, 'write');
123144

124-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
145+
fixRequestBody(
146+
proxyRequest,
147+
createRequestWithBody({ someField: 'some value' }),
148+
fakeProxyResponse(),
149+
{},
150+
);
125151
const expectedBody = JSON.stringify({ someField: 'some value' });
126152
expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
127153
expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
@@ -134,7 +160,12 @@ describe('fixRequestBody', () => {
134160
jest.spyOn(proxyRequest, 'setHeader');
135161
jest.spyOn(proxyRequest, 'write');
136162

137-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
163+
fixRequestBody(
164+
proxyRequest,
165+
createRequestWithBody({ someField: 'some value' }),
166+
fakeProxyResponse(),
167+
{},
168+
);
138169

139170
const expectedBody = querystring.stringify({ someField: 'some value' });
140171
expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -148,7 +179,12 @@ describe('fixRequestBody', () => {
148179
jest.spyOn(proxyRequest, 'setHeader');
149180
jest.spyOn(proxyRequest, 'write');
150181

151-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
182+
fixRequestBody(
183+
proxyRequest,
184+
createRequestWithBody({ someField: 'some value' }),
185+
fakeProxyResponse(),
186+
{},
187+
);
152188

153189
const expectedBody = querystring.stringify({ someField: 'some value' });
154190
expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
@@ -162,11 +198,69 @@ describe('fixRequestBody', () => {
162198
jest.spyOn(proxyRequest, 'setHeader');
163199
jest.spyOn(proxyRequest, 'write');
164200

165-
fixRequestBody(proxyRequest, createRequestWithBody({ someField: 'some value' }));
201+
fixRequestBody(
202+
proxyRequest,
203+
createRequestWithBody({ someField: 'some value' }),
204+
fakeProxyResponse(),
205+
{},
206+
);
166207

167208
const expectedBody = JSON.stringify({ someField: 'some value' });
168209
expect(proxyRequest.setHeader).toHaveBeenCalledWith('Content-Length', expectedBody.length);
169210
expect(proxyRequest.write).toHaveBeenCalledTimes(1);
170211
expect(proxyRequest.write).toHaveBeenCalledWith(expectedBody);
171212
});
213+
214+
it('should return 400 and abort request on "Connection: Upgrade" header', () => {
215+
const proxyRequest = fakeProxyRequest();
216+
const request = createRequestWithBody({ someField: 'some value' });
217+
const proxyResponse = fakeProxyResponse();
218+
proxyRequest.setHeader('connection', 'upgrade');
219+
proxyRequest.setHeader('content-type', 'application/x-www-form-urlencoded');
220+
221+
jest.spyOn(proxyRequest, 'destroy');
222+
jest.spyOn(request, 'destroy');
223+
jest.spyOn(proxyResponse, 'writeHead');
224+
jest.spyOn(proxyResponse, 'end');
225+
226+
const logger = {
227+
error: jest.fn(),
228+
};
229+
230+
fixRequestBody(proxyRequest, request, proxyResponse, { logger });
231+
232+
expect(proxyResponse.writeHead).toHaveBeenCalledWith(400);
233+
expect(proxyResponse.end).toHaveBeenCalledTimes(1);
234+
expect(proxyRequest.destroy).toHaveBeenCalledTimes(1);
235+
expect(request.destroy).toHaveBeenCalledTimes(1);
236+
expect(logger.error).toHaveBeenCalledWith(
237+
`[HPM] HPM_UNEXPECTED_CONNECTION_UPGRADE_HEADER. Aborted request: /test_path`,
238+
);
239+
});
240+
241+
it('should return 400 and abort request on invalid request data', () => {
242+
const proxyRequest = fakeProxyRequest();
243+
const request = createRequestWithBody({ 'INVALID \n\r DATA': '' });
244+
const proxyResponse = fakeProxyResponse();
245+
proxyRequest.setHeader('content-type', 'application/x-www-form-urlencoded');
246+
247+
jest.spyOn(proxyRequest, 'destroy');
248+
jest.spyOn(request, 'destroy');
249+
jest.spyOn(proxyResponse, 'writeHead');
250+
jest.spyOn(proxyResponse, 'end');
251+
252+
const logger = {
253+
error: jest.fn(),
254+
};
255+
256+
fixRequestBody(proxyRequest, request, proxyResponse, { logger });
257+
258+
expect(proxyResponse.writeHead).toHaveBeenCalledWith(400);
259+
expect(proxyResponse.end).toHaveBeenCalledTimes(1);
260+
expect(proxyRequest.destroy).toHaveBeenCalledTimes(1);
261+
expect(request.destroy).toHaveBeenCalledTimes(1);
262+
expect(logger.error).toHaveBeenCalledWith(
263+
`[HPM] HPM_INVALID_REQUEST_DATA. Aborted request: /test_path`,
264+
);
265+
});
172266
});

0 commit comments

Comments
 (0)