ItemFileReadStore fetchItemByIdentity Object.prototype identifier bug

[patrickzope]

If any identifier of the items provided to ItemFileReadStore? is also an Object.prototype property name, ItemFileReadStore? raises an erroneous collision error.

var data = {identifier: 'id', items: [{id: 'toString', value: 'blah'}]};
var store = new dojo.data.ItemFileReadStore({data: data});
store.fetchItemByIdentity({identity: 'toString'});

result:

Error: dojo.data.ItemFileReadStore?: The json data provided by the creation arguments is malformed. Items within the list have identifier: [id]. Value collided: [toString]

Obviously, there is only one item with the identifier 'toString'. This example is contrived, but on Firefox the same can happen with the identifier 'watch', since 'watch' happens to be a nonstandard method in Object's prototype.

Another issue is as follows: fetching an item with fetchItemByIdentity when providing an identifier that is also the name of an Object.prototype property name will return that property instead of null.

var data = {identifier: 'id', items: [{id: 'key', value: 'blah'}]};
var store = new dojo.data.ItemFileReadStore({data: data});
store.fetchItemByIdentity({identity: 'toString', onItem: function (item) {console.log(item);}});

will result in "toString()" being written to console.

Attached is a patch that (I believe) fixes this issue.

This affects Dojo 1.4.x and 1.5.

[patrickzope]

A patch for this bug

[JIm Fulton]

I suspect a better fix would be to write a dedicated dictionary type or dictionary functions that avoid this pitfall and use those (everywhere).

[patrickzope]

A Better patch for the bug

[Jared Jurkiewicz]

Do you have a CLA on file? I cannot look at your patch unless you have signed a CLA.

[patrickzope]

Replying to jaredj:

Do you have a CLA on file? I cannot look at your patch unless you have signed a CLA.

I do now.

[Jared Jurkiewicz]

Awesome. Thank you for the fix, then. I'll go through it later and get it in this week.

[Jared Jurkiewicz]

(In [22784]) Fixing issue with builtin names being identifiers. \!strict fixes #11624

[Jared Jurkiewicz]

Thanks for the patch and tests! Looks good. Committed.

[Jared Jurkiewicz]

In [26230]:

[IGNORE:RALLY,COPYRIGHT] - Fixing issue with prototype properties being called out as identifer. \!strict fixes #11624