macek/jquery-serialize-object

crash maybe due to incorrect array length

younes0 opened this issue · 7 comments

I got this form, with the following json structure:

[{"name":"slots[1395086400]","value":"0"},{"name":"slots[1395086400]","value":"1"},{"name":"slots[1395090000]","value":"0"},{"name":"slots[1395090000]","value":"1"},{"name":"slots[1395172800]","value":"0"},{"name":"slots[1395172800]","value":"1"},{"name":"slots[1395176400]","value":"0"},{"name":"slots[1395176400]","value":"1"},{"name":"slots[1395259200]","value":"0"},{"name":"slots[1395259200]","value":"1"},{"name":"slots[1395262800]","value":"0"},{"name":"slots[1395262800]","value":"1"},{"name":"slots[1395345600]","value":"0"},{"name":"slots[1395345600]","value":"1"},{"name":"slots[1395349200]","value":"0"},{"name":"slots[1395349200]","value":"1"},{"name":"slots[1395432000]","value":"0"},{"name":"slots[1395432000]","value":"1"},{"name":"slots[1395435600]","value":"0"},{"name":"slots[1395435600]","value":"1"},{"name":"slots[1395518400]","value":"0"},{"name":"slots[1395518400]","value":"1"},{"name":"slots[1395522000]","value":"0"},{"name":"slots[1395522000]","value":"1"}]"

when calling serializeObject() on this form , this is what I get logged :

Object {slots: Array[1395522001]}
  slots: Array[1395522001]
    1395086400: "1"
    1395090000: "1"
    1395172800: "1"
    1395176400: "1"
    1395259200: "1"
    1395262800: "1"
    1395345600: "1"
    1395349200: "1"
    1395432000: "1"
    1395435600: "1"
    1395518400: "1"
    1395522000: "1"
    length: 1395522001 // really?!
  __proto__: Array[0]
  __proto__: Object

Trying to access the slots property causes immediate crash with Chrome, not in Firefox, but they both have the incorrect length value.

Edit: the form consists of multiple checkboxes, and each has its checkbox with negative value so it can be sent even if it's unchecked. See: http://stackoverflow.com/questions/1809494/post-the-checkboxes-that-are-unchecked

Try it by yourself: http://pastebin.com/ZpzgbVS4

NB: I don't get this problem with Tobias Cohen' plugin

The "problem" here is that numerical keys are serialized as indices on an array. An illustration:

var arr = [];
arr[3] = 1;
arr.length; // 4

Arrays are going to behave the same, even if you use larger numbers

var arr = [];
arr[1395522000] = 1;
arr.length; // 1395522001

One solution for you would be to have data like

<input name="slots[0][id]" value="1395518400">
<input name="slots[0][value]" value="1">

<input name="slots[1][id]" value="1395522000">
<input name="slots[1][value]" value="1">

<!-- slot[2][id], slot[2][value], etc -->

As an alternative, I have considered changing how numerical keys are serialized using objects instead. That would make the code read something more like this

var arr = {};
arr[1395522000] = 1;
arr.length; // undefined

The primary difference here is we're sort of cheating using an object, but at least it doesn't blow up the "array" when we use large indices.

Advantages:

  • use large indices
  • API for accessing specific key stays the same: arr[n];

Disadvantages:

  • no .length property
  • iterating becomes more complex

However, to make this workable for everyone, I am considering opening up the serializing functions so that they could be overridden when you have corner cases such as your own.

Here's what the default would be. This is the current implementation

FormSerializer.factories.fixed = function(key, value, build) {
  return build([], key, value);
};

You could override this in your own code as such

FormSerializer.factories.fixed = function(key, value) {
  var obj = {};
  obj[key] = value;
  return obj;
};

Or to be a little more elegant, this would do the same thing

FormSerializer.factories.fixed = function(key, value, build) {
  return build({}, key, value);
};

To tie it all together, here's a complete implementation example you might use

<script src="jquery.js"></script>
<script src="jquery.serialize-form.js"></script>
<script>
  // override numeric key factory
  FormSerializer.factories.fixed = function(key, value, build) {
    return build({}, key, value);
  };

  // serialize form
  $("form").submit(function() {
    console.log( $(this).serializeObject() );
  });
</script>

Let me know your thoughts on this.

And to be fair, you're right, you won't get this "problem" with Tobias Cohen's code, but his code will also not serialize your nested keys. You'll just get {"slots[1395522000]":1} with which you will have to do additional parsing/manipulation on the server side.

Okay, I get it. The main problem is that I overrided numerical key arrays which may be a bad practice.
Your first solution looks fine to me, I'm more willing to change the HTML.

The design flaw in this case beeing the unsent unchecked checkoxes and the solution I set up, you could simply add this very particular case as a notice in the documentation instead, this would be enough. I personally never encountered other similar issue.

@younes0 thanks following up.

I'm glad the first solution seems to work for you.

I will still probably end up inverting the control of the factories so that people can override behavior where desired. It's a non-urgent feature, but I'll leave this issue open until I've implemented it.

Please let me know if I can help you in any other way.

That sounds good, thanks.

YOzaz commented

@macek please bring back possibility to overwrite factories, as I'm facing same issue as well. Lots of the frameworks makes life much easier if you're using CRUD object ID as form data key.
I understand that it doesn't comply with W3C directive(s), however - post-processors like PHP do not generate additional NULL values in arrays when submitting a form.

Alternatively, maybe, you can just add a config flag to force casting arrays to objects? Something like:

// foo[]
if (patterns.push.test(k)) {
   var idx = incrementPush(root.replace(/\[\]$/, ''));
   value = build( config['cast_arrays'] ? {} : [], idx, value);
}
// foo[n]
else if (patterns.fixed.test(k)) {
   value = build( config['cast_arrays'] ? {} : [], k, value);
}

Configuration can be done through params in serializeObject() function, or on global prototype I suppose.