HtmlAttributeCollection: Implementation for ICollection<HtmlAttribute>.Remove(HtmlAttribute) violates interface contract

[elgonzo]

1. Description

When trying to remove an attribute instance that's NOT in a HtmlAttributeCollection through the explicit interface implementation for ICollection<HtmlAttribute>.Remove(HtmlAttribute), it is expected that the method returns false.

However, as implemented in HAP 1.11.41 (implementing a fix for #463), it throws an exception instead of returning false.

The ICollection<HtmlAttribute>.Remove(HtmlAttribute) implementation in HtmlAttributeCollection thus violates the ICollection<T>.Remove(T item) interface contract. The contract mandates (https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.icollection-1.remove?system-collections-generic-icollection-1-remove(-0)):

Returns true if item was successfully removed from the ICollection<T>; otherwise, false.
This method also returns false if item is not found in the original ICollection<T>.

2. Exception

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at HtmlAgilityPack.HtmlAttributeCollection.Remove(HtmlAttribute attribute) in C:\Repos\HtmlAgilityPack\HtmlAgilityPack.Shared\HtmlAttributeCollection.cs:line 343
   at HtmlAgilityPack.HtmlAttributeCollection.System.Collections.Generic.ICollection<HtmlAgilityPack.HtmlAttribute>.Remove(HtmlAttribute item) in C:\Repos\HtmlAgilityPack\HtmlAgilityPack.Shared\HtmlAttributeCollection.cs:line 233
   at HtmlAttributeCollection_RemoveAndClearNotUpdatingHashItems.Program.Main(String[] args)

3. Fiddle or Project

dotnetfiddle: https://dotnetfiddle.net/lKA3kg

Click to expand the source code

var html = @"<div class=""foobar"" name=""doctor_doctor"">Hello world!</div>";

var htmlDoc = new HtmlDocument();
htmlDoc.LoadHtml(html);
var divNode = htmlDoc.DocumentNode.SelectSingleNode("//div");

var attr = divNode.Attributes["class"];

ICollection<HtmlAttribute> attributeCollectionAsICollection = (ICollection<HtmlAttribute>) divNode.Attributes;

//
// Remove an attribute instance that is in the Attributes collection
// Expected return value: true
//
var res1 = attributeCollectionAsICollection.Remove(attr);
Console.WriteLine($"Return value after successful removal: {res1}   (should be true)");

//
// Now trying to remove an attribute instance that is not in the Attributes collection (anymore)
// Expected return value: false
//
var res2 = attributeCollectionAsICollection.Remove(attr);
Console.WriteLine($"Return value after successful removal: {res2}   (should be false)");

4. Any further technical details

• HAP version: 1.11.41
• .NET 6.0

[JonathanMagnan]

Hello @elgonzo ,

Thank you for reporting, you are right. We have been a little bit too lazy on this fix.

We updated the method to return false correctly when the item is not or doesn't exists in the latest version v1.11.42

Best Regards,

Jon

[elgonzo]

I can confirm that the issue is fixed in 1.11.42. :-)