backbone-nested icon indicating copy to clipboard operation
backbone-nested copied to clipboard

Infinite recursion

Open sergiopereiraTT opened this issue 11 years ago • 8 comments

I had a bug in my code where, by accident, a reference to the window object was added as an attribute to a NestedModel. That caused an infinite recursion in _setAttr (and subsequent crash) because window has a few self-referencing properties like window.window, window.self, window._top, window.parent, etc.

Even though that was my fault, it revealed that (I think) this could happen with legitimately created recursions (like order.orderLines[0].order or pet.owner.pets[0], etc.)

Maybe NestedModel could try to keep track of the walked objects and avoid the infinite loop.

sergiopereiraTT avatar Sep 17 '13 17:09 sergiopereiraTT

Hmm, I wonder why this wasn't fixed by #80... mind submitting a failing test (that preferably doesn't involve window)?

afeld avatar Sep 17 '13 19:09 afeld

I should have mentioned the version I'm running: 1.1.2 from July 16th, which seems to include the fixes from #80. Maybe it only fails for window then.

sergiopereiraTT avatar Sep 17 '13 21:09 sergiopereiraTT

OK, I don't have a test but pasting this in a JS console seems to recreate the error:

var father = {name: 'bob'}
var child = {name: 'danny', father: father}
father.children = [child]
var family = new Backbone.NestedModel({lastName: 'Smith', head: father})
family.set({'head.name': 'Bobby'})
family.set({'head.name': 'Robert'})
family.set({'head.name': 'Rob'})
// Now try the same value again:
family.set({'head.name': 'Rob'})
// ==> RangeError: Maximum call stack size exceeded

sergiopereiraTT avatar Sep 18 '13 16:09 sergiopereiraTT

Any updates on this going to happen soon?

vkovalskiy avatar Jun 08 '14 22:06 vkovalskiy

@vkovalskiy unfortunately, both I and @afeld are pretty busy. I'll try to get to this in the following week or two.

gkatsev avatar Jun 09 '14 14:06 gkatsev

@gkatsev thanks for reply. However, I believe that the solution would be to simply forbid recursive models because you'll never know when it's ok to stop traversing. I believe the best that can be done here is informing the user that model has recursive attrbutes and throw, possibly stating the problem attribute name.

At least, there would be no browser doom state J

vkovalskiy avatar Jun 14 '14 20:06 vkovalskiy

I agree with @vkovalskiy. Throwing with a nice error message is better than trying to guess the best way to handle it.

sergiopereiraTT avatar Jun 16 '14 20:06 sergiopereiraTT

That may very well be the best way to handle it :grin:

gkatsev avatar Jun 16 '14 20:06 gkatsev