qs' depth

[expressjs]

qs' depth

[AndersonZacharyT]

@dougwilson Hey Doug--thanks for all of your work! I am facing this issue, using Express/LoopBack, and even though my Express is > 1.11 I seem to have hit this wall... anything coming to mind?

More info: My code is hitting this within Express/lib/utils:

function parseExtendedQueryString(str) {
  return qs.parse(str, {
    allowPrototypes: true
  });
}

and as you can see the options for qs are hard-coded, but adding a limit:10 solves all of our problems...

[dougwilson]

Hi @AndersonZacharyT , I don't think that file has anything to do with the body-parser module. If you are talking about the query string in Express, then you can do the following:

app.set('query parser', function (str) {
  return qs.parse(str, {
    allowPrototypes: true,
    limit: 10
  });
})

[dougwilson]

So the general answer is probably not, but nothing is off the table :) But before I continue on, I was wondering what was the reason you choose to use the urlencoded format vs JSON?

[chiel]

Well, using this in a cms which I am writing which has some fairly complicated/nested forms going on, and they are basically just submits to other pages. I am moving towards submitting them over XHR though, so doing that as json would fix this issue, basically, but requires more effort.

That's why I figured I'd ask to see about support, or whether there was something obvious I'd missed.

[dougwilson]

Ok, makes sense, I just wanted to know :) So, if you were to pick a depth for what you are seeing, what would it be?

[chiel]

Nothing too major, probably around 10. But in all honesty, it might be more logical for me to simply switch over to json if nobody else has ever run into this issue. It does seem like a bit of an edge-case and I'm sure there's a good reason for the default depth setting being only 5.

It's just cause it's a recursive navigation module so there's a potential of inputs in inputs in inputs (endlessly :p).

So it's up to you really. Either way - thanks for taking the time to reply!

[dougwilson]

Gotcha. Basically, it sounds like if say we were to make it 10, you could very well suddenly need 11, etc.

As for the reason, "5" itself is semi-arbitrary, but in general, the parsing of this weird structure is pretty slow and the deeper it is, the slower it'll get if allowed. I'm also noodling a bit here to get you a good non-JSON (i.e. plain ole HTML forms) answer.

[chiel]

Yea that's the thing, there are good reasons not to increase the depth (speed probably being the main one), so it seems silly to increase it willy-nilly. As you point out, it could very well be that a higher depth is suddenly needed.

Since my cms leans heavily on javascript already (i.e., it doesn't work without it :p), I should probably just switch to submitting forms over XHR as json, and use the json bodyparser middleware.

I'm struggling to find a solution that would potentially work with plain old forms with heavy nesting. Guess you're shit out of luck in that case haha.

Cheers again for the thoughts.

[joshdickson]

+1 I ran into this issue this morning and took forever to trace it back to this. 5 is completely arbitrary and I never found it in docs. Frustrating.

What is the suggested work around for this? I see PR pending with this but don't see a comment on when this might be out.

[dougwilson]

I'm' planning on removing the depth limit completely with 1.11.0. It's not documented anywhere because we don't set the limit anywhere in this module.

[chiel]

Yup, 5 is simply the default set by the qs module itself. Removing it probably gets rid of people seeing this behaviour as a bug though.

[joshdickson]

What's the timeframe for 1.11? I see tracker for 2.0 but not 1.11

[dougwilson]

Probably tonight (US time)

[joshdickson]

That would be outstanding, thank you.

[dougwilson]

So I just wanted to update you that the "probably tonight" may end up slipping to a definitely Friday night, I'm sorry :(

[chiel]

Cheers for the feedback and thanks for your hard work. :)

[joshdickson]

It had to be last night! You've broken our code base!

Tonight is just fine, thanks again :)

[joshdickson]

Cheers thanks

[dougwilson]

Version 1.11.0 doesn't allow you to set the depth, but there is no longer a limit, so it's not necessary to control the value.

[joshdickson]

Already have it in QA and works great for me, thanks again.

[dougwilson]

Great, no problem :) I resisted lifting that for a while because the qs authors say to "keep it at a reasonable small number", but I did a bunch of performance testing with this change and I don't see any reason why it cannot be Infinity.