Skip to content

Commit 8eaf213

Browse files
committed
perf(@angular/ssr): avoid buffering request body when sanitizing headers
Avoid teeing the incoming request body stream with `request.clone()` when removing untrusted `X-Forwarded-*` headers in `sanitizeRequestHeaders`. Teeing the stream caused the unconsumed branch to buffer the uploaded body in memory. Passing `request` directly to `new Request(request, { headers })` transfers the stream without teeing or buffering. Additionally, `request.signal` is automatically inherited by the new `Request` instance without needing to pass it explicitly. Closes #33706
1 parent 40a923a commit 8eaf213

2 files changed

Lines changed: 23 additions & 3 deletions

File tree

packages/angular/ssr/src/utils/validation.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export function validateUrl(url: URL, allowedHosts: ReadonlySet<string>): void {
8888

8989
/**
9090
* Sanitizes the proxy headers of a request by removing unallowed `X-Forwarded-*` headers.
91-
* If no headers need to be removed, the original request is returned without cloning.
91+
* If no headers need to be removed, the original request is returned unchanged.
9292
*
9393
* @param request - The incoming `Request` object to sanitize.
9494
* @param trustProxyHeaders - A set of allowed proxy headers.
@@ -117,8 +117,7 @@ export function sanitizeRequestHeaders(
117117
}
118118

119119
return headersDeleted
120-
? new Request(request.clone(), {
121-
signal: request.signal,
120+
? new Request(request, {
122121
headers,
123122
})
124123
: request;

packages/angular/ssr/test/utils/validation_spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,5 +493,26 @@ describe('Validation Utils', () => {
493493
expect(secured.headers.get('host')).toBe('example.com');
494494
expect(secured.headers.get('forwarded')).toBe('host=proxy.com;proto=https');
495495
});
496+
497+
it('should not tee request body and should preserve abort signal when removing unallowed headers', async () => {
498+
const controller = new AbortController();
499+
const req = new Request('http://example.com', {
500+
method: 'POST',
501+
body: 'test body',
502+
signal: controller.signal,
503+
headers: {
504+
'host': 'example.com',
505+
'x-forwarded-host': 'evil.com',
506+
},
507+
});
508+
509+
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(undefined));
510+
511+
expect(req.bodyUsed).toBeTrue();
512+
expect(await secured.text()).toBe('test body');
513+
514+
controller.abort();
515+
expect(secured.signal.aborted).toBeTrue();
516+
});
496517
});
497518
});

0 commit comments

Comments
 (0)