roman01la/JSONFormData

Form input types are not being enforced when data is converted into json

Closed this issue · 10 comments

Hello.
I'm trying to submit a form using the JSON form data and I noticed that the input form type is not being enforced when type is text and the value is a number.
In the beginning I thought a it could be a bug but then I've checked the code and I noticed that there´s this block of code that parses the value into an integer. A few lines below the equality comparator is used (where I would think of the strict equality comparator instead) to check whether to set the coercedValue or the actual value.

JSONFormData.prototype.putFormData = function(path, value) {
    var self = this,
      accessorRegex = /\[(.*?)]/g,
      matches,
      accessors = [],
      firstKey = path.match(/(.+?)\[/),
      coercedValue = parseInt(value, 10);

    if(firstKey === null) {
      firstKey = path;
    } else {
      firstKey = firstKey[1];
    }

    /* use coerced integer value if we can */
    value = (coercedValue == value) ? coercedValue : value;

Can anyone explain why this is being done? =)
Cheers, Eliseu

I think we can ask @dariushoule

Heya guys,

See Example 2 in the spec for an idea of why this is being done: http://darobin.github.io/formic/specs/json/


<form enctype='application/json'>
  <input type='number' name='bottle-on-wall' value='1'>
  <input type='number' name='bottle-on-wall' value='2'>
  <input type='number' name='bottle-on-wall' value='3'>
</form>

// produces
{
  "bottle-on-wall":   [1, 2, 3]
}

I think you are right though -- there is a problem with this. The better way to handle it would be to look at the input type rather than attempting to coerce any input. I'll create a PR later today to refactor this.

PR to address this is open: #5

Great, thanks!

Input with type of number returns a value of type string. So what's the reason to check for number type? Also everything is encodes to JSON string anyway.

Hi Roman,
You can see the difference as follows:

Without respecting type:
{ bottle-on-wall: [ "1", "2" ,"3" ] }

With respecting type (this is how it works currenty):
{ bottle-on-wall: [ 1, 2, 3 ] }

It affects how the JSON blob is decoded. To stay true to the spec the decoder should populate this array with integer values when decoding over strings - since the input types are number:
<input type='number' name='bottle-on-wall' value='1'>

This is why I put the patch in to check what type the input has and parse the int if it is number:
value = (type === 'number') ? parseInt(value, 10) : value;

@dariushoule I see, ok, the spec has the example. Also, there's related bug on spec repo. Looks like we should respect number type for range type as well. Not sure about mentioned boolean value for type="checkbox" and timestamp format for type="date", both are not valid value formats according to States of the type attribute spec.

I'm working on making the polyfill more accessible via NPM and Bower, here's umd branch with latest changes.

@dariushoule I've managed to replace a weak comparison here with another approach, using isNan.

As a side note: looks like Blink is starting implementation of Streams API (some stuff is already in Chrome Canary). So this might allow to stream large base64 encoded files without memory issues on browser side, but also it may require to develop another parser on backend.