Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#13213 closed defect (fixed)

dojo.Stateful internal functions name collision with holding object properties

Reported by: nvd_ai61 Owned by: Kris Zyp
Priority: high Milestone: 1.8
Component: Core Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

A dangerous usage of dojo.Stateful is to let user add properties with the same key as dojo.Stateful functions, i.e., set, get, watch, postscript.

Let's say I keep a list of products and their prices and one item is a 'watch':50. If I set this item in my stateful object, in dojo.Stateful line 69 in dojo 1.7.0b1 does:

this[name] = value;

which sets this[ 'watch' ]=50, which overrides the dojo.Stateful.watch function!

such collision should be avoided.

Change History (6)

comment:1 Changed 8 years ago by bill

Component: GeneralCore
Owner: set to Kris Zyp

Seems like this is just an inherent limitation with the design of Stateful. The way around it would be to keep all the properties in a sub-hash (like Stateful._properties), although that would be a backwards-compatibility breaking change. (IIRC Stateful was added in 1.6.)

comment:2 Changed 8 years ago by ben hockey

i may be mistaken but i think it's also intentional that the properties are on the object so that you can directly reference them as statefulInstance.foo rather than it being necessary to use statefulInstance.get('foo');

this makes clashes possible but i think the convenience/performance tradeoff of direct property access outweighs the concern for clashes.

comment:3 Changed 8 years ago by nvd_ai61

I agree with neonstalwalt. the convenience and performance of direct access to properties are great and I won't want to lose that by using a sub-hash.

However the clashes are still not welcome, particularly because dojo.Stateful does not always keep the properties that the developer is aware of. For instance, I am using dojo.Stateful as the base class for all of my model classes in a MVC architecture. Therefore, the Stateful objects are being filled by the keys that are entered by the user in the view layer. Validating against clashes in the view(i.e., avoiding user to enter a name such as 'watch') is just wrong. The same can happen if Stateful is being fed from a backend database, e.g., there is a 'watch' product in my online store to sell.

I suggest one of the following solutions :

1- to highlight these clashes in dojo.Stateful document and leave it as it is.

2- name all the functions of Stateful to start with '_' character (i.e., _set, _get, _watch) and avoid adding properties with the underscored keys.

For my application, I have modified Stateful according to the second solution.

comment:4 Changed 8 years ago by Kris Zyp

You don't have to use the instance to retrieve the method. It should be perfectly fine to do:

myStateful = new dojo.Stateful(); add a colliding property myStateful.watch = "foo"; call watch without collision dojo.Stateful.prototype.watch.call(myStateful, callback); call set safely dojo.Stateful.prototype.set.call(myStateful, "watch", "bar");

comment:5 Changed 7 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

I've updated the documentation to warn about these issues.

comment:6 Changed 7 years ago by bill

Milestone: tbd1.8
Note: See TracTickets for help on using tickets.