_removeCurrentsFromItems modifies a list it is iterating over with a for(i=0...) style loop

[Skaffen]

I posted a comment on this on the closed ticket #5985 but I guess no-one saw it.

The outer of the loop is: for (var i=0; i < this.items.length; i++) {

Which has an inner loop which does an iteration and can do:

if(list[j] == this.items[i].item[0])

this.items.splice(i,1);

So the meaning of items[i] will change if the splice goes off, which means that this loop can skip doing some of the comparisons it should be doing and potentially even have this.items[i] cause an error if the list shrinks below what the current [i] value is. I've no idea really what _removeCurrentsFromItems is for, so I can't cook up a test case - hopefully someone who knows more about the purpose of this function can cook up a test case which will demonstrate the bug.

The fix to the code would be iterate backwards like other similar code does, e.g. for (var i = this.items.length - 1; i >= 0; i--), and to break out of the inner 'j' loop if a match is found that triggers this.items.splice(i,1).

[mikesherov]

Thanks for contributing! The code in reference no longer exists as is. Since there's no test case (as was asked for), I need to mark this as notabug.