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

Make the maximum items in an array configurable. #65

Closed
wants to merge 1 commit into from
Closed

Make the maximum items in an array configurable. #65

wants to merge 1 commit into from

Conversation

bobbysciacchitano
Copy link

We're using body-parser to handle a large array however we hit 100 limit and got an unexpected result of the array being converted into an object.

This pull request makes the arrayLimit property configurable for use cases where there is a array larger than 100. Could potentially extend it further to validate and throw an exception if the limit is hit.

@Fishrock123 Fishrock123 added the pr label Oct 30, 2014
@dougwilson
Copy link
Contributor

Hi! Please read the discussion over at #55 (comment) while I look at your PR. The gist is unless the option will change the behavior of extended: false in some way, it won't be added.

@Fishrock123
Copy link
Contributor

See #60 also.. and yeah.

@dougwilson
Copy link
Contributor

A user has also created https://www.npmjs.org/package/urlencoded-request-parser as an immediate work-around, but really you should really consider why you are creating a HTML <form> with over 100 <input> fields. And if you're not, then why can't you use JSON? The urlencoded is purely for raw HTML <form> elements.

@dougwilson
Copy link
Contributor

So just for the sake on consistency, I'm seriously thinking of just setting arrayLimit to something even higher (I assumed, really, people would not be sending arrays > 100 items in the very inefficient and slow-to-parse urlencoded format). Can you share what the value you wanted to change it to was?

cc @adrian-ludwig @aez @ApeChimp @jcready

@bobbysciacchitano
Copy link
Author

Thanks, I'll take a look at a couple of other body parsing repos to look at possible options.

Our use case was a fairly simple UI which parses a json file, displays it in a table, allows the user to make changes and then save the file back on submit [to prevent direct manipulation of the JSON].

The list size was finalised at 101 items which is where we saw the issue.

@dougwilson
Copy link
Contributor

Ok. So perhaps I'll just set the limit to... 500? :)

@dougwilson
Copy link
Contributor

Actually, I know a way to support Infinity limit without introducing the security issue the limit was added to fix. Win for everyone!

dougwilson added a commit that referenced this pull request Oct 30, 2014
closes #42
closes #43
closes #46
closes #58
closes #60
closes #61
closes #65
@dougwilson
Copy link
Contributor

The code on master now addresses your use-case and similar ones automagically.

dougwilson added a commit that referenced this pull request Nov 22, 2014
closes #42
closes #43
closes #46
closes #58
closes #60
closes #61
closes #65
@expressjs expressjs locked and limited conversation to collaborators Nov 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants