Skip to content

Commit 3316980

Browse files
authored
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`.
1 parent 44ff584 commit 3316980

2 files changed

Lines changed: 37 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: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,5 +493,40 @@ 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 transfer request body without teeing when removing unallowed headers', async () => {
498+
const req = new Request('http://example.com', {
499+
method: 'POST',
500+
body: 'test body',
501+
headers: {
502+
'host': 'example.com',
503+
'x-forwarded-host': 'evil.com',
504+
},
505+
});
506+
507+
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(undefined));
508+
509+
// In the Fetch specification, calling `request.clone()` tees the body stream and leaves
510+
// `req.bodyUsed` as `false`. Passing `req` directly to `new Request(req, ...)` transfers the
511+
// underlying stream without teeing, immediately marking `req.bodyUsed` as `true`.
512+
expect(req.bodyUsed).toBeTrue();
513+
expect(await secured.text()).toBe('test body');
514+
});
515+
516+
it('should preserve abort signal when removing unallowed headers', () => {
517+
const controller = new AbortController();
518+
const req = new Request('http://example.com', {
519+
signal: controller.signal,
520+
headers: {
521+
'host': 'example.com',
522+
'x-forwarded-host': 'evil.com',
523+
},
524+
});
525+
526+
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(undefined));
527+
528+
controller.abort();
529+
expect(secured.signal.aborted).toBeTrue();
530+
});
496531
});
497532
});

0 commit comments

Comments
 (0)