Three.js: For...in loop in GLTFParser can cause gltf model load to fail

[SamiraAtMicrosoft]

Description of the problem

GLTFParser.prototype.loadMeshes and GLTFParser.prototype.loadNodes use for...in loops.

Since a for...in loop in JavaScript iterates over an objects properties, these can cause the parse to fail if there is a custom property added to the Array object elsewhere on the site. For example, in GLTFParser.prototype.loadMeshes there is:

for ( var name in primitives ) {
    var primitive = primitives[ name ];
    var geometry = geometries[ name ];

If another script does something like:

Array.prototype.newCustomProp = function (num) {
}

Then eventually name would equal newCustomProp and the attempt to find primitive/geometry will fail.

Three.js version

• [x] Dev
• [ ] r87
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

[Mugen87]

It is bad practice to extend the base prototypes of JavaScript.

Read this for more information: Maintainable JavaScript: Don’t modify objects you don’t own

Also see #11972

[mrdoob]

If another script does something like:

Array.prototype.newCustomProp = function (num) {}

If you ever see someone doing that, invite him to quite the javascript world please 😉

[mrdoob]

However, if primitives is an array, we shouldn't use for in.
/cc @donmccurdy

[RemusMar]

Array.prototype.newCustomProp = function (num) {}
If you ever see someone doing that, invite him to quite the javascript world please

The market is full of software "developers" these days.
Next time they might start to prototype the Java Island and the Java Sea.

[donmccurdy]

Most of these for...in loops are left from glTF 1.0, which used object syntax for a lot of things. We can replace them with traditional for loops yes.