tj / node-querystring

querystring parser for node and the browser - supporting nesting (used by Express, Connect, etc)
MIT License
455 stars 66 forks source link

Support objects with numeric keys. #62

Open abecciu opened 11 years ago

abecciu commented 11 years ago

@visionmedia I'd really like to get this merged. What do you think?

Tests are failing in master too for node 0.11.

tj commented 11 years ago

this case already works:

> require('./').parse('123=foo')
{ '123': 'foo' }

as for the quoted ones I'm not a huge fan of that personally, what's the use-case?

abecciu commented 11 years ago

@visionmedia I have a particular use-case where I need to have hashes with numeric keys stored in a parent hash.

Here's an example with qs master:

> qs.parse(qs.stringify({a: { '123': { a: 2 } } }))
{ a: [ { a: '2' } ] }

Rather than creating a hash of numeric keys, it treats the numbers as indexes of an array.

My solution is to escape the numeric keys in an object with single quotes, so that the parser is not confused with array indexes.

This is my version:

> qs.parse(qs.stringify({a: { '123': { a: 2 } } }))
{ a: { '123': { a: '2' } } }
abecciu commented 11 years ago

@visionmedia sorry for keep pushing, but I really want to get this merged. I think it's an edge case that should be handled correctly and of course it's super useful to me. :)

Anything else I should do?

Thanks!

tj commented 11 years ago

needs to support double quotes as well if we're going that route

abecciu commented 11 years ago

@visionmedia just added support for double quotes. Thanks!

kamikat commented 11 years ago

Have the same issue with qs parsing foo[1002]=bar&foo[9039]=baz which the number 1002 and 9039 are actually strings in application and expected be parsed to {foo: {'1002': 'bar', '9039': 'baz'} }.

abecciu commented 11 years ago

@shinohane this patch fixes that.

MitchellMcKenna commented 10 years ago

Related Issue: https://github.com/visionmedia/node-querystring/issues/14

buschtoens commented 10 years ago

Can you add an example, what's not working?

silkcom commented 10 years ago

I fail to see why this wouldn't just allow without quotes at all, and just treat them as objects if indices are set. Example:

qs.parse(qs.stringify({a: { 123: { a: 2 } } })) { a: { 123: { a: 2 } } }

or

qs.parse(qs.stringify({a: { '123': { a: 2 } } })) { a: { '123': { a: 2 } } }

silkcom commented 10 years ago

What would it take to get this merged in? BodyParser is based off this, and it's broken for cases such as having items[1] = on, and item[2] = on. Should be { items: {1: "on", 2: "on"} } but it's { items: [on, on] } instead. Which makes it completely worthless.

silkcom commented 10 years ago

looking over the failure here, it seems like something in node .11 removes hasOwnProperty which seems like it's a problem not related with this fix (though it's possible this fix is showing off the problem.

silkcom commented 10 years ago

nm, looking it over, master needs to be merged into this. Master has an extra hasOwnProperty call. I really think though that the quotes should be optional. The isint check should just be removed entirely and if there is something inside the []'s it's an object, if there's not it's an array

silkcom commented 10 years ago

created https://github.com/visionmedia/node-querystring/pull/98 which doesn't fix the quotes, but it does make {}'s the default