Fix XmlDeserializer when XML uses same tag in nested elements

[pumamood]

User description

Description

This pull request improves the robustness and correctness of XML deserialization in the XmlDeserializer class, particularly around handling nested elements and root element selection. It addresses bugs where nested lists were incorrectly flattened and where ambiguous root element names caused errors. The changes are thoroughly tested with new unit tests and supporting sample classes.

Improvements to XML deserialization logic:

• Updated root element selection to prefer the shallowest matching element, avoiding errors when multiple elements have the same name, and to check for direct child elements before searching descendants. This fixes issues where the wrong element was chosen as the root.
• Refined list deserialization to use only direct children of the container element (via Elements()) instead of all descendants (via Descendants()), preventing deeply nested items from being incorrectly included in top-level lists. [1] [2]
• Improved handling of generic type names and XML element name validity by introducing a check that skips invalid XML element names (such as those containing backticks).

Bug fixes and test coverage:

• Enhanced the mapping logic to correctly identify the container for generic collections, first checking if the root itself matches the property name before searching for a child element.
• Added comprehensive tests and sample classes to verify correct deserialization of nested lists, root element ambiguity, and container-based list handling, ensuring the fixes are robust and preventing regressions. [1] [2]* Fix XmlDeserializer nested element bugs

Purposee

• Fix Bug HandleListDerivative now uses Elements() on containers instead of Descendants()
• Fix Bug Deserialize RootElement selection prefers shallowest match
• Fix Bug RemoveNamespace filters null values

Checklist

• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

---

Auto-created Ticket

#2337

PR Type

Bug fix, Tests

---

Description

• Fix nested list deserialization using Elements() instead of Descendants()

• Improve root element selection to prefer shallowest matching element

• Filter null values in RemoveNamespace method

• Add validation for XML element names containing invalid characters

• Enhance container-based list handling with direct child element matching

---

Diagram Walkthrough

flowchart LR
  A["XML Document"] -->|"Parse & Find Root"| B["Root Element Selection"]
  B -->|"Prefer Shallowest Match"| C["Selected Root Element"]
  C -->|"Deserialize Lists"| D["HandleListDerivative"]
  D -->|"Find Container"| E["Container Element"]
  E -->|"Use Elements() for Direct Children"| F["Deserialized List Items"]
  G["RemoveNamespace"] -->|"Filter Null Attributes"| H["Cleaned XML"]

Loading

File Walkthrough

	Relevant files
Bug fix	XmlDeserializer.cs Refactor root selection and list deserialization logic      src/RestSharp.Serializers.Xml/XmlDeserializer.cs Modified root element selection to prefer shallowest matching element using OrderBy on ancestor count Changed from SingleOrDefault() to Element() first, then DescendantsAndSelf() with ordering Added IsValidXmlElementName() helper to skip invalid XML names containing backticks Enhanced HandleListDerivative to find container element first before searching descendants Updated list deserialization to use Elements() on containers instead of Descendants() Added null filtering in RemoveNamespace to prevent null attribute assignments Implemented multiple name variation matching strategies for direct children only +102/-23
Tests	NestedElementTestClasses.cs Add test sample classes for nested elements                            test/RestSharp.Tests.Serializers.Xml/SampleClasses/NestedElementTestClasses.cs Added Item class with recursive SubItems list property Added ItemContainer class to hold list of items Added ItemWithGroup and ItemGroup classes for nested group testing Added ItemsResponse class for complex nested structure testing +25/-0    XmlDeserializerTests.cs Add comprehensive tests for nested element fixes                  test/RestSharp.Tests.Serializers.Xml/XmlDeserializerTests.cs Added test for nested lists not including deeply nested items Added test for RootElement handling duplicate nested names without throwing Added test for RootElement preferring shallowest match Added test for RootElement trying direct child first Added test for container-based list using direct children only Added test for nested items also using direct children correctly +165/-1

---

[qodo-free-for-open-source-projects]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
🟢	No codebase code duplication found New Components Detected (Top 5): - Deserialize_Nested_List_Should_Not_Include_Deeply_Nested_Items - Deserialize_RootElement_Should_Not_Throw_On_Duplicate_Nested_Names - Deserialize_RootElement_Should_Prefer_Shallowest_Match - Deserialize_RootElement_Should_Try_Direct_Child_First - Deserialize_List_With_Container_Should_Use_Direct_Children_Only
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	The fallback logic reintroduces the original bug The suggestion points out that the fallback logic in HandleListDerivative uses Descendants(), which was the source of the original bug. This undermines the fix because if a container element isn't found, the deserializer reverts to the old, incorrect behavior. Examples: src/RestSharp.Serializers.Xml/XmlDeserializer.cs [364-419] if (container != null && container.HasElements) { // Try to match item elements by name variations var itemName = t.Name.AsNamespaced(Namespace); var directChildren = container.Elements(itemName).ToList(); if (!directChildren.Any()) { var lowerName = name?.ToLower(Culture).AsNamespaced(Namespace); directChildren = container.Elements(lowerName).ToList(); } ... (clipped 46 lines) Solution Walkthrough: Before: object HandleListDerivative(XElement root, ...) { // ... find container element ... XElement? container = FindContainerElement(root, ...); IList<XElement> elements; if (container != null) { // Correct behavior: use direct children elements = container.Elements(...).ToList(); // ... try name variations with Elements() ... } else { // Incorrect fallback: reintroduces the bug elements = root.Descendants(...).ToList(); // ... try name variations with Descendants() ... } PopulateListFromElements(t, elements, list); return list; } After: object HandleListDerivative(XElement root, ...) { // ... find container element ... XElement? container = FindContainerElement(root, ...); // If no container is found, use the root element itself. // This ensures we always look at direct children of *some* element. var source = container ?? root; // Always use direct children, removing the buggy fallback. IList<XElement> elements = source.Elements(...).ToList(); // ... try name variations with Elements() on `source` ... PopulateListFromElements(t, elements, list); return list; } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw where the fallback logic in HandleListDerivative reverts to using Descendants(), which reintroduces the original bug of flattening nested lists that this PR aims to fix.	High
General	Remove redundant case-insensitive string comparison Remove the redundant case-insensitive string comparison by deleting the second rootName.Equals check. src/RestSharp.Serializers.Xml/XmlDeserializer.cs [351-359] if (container == null && IsValidXmlElementName(propName)) { var rootName = root.Name.LocalName; - var propNameLower = propName.ToLower(Culture); - if (rootName.Equals(propName, StringComparison.OrdinalIgnoreCase) || - rootName.Equals(propNameLower, StringComparison.OrdinalIgnoreCase)) { + if (rootName.Equals(propName, StringComparison.OrdinalIgnoreCase)) { container = root; } } Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies a redundant conditional check, as StringComparison.OrdinalIgnoreCase makes the second check unnecessary, thus simplifying the code.	Low
More

[pumamood]

@dotnet-policy-service agree