Fix issue 12495: Infinite loop in ToolStripItemCollection.AddRange (#12513)

Fixes #12495 and #4454

Proposed changes
Early return for empty collection
Converts the ToolStripItemCollection into a temporary array (using ToArray()) to avoid modifying the original collection during iteration. This ensures that items can be safely added to the new collection without causing exceptions or unintended behavior, especially when items are removed from the original collection if they have a different owner control.
Add unit test for issue Infinite loop in ToolStripItemCollection.AddRange #12495 case
Regression?
Yes
Test methodology
Test fixing for GH issues: 12495 and 4454 manually
Unit test
This commit is contained in:
v-olzhan 2024-11-27 00:14:44 +00:00 коммит произвёл GitHub
Родитель d629bcefc2
Коммит c56379f4d3
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
2 изменённых файлов: 37 добавлений и 5 удалений

Просмотреть файл

@ -141,16 +141,23 @@ public class ToolStripItemCollection : ArrangedElementCollection, IList
throw new NotSupportedException(SR.ToolStripItemCollectionIsReadOnly);
}
// Return early if the collection is empty.
if (toolStripItems.Count == 0)
{
return;
}
// ToolStripDropDown will look for PropertyNames.Items to determine if it needs
// to resize itself.
using (new LayoutTransaction(_owner, _owner!, PropertyNames.Items))
{
for (int i = 0; i < toolStripItems.Count; i++)
// Create a temporary array to avoid modifying the original collection during iteration.
// Items will be removed from toolStripsItems collection when they are added to this collection
// if they had a different owner control.
var itemsToAdd = toolStripItems.InnerList.ToArray();
foreach (ToolStripItem item in itemsToAdd)
{
// Items are removed from their origin when added to a different owner.
// Decrement the index to always add the items from index 0 which will preserve
// the original order and avoid a pesky ArgumentOutOfRangeException.
Add(toolStripItems[i--]);
Add(item);
}
}
}

Просмотреть файл

@ -117,6 +117,8 @@ public class ToolStripItemCollectionTests
toolStripDropDownButton.DropDownItems.Add("b");
toolStripDropDownButton.DropDownItems.Add("c");
contextMenuStrip.Items.AddRange(toolStripDropDownButton.DropDownItems);
Assert.Empty(toolStripDropDownButton.DropDownItems);
Assert.Equal(3, contextMenuStrip.Items.Count);
// Validate order.
@ -125,6 +127,29 @@ public class ToolStripItemCollectionTests
Assert.Equal("c", contextMenuStrip.Items[2].Text);
}
[WinFormsFact]
public void ToolStripItemCollection_AddRange_ToolStripItemCollection_SameOwner_Success()
{
using ToolStrip toolStrip = new();
// Create a ToolStripItemCollection with 2 items
ToolStripItemCollection itemCollection = new(toolStrip,
[
new ToolStripButton("Button 1"),
new ToolStripButton("Button 2")
]);
toolStrip.Items.Count.Should().Be(0);
toolStrip.Items.AddRange(itemCollection);
itemCollection.Count.Should().Be(2);
toolStrip.Items.Count.Should().Be(2);
toolStrip.Items[0].Text.Should().Be("Button 1");
toolStrip.Items[1].Text.Should().Be("Button 2");
}
private ToolStripItem[] CreateToolStripItems(params (string Key, string Name)[] items) =>
items.Select(item => new ToolStripButton(item.Name) { Name = item.Key }).ToArray();