tj / node-querystring

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

numeric keys as arrays #14

Open tj opened 13 years ago

tj commented 13 years ago
console.log(qs.parse('_method=put&data%5Bprojects%5D%5B0%5D%5Bname%5D=Stratus+Editor23&data%5Bprojects%5D%5B0%5D%5Bpath%5D=%2Fhome%2Fx%2Fnode%2Fstratus%2Fstratus&data%5Bprojects%5D%5B1%5D%5Bname%5D=Menu66&data%5Bprojects%5D%5B1%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Fmenu66&data%5Bprojects%5D%5B2%5D%5Bname%5D=499+(New)&data%5Bprojects%5D%5B2%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Fe499&data%5Bprojects%5D%5B3%5D%5Bname%5D=Utopia+Engine&data%5Bprojects%5D%5B3%5D%5Bpath%5D=%2Fhome%2Fx%2Fapps%2Futopia_engine&data%5Bprojects%5D%5B4%5D%5Bname%5D=499+(Old)&data%5Bprojects%5D%5B4%5D%5Bpath%5D=%2Fhome%2Fx%2Frails%2Fe499&data%5Bprojects%5D%5B5%5D%5Bname%5D=Stratus+2.0&data%5Bprojects%5D%5B5%5D%5Bpath%5D=%2Fhome%2Fx%2Fnode%2Fstratus%2Fstratus-2').data);

currently gives you:

  { projects: 
     { '0': 
        { name: 'Stratus Editor23',
          path: '/home/x/node/stratus/stratus' },
       '1': { name: 'Menu66', path: '/home/x/apps/menu66' },
       '2': { name: '499 (New)', path: '/home/x/apps/e499' },
       '3': 
        { name: 'Utopia Engine',
          path: '/home/x/apps/utopia_engine' },
       '4': { name: '499 (Old)', path: '/home/x/rails/e499' },
       '5': 
        { name: 'Stratus 2.0',
          path: '/home/x/node/stratus/stratus-2' } } }
Marak commented 13 years ago

Ohh heh, here it is!

aredridel commented 13 years ago

Ooh!

Delapouite commented 13 years ago

qs.parse('user[0]=tj&user[1]=TJ'); // { user: [ 'tj', 'TJ' ] }

qs.parse('user[1]=tj&user[2]=TJ'); // { user: [ 'tj', 'TJ' ] } but should be { user: {'1': 'tj', '2': 'TJ'} }

tj commented 13 years ago

@Delapouite hmm tough call, that could be a sparse array as well

aredridel commented 13 years ago

It should come out as a sparse array, I'd think.

tj commented 13 years ago

yeah that's what I would expect

troyk commented 12 years ago

This is related, not sure if you want a separate issue, but it's nice to drop the key all together when the ui allows the user to 'add another' functionality of nested form data.

console.log(qs.parse('workout%5Bname%5D=Stud&exercises%5B%5D%5Bsets%5D=1&exercises%5B%5D%5Bname%5D=Push+Up&exercises%5B%5D%5Bsets%5D=3&exercises%5B%5D%5Bname%5D=Pull+Up&exercises%5B%5D%5Bsets%5D=3&exercises%5B%5D%5Bname%5D=Situp+Up');

gives you:

{ workout: { name: 'Stud' },
  exercises: [ '1', 'Push Up', '3', 'Pull Up', '3', 'Situp Up' ] }

where (especially peeps used to rails) expect:

{ workout: { name: 'Stud' },
  exercises: 
   [ { sets: '1', name: 'Push Up' },
     { sets: '3', name: 'Pull Up' },
     { sets: '3', name: 'Situp Up' } ] }
tj commented 12 years ago

@troyk agreed, that looks like a bug to me

troyk commented 12 years ago

@visionmedia I was playing with it last night and gave up, but I'll go check it out again and see if I can muster up a pull request.

troyk commented 12 years ago

Ok, so after messing with the current implementation, I decided to take a crack at porting Rack's to javascript, as I'm sure the Rack implementation is the right feature set. But it's getting clobbered on these tests:


    qs.parse('a[>=]=23')
      .should.eql({ a: { '>=': '23' }});

    qs.parse('a[<=>]==23')
      .should.eql({ a: { '<=>': '=23' }});

    qs.parse('a[==]=23')
      .should.eql({ a: { '==': '23' }});

and a few more. Shouldn't 'a[>=]=23' really be 'a%5B%3E%3D%5D=23' ? If so, I'll go add the encoding to the tests to and keep moving forward. If I'm missing something, just let me know. Below is where I'm at, let me know what you think about proceeding down this path.


function normalizeParams(params, name, v) {
  var matches = name.match(/^[\[\]]*([^\[\]]+)\]*/);
  if (!matches) return;

  if ('undefined' == typeof v) v = '';

  var k     = matches[1]
    , after = name.substr(k.length);

  if (after == '') {
    params[k] = v;
  } else if (after == '[]') {
    if (!params[k]) params[k] = [];
    params[k].push(v);
  } else if (matches = after.match(/^\[\]\[([^\[\]]+)\]$/) || after.match(/^\[(\d{1,9})?\](.+)$/)) {
    if (!params[k]) params[k] = [];
    var child_key = matches[1]
      , param_len = params[k].length 
      , last_val  = params[k][param_len - 1];

    if ('object' == typeof last_val && !Array.isArray(last_val) && !last_val[child_key]) {
      normalizeParams(last_val, child_key, v);
    } else if (child_key == String(param_len)) {
      params[k].push(v);
    } else {
      params[k].push(normalizeParams({},child_key,v));
    }
  } else {
    params[k] = normalizeParams(params[k] || {}, after, v);
  }
  return(params);
}

exports.parse = function(str) {
  var params = {}
  if (null == str || '' == str) return params;

  String(str).split('&').forEach(function(pair){
    pair = decodeURIComponent(pair.replace(/\+/g, ' ')).split('=',2);
    console.log(pair);
    normalizeParams(params,pair[0],pair[1]);
  });

  return params; 
}
spikebrehm commented 11 years ago

IMO these should stay as objects with string keys, to match Rack/Rails behavior. We're running into an issue using Express to do some proxying of requests from client to Rails app. connect.bodyParser() uses qs and causes params like nodes[500]=159&nodes[501]=39 to turn into:

{"nodes": ["159", "39"]}

Whereas Rack/Rails sees the same params as:

{"nodes": {"500": "159", "501": "39"}}

Not that "because the Ruby community does it" is a great reason, but why break from convention?

spikebrehm commented 11 years ago

I guess a (gross) workaround is to do something like this:

<input type="hidden" name="nodes[_]" value="">