Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed the maximum size of request body allowed in HTTP request #10108

Closed
wants to merge 7 commits into from
Closed

Changed the maximum size of request body allowed in HTTP request #10108

wants to merge 7 commits into from

Conversation

harshmadhani
Copy link
Contributor

I have made changes to the maximum size allowed for a HTTP request body by default from unlimited to 2 MB. This is to resolve the #10079 issue

@geoand
Copy link
Contributor

geoand commented Jun 19, 2020

Thanks for this!

I am not sure it's the proper default though. @stuartwdouglas what does Undertow default to?

@jaikiran
Copy link
Member

From what I can see[1] Undertow itself leaves it undefined (i.e. no limit). However, in WildFly server (which underneath uses Undertow), it's default set to 10MB as an upper limit[2]

[1] https://github.com/undertow-io/undertow/blob/master/core/src/main/java/io/undertow/UndertowOptions.java#L49

[2] https://github.com/wildfly/wildfly/blob/master/undertow/src/main/resources/schema/wildfly-undertow_11_0.xsd#L90 (the max-post-size ultimately translates to the UndertowOptions.MAX_ENTITY_SIZE https://github.com/wildfly/wildfly/blob/330da67750e2b9c910c384723cdbff6f5ac32ec2/undertow/src/main/java/org/wildfly/extension/undertow/ListenerResourceDefinition.java#L150)

@stuartwdouglas
Copy link
Member

I think 10mb makes more sense than 2mb. In general though this is very hard to put a meaningful limit on, as different systems have different requirements. If you are processing a REST request and fully buffering the request that if you are uploading a file and writing the data to disk as it comes in.

@geoand
Copy link
Contributor

geoand commented Jun 22, 2020

Thanks for the input folks. So I think we agree that 10MB is a better default. @harshmadhani can you please update the PR?

@harshmadhani
Copy link
Contributor Author

@geoand yes I guess 10 MB is a better default. I will update the PR.

@geoand
Copy link
Contributor

geoand commented Jun 22, 2020

Can you please squash the commits into a single one?

@harshmadhani
Copy link
Contributor Author

@geoand ok let me raise a seperate merge request for the same. I will close this one

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

Successfully merging this pull request may close these issues.

5 participants