Skip to content

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
Collaborator

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