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

Req.Body Truncated in Versions >= 1.6.0 #41

Closed
jackryon opened this issue Aug 25, 2014 · 11 comments
Closed

Req.Body Truncated in Versions >= 1.6.0 #41

jackryon opened this issue Aug 25, 2014 · 11 comments
Assignees

Comments

@jackryon
Copy link

Upon updating my project dependencies I realized that url encoded data originating from my client app seemed to be getting severely truncated. Installing previous tags back to 1.5.x seemed to remedy the problem.. 2 quick screenshots illustrate these issues:

installed tag 1.5.2
bp-1 5 2

installed tag 1.6.0
bp-1 6 0

Big difference!
Haven't pinned down the change that's responsible for this error, but I thought you should know about this. I'm sticking with the 1.5's for the time being.
Thanks.

@dougwilson dougwilson self-assigned this Aug 25, 2014
@dougwilson
Copy link
Contributor

Yea. You need to either use {extended: false} or ask over at https://github.com/hapijs/qs/ to make the parametersLimit configurable and I can expose the configuration here.

It also seems like you're sending a really big object over urlencoded. Perhaps you just want to use JSON :)?

@dougwilson
Copy link
Contributor

and in case you were wondering, the reason this changed in a non-major version is because using bodyParser.urlencoded({ extended: true }) with version < 1.6.0 expose the server to a really easy DoS attack.

@jackryon
Copy link
Author

Thanks for the quick reply.
Using extended: false with 1.6.0 still yields incomplete data, though.

bp-1 6 0-extended-false

I'm about to try sending up application/json.

@dougwilson
Copy link
Contributor

Using extended: false with 1.6.0 still yields incomplete data, though.

Ah, yes, I forgot Node.js core also has a default limit of 1,000 parameters (though it's limit is configurable): https://github.com/joyent/node/blob/v0.10.24/lib/querystring.js#L172

I'm going to talk with the qs guys to get theirs parameter limit configurable and I can expose the configuration, but yea, overall I think your use-case may be better served using JSON instead of urlencoded if you are not using a straight up HTML <form> to make the submission.

@jackryon
Copy link
Author

Awesome. I appreciate your help.

@dougwilson
Copy link
Contributor

So for the extended module, there is no way right now it can even handle > 1000 parameters. Feeding it 2000 parameters make it take ~1710ms on my machine, an eternity.

@dougwilson
Copy link
Contributor

If you wish to keep using very large urlencoded payloads, you will be able to configure the limit in the upcoming 1.7.0:

app.use(bodyParser.urlencoded({ extended: true, parameterLimit: 5000 }))

@dougwilson
Copy link
Contributor

In addition, the module will no longer give you weird truncated req.body and will instead completely reject the payload with a 415 if there are too many parameters so you are not accidentally processing an incomplete payload.

@jkrems
Copy link

jkrems commented Aug 28, 2014

Are the 1710ms a pathological example or is this really the expected runtime for parsing 2000 elements? It seems like a lot.

@dougwilson
Copy link
Contributor

Are the 1710ms a pathological example or is this really the expected runtime for parsing 2000 elements? It seems like a lot.

It is a real example. qs has fixed this, though, so it only takes 80ms now for the same parse.

@jkrems
Copy link

jkrems commented Aug 28, 2014

Ah, thanks. Glad to hear that, got me nervous for a minute! :)

dougwilson added a commit that referenced this issue Aug 30, 2014
dougwilson added a commit that referenced this issue Sep 2, 2014
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

No branches or pull requests

3 participants