HtmlAttributeCollection: ICollection<HtmlAttribute>.Clear() and ICollection<HtmlAttribute>.Remove(HtmlAttribute) methods forget to update Hashitems

[elgonzo]

1. Description

When taking a look at the fix for #461 out of curiosity, i noticed another two issues with HtmlAttributeCollection's ICollection<HtmlAttribute>.Clear() and ICollection<HtmlAttribute>.Remove(HtmlAttribute) implementations.

Both method implementations forget to update the Hashitems dictionary after directly altering the items collection.

html-agility-pack/src/HtmlAgilityPack.Shared/HtmlAttributeCollection.cs

Lines 154 to 157 in 127fe37

	void ICollection<HtmlAttribute>.Clear()
	{
	items.Clear();
	}

html-agility-pack/src/HtmlAgilityPack.Shared/HtmlAttributeCollection.cs

Lines 231 to 234 in 127fe37

	bool ICollection<HtmlAttribute>.Remove(HtmlAttribute item)
	{
	return items.Remove(item);
	}

The issue manifests and becomes easily observable when working with an HtmlAttributeCollection referenced by a ICollection<HtmlAttribute> variable without repeatedly re-obtaining the collection from the owning node's HtmlNode.Attributes property. (Emptying the internal items list by either clearing or removing the last element from it will cause the getter of the HtmlNode.Attributes property to create and provide a new HtmlAttributeCollection, thus masking the issue.)

2. Fiddle or Project

https://dotnetfiddle.net/D735Ot

Click to expand the source code

using HtmlAgilityPack;
using System;
using System.Collections.Generic;
using System.Linq;

namespace HtmlAttributeCollection_RemoveAndClearNotUpdatingHashItems
{
    class Program
    {
        static void Main(string[] args)
        {
            Remove_Issue_Demo();

            Console.WriteLine();
            Console.WriteLine();

            Clear_Issue_Demo();
        }


        static void Remove_Issue_Demo()
        {
            Console.WriteLine("Demo of AttributeCollection item removal issue");
            Console.WriteLine("----------------------------------------------");
            var (doc, attributes) = CreateHtmlDocument();

            Console.WriteLine($"Number of attributes in collection before removal: {attributes.Count}");
            var attrBeforeRemoval = attributes["class"];
            Console.WriteLine($"Attribute obtained through this[\"class\"] indexer before removal: {attrBeforeRemoval.Name}={attrBeforeRemoval.Value}");

            RemoveAnAttribute(attributes);

            Console.WriteLine($"Number of attributes in collection after removal: {attributes.Count}");
            var attrAfterRemoval = attributes["class"];
            Console.WriteLine($"Attribute obtained through this[\"class\"] indexer after removal: {attrAfterRemoval.Name}={attrAfterRemoval.Value}");



            static void RemoveAnAttribute(ICollection<HtmlAttribute> attributes)
            {
                var attr = attributes.First();
                attributes.Remove(attr);
            }
        }


        static void Clear_Issue_Demo()
        {
            Console.WriteLine("Demo of AttributeCollection clearing issue");
            Console.WriteLine("------------------------------------------");
            var (doc, attributes) = CreateHtmlDocument();

            Console.WriteLine($"Number of attributes in collection before clearing: {attributes.Count}");
            var attrBeforeClearing = attributes["class"];
            Console.WriteLine($"Attribute obtained through this[\"class\"] indexer before clearing: {attrBeforeClearing.Name}={attrBeforeClearing.Value}");

            Clear(attributes);

            Console.WriteLine($"Number of attributes in collection after clearing: {attributes.Count}");
            var attrAfterClearing = attributes["class"];
            Console.WriteLine($"Attribute obtained through this[\"class\"] indexer after clearing: {attrAfterClearing.Name}={attrAfterClearing.Value}");


            static void Clear(ICollection<HtmlAttribute> attributes)
            {
                attributes.Clear();
            }
        }


        static (HtmlDocument doc, HtmlAttributeCollection attributes) CreateHtmlDocument()
        {
            var html = @"<div class=""foobar"">Hello world!</div>";

            var htmlDoc = new HtmlDocument();
            htmlDoc.LoadHtml(html);
            return (htmlDoc, htmlDoc.DocumentNode.SelectSingleNode("//div").Attributes);
        }
    }
}

Note how after the attribute removal or clearing of the attribute collection the item count of the collection is being reported as 0, yet the removed attribute can still be accessed through the this[string name] indexer.

3. Any further technical details

• HAP version: 1.11.40
• NET 5.0

[JonathanMagnan]

Hello @elgonzo ,

Thank you for reporting,

We will look at it.

Best Regards,

Jon

---

Sponsorship
Help us improve this library

Performance Libraries
context.BulkInsert(list, options => options.BatchSize = 1000);
Entity Framework Extensions • Bulk Operations • Dapper Plus

Runtime Evaluation
Eval.Execute("x + y", new {x = 1, y = 2}); // return 3
C# Eval Function • SQL Eval Function

[JonathanMagnan]

Hello @elgonzo ,

Just to let you know that the fix for this issue will be part of next release (probably next week).

Best Regards,

Jon

[JonathanMagnan]

Hello @elgonzo ,

The v1.11.41 has been finally released.

Both methods are now calling methods that were handling correctly the Hashitems : d0dbd83

Best Regards,

Jon

---

Is this library useful to you? Please help us by becoming a sponsor to keep it alive and supported.

[elgonzo]

I can confirm that 1.11.41 fixes the issue! :)