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

Increase the size limit of the request's body #285

Closed
wants to merge 1 commit into from

Conversation

haikyuu
Copy link

@haikyuu haikyuu commented Dec 19, 2016

The default limit of a request's body is 100kb, which is not enough if you want to save something big (an image for example) in the database.
This is the link to the limit option in the docs.

The default limit of a request's body is `100kb`, which is not enough if you want to save something big (an image for example) in the database.
Here is the link to the `limit` option in the docs: https://github.com/expressjs/body-parser#limit
@calebmer
Copy link
Collaborator

Relay solves this problem by allowing to attach files. See the mutation config getFiles function. Could we perhaps support that behavior instead of extending the request body limit? My fear with extending the body limit is that it could allow attackers to abuse the system and insert insanely large things.

I'd also like to know the logic for why the default is what is. There must be some reason, right?

@haikyuu
Copy link
Author

haikyuu commented Dec 20, 2016

Yes, why is the parser limit defaulted to 100kb?. There was a security issue and there is a hit on performance since parsing the body blocks the event loop.
I think it's a much better idea to implement getFiles like in relay.

I am not particularly familiar with Relay. Any thoughts on how we could do that?

@calebmer
Copy link
Collaborator

It looks like when Relay detects the user has defined some files it switches from sending JSON to FormData with a Content-Type of multipart/form-data. The same format used by HTML5 forms. See: https://github.com/facebook/relay/blob/30162b25194705e770301922023f1d03bf1791e7/src/network-layer/default/RelayDefaultNetworkLayer.js#L93-L111

An example of that format can be found in the HTML4 specification: https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4

For parsing multipart form bodies, it looks like body-parser won't do it, but it does list alternatives: https://www.npmjs.com/package/body-parser

@calebmer
Copy link
Collaborator

This MDN article is helpful for context: https://developer.mozilla.org/en-US/docs/Web/API/FormData/Using_FormData_Objects

@calebmer
Copy link
Collaborator

calebmer commented Mar 5, 2017

Going to close this PR. While we should still consider our approach to uploading files that discussion probably belongs to another issue. We are also adding the size limit as an option in this PR: #379

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

Successfully merging this pull request may close these issues.

2 participants