Bug fix: VendorSpecificAttribute should supports multiple vendor attributes

[danjiewu]

A single VendorSpecificAttribute may contain multiple sub-attributes. All sub-attributes should be parsed in a loop.

[Copilot]

Pull request overview

Fixes parsing of RADIUS Vendor-Specific Attributes (VSA, type 26) so a single VSA payload can contain multiple vendor sub-attributes.

Changes:

• Reworked VendorSpecificAttribute parsing to extract the Vendor-Id and then parse repeated vendor sub-attributes into AttrNodes.
• Replaced the previous “single sub-attribute” shape (VendorCode, Length, Value) with a node-list model.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This change removes the previously public VendorCode, Length, and Value members from VendorSpecificAttribute. Other parts of the repo still reference them (e.g., RadiusPacketParser and RadiusCoreTests), so this will not compile as-is. Either update those call sites in this PR to use AttrNodes (loop over nodes), or keep backward-compatible properties (e.g., mapping to the first node) to avoid a breaking change.

Suggested change

	public byte VendorCode
	{
	get => AttrNodes.Count > 0 ? AttrNodes[0].VendorCode : (byte)0;
	set => GetOrCreateFirstNode().VendorCode = value;
	}
	public byte Length
	{
	get => AttrNodes.Count > 0 ? AttrNodes[0].Length : (byte)0;
	set => GetOrCreateFirstNode().Length = value;
	}
	public byte[]? Value
	{
	get => AttrNodes.Count > 0 ? AttrNodes[0].Value : null;
	set => GetOrCreateFirstNode().Value = value;
	}
	private VendorAttrNode GetOrCreateFirstNode()
	{
	if (AttrNodes.Count == 0)
	{
	AttrNodes.Add(new VendorAttrNode());
	}
	return AttrNodes[0];
	}

[Copilot]

The parsing loop can read past the end of contentBytes: while (offset < contentBytes.Length) allows entry even when only 1 byte remains, but the code reads both VendorCode and Length. Consider changing the loop condition to require at least 2 bytes remaining (type+length), and fail fast (or stop) if the buffer is truncated.

[Copilot]

This code allocates 1-byte arrays and uses Buffer.BlockCopy just to read a single byte for VendorCode/Length. You can read directly from contentBytes[offset] and increment offset, which reduces allocations and simplifies the loop.

Suggested change

	var vendorType = new byte[1];
	Buffer.BlockCopy(contentBytes, offset, vendorType, 0, 1);
	node.VendorCode = vendorType[0];
	offset++;
	var vendorLength = new byte[1];
	Buffer.BlockCopy(contentBytes, offset, vendorLength, 0, 1);
	node.Length = vendorLength[0];
	node.VendorCode = contentBytes[offset];
	offset++;
	node.Length = contentBytes[offset];

[Copilot]

VendorId is now a public mutable field. Elsewhere in the dictionary model (e.g., DictionaryVendorAttribute), these identifiers are readonly, which prevents accidental mutation after parsing. Consider making VendorId readonly again (or a property with a private setter) to preserve immutability and align with existing patterns.

Suggested change

	public uint VendorId;
	public readonly uint VendorId;

[Copilot]

There is an existing unit test for single-sub-attribute VSAs; this change introduces support for multiple sub-attributes but doesn't add/adjust tests to validate: (1) multiple vendor sub-attributes are parsed in order, and (2) malformed/truncated sub-attributes (bad length) are handled deterministically. Please extend the test suite accordingly.

[danjiewu]

VendorAttrNode length check