Skip to content

fix(http): Introduce a max buffer size for fetch requests on SSR#68927

Open
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fetch-max-buffer
Open

fix(http): Introduce a max buffer size for fetch requests on SSR#68927
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fetch-max-buffer

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

By default, the FetchBackend on SSR will limit the response body size to 10 MB. If the response body exceeds this limit, an error will be thrown.

This default value can be configured by providing a different value for the HTTP_FETCH_MAX_RESPONSE_SIZE injection token.

This is to prevent DoS on the server when loading large files

@pullapprove pullapprove Bot requested review from atscott and kirjs May 26, 2026 08:36
@angular-robot angular-robot Bot added the area: common/http Issues related to HTTP and HTTP Client label May 26, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 26, 2026
By default, the `FetchBackend` on SSR will limit the response body size to 10 MB.
If the response body exceeds this limit, an error will be thrown.

This default value can be configured by providing a different value for the `HTTP_FETCH_MAX_RESPONSE_SIZE` injection token.

This is to prevent DoS on the server when loading large files
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should consider adding a per-request configuration. If a developer needs to handle a single request larger than 10MB (which is already an edge case), modifying the global setting would inadvertently allow all requests to exceed that limit.

OOC, how did we we came up with a 10mb as a sensible default? IMO, 10mb is still rather large which can still easily cause a DoS.

chunks.push(value);
receivedLength += value.length;

if (this.maxResponseSize !== null && receivedLength > this.maxResponseSize) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT if the content length header is provider and it's larger than the maxResponseSize we can skip this logic and get and open the reader.

const req = new HttpRequest('GET', '/test', {responseType: 'text'});
const events = await trackEvents(backend.handle(req));

expect(events.length).toBe(2);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT

Suggested change
expect(events.length).toBe(2);
expect(events).toHaveSize(2);

const events = await trackEvents(backend.handle(req));
const error = events[1] as HttpErrorResponse;

expect(events.length).toBe(2);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT

Suggested change
expect(events.length).toBe(2);
expect(events).toHaveSize(2);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: common/http Issues related to HTTP and HTTP Client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants