`jQuery.extend(deep, [])` causes infinite loop with self-referential attributes

[[email protected]…]

I posted this as a pull request on GitHub ​https://github.com/jquery/jquery/pull/863. It usually creates an issue for it, didn't realize GitHub issues were disabled for jQuery, so reposting here.

When jQuery tries to deep clone the object, it creates an infinite loop:

​jquery/src/core.js#L304

The important part is:

for ( name in options ) {
  src = target[ name ];
  copy = options[ name ];

In Ember there is a setting to turn on accessors and to extend native objects. This creates a method on an array like array['[]'], which just returns itself (this):

​ember-runtime/lib/mixins/enumerable.js#L623

I think there are two bugs here:

1. That part of $.extend should be using hasOwnProperty, but I'm not entirely sure about this. It returns false on accessors.
2. I've added if (options === copy) continue, which wasn't covered with just if (target === copy) continue, which is the problem with a property pointing to itself.

Here is where I ran into the error in some code:

[].hasOwnProperty('length') // true
[].hasOwnProperty('[]') // false
var array = []
array['[]'] === array // true

Would adding this fix it you think?

for ( name in options ) {
  if (options.hasOwnProperty(name)) {
    src = target[ name ];
    copy = options[ name ];
  // ...

It looks like this is probably a jQuery bug but it's getting late so I can't quite tell.

Try running $.extend(true, []) in the web console with Ember.USE_ACCESSORS = true, and it should create an infinite loop.

(came across this using the jquery-file-upload plugin with ember accessors, it doesn't work b/c the ajax call it's making passes the files array into the $.extend method ​right here)

[[email protected]…]

Here is a jsFiddle demonstrating: ​http://jsfiddle.net/viatropos/hxgEC/

[Rick Waldron]

​http://docs.jquery.com/Won't_Fix#Object.prototype_Issues

[Rick Waldron]

Duplicate of #2721.

[anonymous]

Hey, there are two issues here, see the jsFiddle: ​http://jsfiddle.net/viatropos/hxgEC/3/

// infinite loop
$('#one').click(function() {
  var array = [];
  // self referential attribute
  array.self = array;

  try {
    $.extend(true, array);
  } catch (error) {
    _log(error.toString());
    alert(error);
  }
});

That is an even more significant problem

[dmethvin]

Saw this earlier but haven't had time to look at it in detail.

Is there a general way to fix the infinite loop though? The cycle could occur anywhere within the data structure, and it could traverse through several other objects that end up back at one already visited. You've picked an easy case.

The effect of $.extend()ing custom objects, or native objects with augmented properties, isn't something we've ever defined because its use case was for extending plain objects and arrays.

[anonymous]

You're right, the problem could get complicated. But, the part of the pull request adding options === copy solves it for the simple case: ​https://github.com/viatropos/jquery/commit/3e82e53137049908b8c461708e04bd4ceff48986#L0R314

By making that change, I was able to use accessors, which happened to use self referential attributes, in the jQuery-File-Upload plugin, which passes the FileList array as a value in the options hash to $.ajax. By using the Ember.USE_ACCESSORS, that messes with the for loop like you guys pointed to in the "Won't Fix" bugs, but the real problem was that the accessor just caused a self referential attribute to become "visible" so to speak. So, solving this simple case would be very helpful.

[gibson042]

Replying to dmethvin:

Is there a general way to fix the infinite loop though? The cycle could occur anywhere within the data structure, and it could traverse through several other objects that end up back at one already visited. You've picked an easy case.

There is a ​general way, but it's historically been deemed too heavy (both in code size and performance) for us to implement.

[dmethvin]

@gibson042, true, I had ignored that as a solution given its cost, and since most uses don't encounter it I think we should continue to ignore it. :-)

As for this specific case it's a bit cheaper to implement since it doesn't require a list.