diff --git a/src/RestSharp.Serializers.Xml/XmlDeserializer.cs b/src/RestSharp.Serializers.Xml/XmlDeserializer.cs index 99959b221..4b5a43d35 100644 --- a/src/RestSharp.Serializers.Xml/XmlDeserializer.cs +++ b/src/RestSharp.Serializers.Xml/XmlDeserializer.cs @@ -40,8 +40,16 @@ public class XmlDeserializer : IXmlDeserializer, IWithRootElement, IWithDateForm var root = doc.Root; var rootElement = response.RootElement ?? RootElement; - if (rootElement != null && doc.Root != null) - root = doc.Root.DescendantsAndSelf(rootElement.AsNamespaced(Namespace)).SingleOrDefault(); + if (rootElement != null && doc.Root != null) { + var namespacedRoot = rootElement.AsNamespaced(Namespace); + // Prefer the shallowest match to avoid nested elements with the same name + root = namespacedRoot != null + ? doc.Root.Element(namespacedRoot) + ?? doc.Root.DescendantsAndSelf(namespacedRoot) + .OrderBy(e => e.Ancestors().Count()) + .FirstOrDefault() + : null; + } // autodetect xml namespace if (Namespace.IsEmpty()) @@ -76,10 +84,15 @@ static void RemoveNamespace(XDocument xdoc) { ? new(XNamespace.None.GetName(a.Name.LocalName), a.Value) : a ) + .Where(a => a != null) ); } } + static bool IsValidXmlElementName(string name) => + // Generic type names contain backtick (e.g., "List`1") which is invalid in XML element names + !name.Contains('`'); + protected virtual object Map(object x, XElement? root) { var objType = x.GetType(); var props = objType.GetProperties(); @@ -177,12 +190,12 @@ protected virtual object Map(object x, XElement? root) { } } else if (type.IsEnum) { - var converted = type.AsType().FindEnumValue(value.ToString()!, Culture); + var converted = type.AsType().FindEnumValue(value.ToString(), Culture); prop.SetValue(x, converted, null); } else if (asType == typeof(Uri)) { - var uri = new Uri(value.ToString()!, UriKind.RelativeOrAbsolute); + var uri = new Uri(value.ToString(), UriKind.RelativeOrAbsolute); prop.SetValue(x, uri, null); } @@ -191,8 +204,8 @@ protected virtual object Map(object x, XElement? root) { } else if (asType == typeof(DateTime)) { value = DateFormat.IsNotEmpty() - ? DateTime.ParseExact(value.ToString()!, DateFormat!, Culture) - : DateTime.Parse(value.ToString()!, Culture); + ? DateTime.ParseExact(value.ToString(), DateFormat!, Culture) + : DateTime.Parse(value.ToString(), Culture); prop.SetValue(x, value, null); } @@ -219,24 +232,35 @@ protected virtual object Map(object x, XElement? root) { } } else if (asType == typeof(decimal)) { - value = decimal.Parse(value.ToString()!, Culture); + value = decimal.Parse(value.ToString(), Culture); prop.SetValue(x, value, null); } else if (asType == typeof(Guid)) { var raw = value.ToString(); - value = string.IsNullOrEmpty(raw) ? Guid.Empty : new(value.ToString()!); + value = string.IsNullOrEmpty(raw) ? Guid.Empty : new(value.ToString()); prop.SetValue(x, value, null); } else if (asType == typeof(TimeSpan)) { - var timeSpan = XmlConvert.ToTimeSpan(value.ToString()!); + var timeSpan = XmlConvert.ToTimeSpan(value.ToString()); prop.SetValue(x, timeSpan, null); } else if (type.IsGenericType) { var list = (IList)Activator.CreateInstance(asType)!; - var container = GetElementByName(root, name); + XElement? container = null; + + // First check if root itself is the container (matches property name) + if (root != null && name?.LocalName != null) { + var rootName = root.Name.LocalName; + if (rootName.Equals(name.LocalName, StringComparison.OrdinalIgnoreCase)) { + container = root; + } + } + + // If root is not the container, try to find it as a child element + container ??= GetElementByName(root, name); if (container?.HasElements == true) { var first = container.Elements().FirstOrDefault(); @@ -261,7 +285,7 @@ protected virtual object Map(object x, XElement? root) { else { //fallback to type converters if possible - if (TryGetFromString(value.ToString()!, out var result, asType)) { + if (TryGetFromString(value.ToString(), out var result, asType)) { prop.SetValue(x, result, null); } else { @@ -306,37 +330,40 @@ object HandleListDerivative(XElement root, string propName, Type type) { var list = (IList)Activator.CreateInstance(type)!; - IList elements = root.Descendants(t.Name.AsNamespaced(Namespace)).ToList(); - var name = t.Name; var attribute = t.GetAttribute(); if (attribute != null) name = attribute.Name; - if (!elements.Any()) { - var lowerName = name?.ToLower(Culture).AsNamespaced(Namespace); + // Try to find a container element first using the property name + XElement? container = null; - elements = root.Descendants(lowerName).ToList(); + // Try the property name first (skip if it contains invalid XML name characters like ` in generic types) + if (IsValidXmlElementName(propName)) { + container = GetElementByName(root, propName.AsNamespaced(Namespace)); } - if (!elements.Any()) { - var camelName = name?.ToCamelCase(Culture).AsNamespaced(Namespace); + // Check if root itself matches the container naming + if (container == null && IsValidXmlElementName(propName)) { + var rootName = root.Name.LocalName; - elements = root.Descendants(camelName).ToList(); + if (rootName.Equals(propName, StringComparison.OrdinalIgnoreCase)) { + container = root; + } } - if (!elements.Any()) - elements = root.Descendants() - .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == name) - .ToList(); + IList elements; - if (!elements.Any()) { - var lowerName = name?.ToLower(Culture).AsNamespaced(Namespace); - - elements = root.Descendants() - .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == lowerName) - .ToList(); + // If we found a specific container, use Elements() to get only direct children + // This prevents nested lists from being incorrectly flattened + if (container is { HasElements: true }) { + elements = TryFindElementsByNameVariations(container, t.Name, name, useDirectChildrenOnly: true); + } + else { + // No container found - use Descendants() for backward compatibility + // This handles cases where items are nested at varying depths without a clear container + elements = TryFindElementsByNameVariations(root, t.Name, name, useDirectChildrenOnly: false); } PopulateListFromElements(t, elements, list); @@ -349,6 +376,67 @@ object HandleListDerivative(XElement root, string propName, Type type) { return list; } + IList TryFindElementsByNameVariations(XElement source, string typeName, string? itemName, bool useDirectChildrenOnly) { + IList elements; + + if (useDirectChildrenOnly) { + // Use Elements() for direct children only + elements = source.Elements(typeName.AsNamespaced(Namespace)).ToList(); + + if (!elements.Any()) { + var lowerName = itemName?.ToLower(Culture).AsNamespaced(Namespace); + elements = source.Elements(lowerName).ToList(); + } + + if (!elements.Any()) { + var camelName = itemName?.ToCamelCase(Culture).AsNamespaced(Namespace); + elements = source.Elements(camelName).ToList(); + } + + if (!elements.Any()) { + elements = source.Elements() + .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == itemName) + .ToList(); + } + + if (!elements.Any()) { + var lowerName = itemName?.ToLower(Culture); + elements = source.Elements() + .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == lowerName) + .ToList(); + } + } + else { + // Use Descendants() for backward compatibility when no container is found + elements = source.Descendants(typeName.AsNamespaced(Namespace)).ToList(); + + if (!elements.Any()) { + var lowerName = itemName?.ToLower(Culture).AsNamespaced(Namespace); + elements = source.Descendants(lowerName).ToList(); + } + + if (!elements.Any()) { + var camelName = itemName?.ToCamelCase(Culture).AsNamespaced(Namespace); + elements = source.Descendants(camelName).ToList(); + } + + if (!elements.Any()) { + elements = source.Descendants() + .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == itemName) + .ToList(); + } + + if (!elements.Any()) { + var lowerName = itemName?.ToLower(Culture); + elements = source.Descendants() + .Where(e => e.Name.LocalName.RemoveUnderscoresAndDashes() == lowerName) + .ToList(); + } + } + + return elements; + } + protected virtual object? CreateAndMap(Type t, XElement element) { object? item; diff --git a/test/RestSharp.Tests.Serializers.Xml/SampleClasses/NestedElementTestClasses.cs b/test/RestSharp.Tests.Serializers.Xml/SampleClasses/NestedElementTestClasses.cs new file mode 100644 index 000000000..d6a512c44 --- /dev/null +++ b/test/RestSharp.Tests.Serializers.Xml/SampleClasses/NestedElementTestClasses.cs @@ -0,0 +1,25 @@ +namespace RestSharp.Tests.Serializers.Xml.SampleClasses; + +// Test classes for nested element bugs + +public class Item { + public int Id { get; set; } + public List SubItems { get; set; } +} + +public class ItemContainer { + public List Items { get; set; } = new(); +} + +public class ItemWithGroup { + public int Id { get; set; } + public ItemGroup Group { get; set; } +} + +public class ItemGroup { + public List Items { get; set; } +} + +public class ItemsResponse { + public List Items { get; set; } = new(); +} diff --git a/test/RestSharp.Tests.Serializers.Xml/XmlDeserializerTests.cs b/test/RestSharp.Tests.Serializers.Xml/XmlDeserializerTests.cs index 5f26af53b..1c3454356 100644 --- a/test/RestSharp.Tests.Serializers.Xml/XmlDeserializerTests.cs +++ b/test/RestSharp.Tests.Serializers.Xml/XmlDeserializerTests.cs @@ -1076,4 +1076,168 @@ public void Ignore_ReadOnly_Property_That_Exists_In_Data() { Assert.Null(p.ReadOnlyProxy); } -} \ No newline at end of file + + [Fact] + public void Deserialize_Nested_List_Should_Not_Include_Deeply_Nested_Items() { + // Bug #1 test: HandleListDerivative should use Elements() on containers, not Descendants() + const string xml = """ + + + + 1 + + 2 + 3 + + + + + """; + + var deserializer = new XmlDeserializer(); + var result = deserializer.Deserialize(new RestResponse { Content = xml })!; + + // Should only have 1 item (id=1), not 3 (id=1,2,3) + Assert.NotNull(result); + Assert.NotNull(result.Items); + Assert.Single(result.Items); + Assert.Equal(1, result.Items[0].Id); + } + + [Fact] + public void Deserialize_RootElement_Should_Not_Throw_On_Duplicate_Nested_Names() { + // Bug #2 test: RootElement selection should handle duplicate names gracefully + const string xml = """ + + + + 72 + + + 74 + + + + + + """; + + var deserializer = new XmlDeserializer { RootElement = "items" }; + + // Should not throw InvalidOperationException: Sequence contains more than one element + var exception = Record.Exception(() => + deserializer.Deserialize(new RestResponse { Content = xml }) + ); + + Assert.Null(exception); + } + + [Fact] + public void Deserialize_RootElement_Should_Prefer_Shallowest_Match() { + // Bug #2 test: When multiple elements match RootElement, prefer the shallowest one + const string xml = """ + + + + 72 + + + 74 + + + + + + """; + + var deserializer = new XmlDeserializer { RootElement = "items" }; + var result = deserializer.Deserialize(new RestResponse { Content = xml })!; + + // Should deserialize from the top-level , not the nested one + Assert.NotNull(result); + Assert.NotNull(result.Items); + Assert.Single(result.Items); + Assert.Equal(72, result.Items[0].Id); + } + + [Fact] + public void Deserialize_RootElement_Should_Try_Direct_Child_First() { + // Bug #2 test: Should try Element() before DescendantsAndSelf() + const string xml = """ + + + direct + + + """; + + var deserializer = new XmlDeserializer { RootElement = "data" }; + var result = deserializer.Deserialize(new RestResponse { Content = xml }); + + Assert.Equal("direct", result.One); + } + + [Fact] + public void Deserialize_List_With_Container_Should_Use_Direct_Children_Only() { + // Test that container-based list deserialization uses Elements() not Descendants() + const string xml = """ + + + + 10 + + 20 + + + + 11 + + + + """; + + var deserializer = new XmlDeserializer(); + var result = deserializer.Deserialize(new RestResponse { Content = xml })!; + + Assert.NotNull(result.Items); + Assert.Equal(2, result.Items.Count); + Assert.Equal(10, result.Items[0].Id); + Assert.Equal(11, result.Items[1].Id); + } + + [Fact] + public void Deserialize_Nested_Items_Should_Also_Use_Direct_Children() { + // Test that nested subitems also correctly use direct children + const string xml = """ + + + + 1 + + 2 + + 3 + + 4 + + + + + + + """; + + var deserializer = new XmlDeserializer(); + var result = deserializer.Deserialize(new RestResponse { Content = xml })!; + + Assert.NotNull(result.Items); + Assert.Single(result.Items); + + var topItem = result.Items[0]; + Assert.Equal(1, topItem.Id); + Assert.NotNull(topItem.SubItems); + Assert.Equal(2, topItem.SubItems.Count); + Assert.Equal(2, topItem.SubItems[0].Id); + Assert.Equal(3, topItem.SubItems[1].Id); + } +}