Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#11624 closed defect (fixed)

ItemFileReadStore fetchItemByIdentity Object.prototype identifier bug

Reported by: patrickzope Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.6
Component: Data Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

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.

Attachments (2)

ItemFileReadStore.patch (2.8 KB) - added by patrickzope 9 years ago.
A patch for this bug
BetterItemFileReadStore.patch (4.2 KB) - added by patrickzope 9 years ago.
A Better patch for the bug

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by patrickzope

Attachment: ItemFileReadStore.patch added

A patch for this bug

comment:1 Changed 9 years ago by 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).

Changed 9 years ago by patrickzope

A Better patch for the bug

comment:2 Changed 9 years ago by Jared Jurkiewicz

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

comment:3 in reply to:  2 Changed 9 years ago by 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.

comment:4 Changed 9 years ago by Jared Jurkiewicz

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

comment:5 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

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

comment:6 Changed 9 years ago by Jared Jurkiewicz

Milestone: tbd1.6

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

comment:7 Changed 8 years ago by Jared Jurkiewicz

In [26230]:

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

Note: See TracTickets for help on using tickets.