From 20b1ab048b200c8988ad73a74dd9ceab49611ff6 Mon Sep 17 00:00:00 2001 From: Mahmood Malekloo Date: Tue, 11 Mar 2025 23:40:18 +0330 Subject: [PATCH 1/5] fix: test data comparation based on the xml ddata type --- .../IdentityComparer.cs | 107 ++++++++++++++++++ .../DSigSerializerTests.cs | 7 +- 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs index dc7f78345b..e63a1a0555 100644 --- a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs +++ b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs @@ -20,6 +20,7 @@ using System.Security.Cryptography.X509Certificates; using System.Text.Json; using System.Text.RegularExpressions; +using System.Xml.Linq; using Microsoft.IdentityModel.JsonWebTokens; using Microsoft.IdentityModel.Protocols; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -142,6 +143,7 @@ public class IdentityComparer { typeof(SignedInfo).ToString(), CompareAllPublicProperties }, { typeof(SigningCredentials).ToString(), CompareAllPublicProperties }, { typeof(string).ToString(), AreStringsEqual }, + { typeof(XDocument).ToString(), AreXmlsEqual }, { typeof(SymmetricSecurityKey).ToString(), CompareAllPublicProperties }, { typeof(TimeSpan).ToString(), AreTimeSpansEqual }, { typeof(TokenValidationParameters).ToString(), CompareAllPublicProperties }, @@ -1170,9 +1172,114 @@ public static bool AreStringDictionariesEqual(Object object1, Object object2, Co context.Diffs.AddRange(localContext.Diffs); return localContext.Diffs.Count == 0; } + public static bool AreXmlsEqual(object xml1, object xml2, CompareContext context) + { + return AreXmlsEqual((XDocument)xml1, (XDocument)xml2, "xml1", "xml2", context); + } + + private static bool AreXmlsEqual(XDocument xml1, XDocument xml2, string name1, string name2, CompareContext context) + { + var localContext = new CompareContext(context); + if (!ContinueCheckingEquality(xml1, xml2, localContext)) + return context.Merge(localContext); + + if (ReferenceEquals(xml1, xml2)) + return true; + + if (xml1 == null) + localContext.Diffs.Add($"({name1} == null, {name2} == {xml2.ToString()}."); + + if (xml2 == null) + localContext.Diffs.Add($"({name1} == {xml1.ToString()}, {name2} == null."); + + if (!CompareXmlElements(xml1.Root, xml2.Root, localContext)) + { + + localContext.Diffs.Add($"'{name1}' != '{name2}', StringComparison: '{context.StringComparison}'"); + localContext.Diffs.Add($"'{xml1.ToString()}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{xml2.ToString()}'"); + } + + + return context.Merge(localContext); + } + + /// + /// Compares two XML elements for equality, ignoring order of attributes and child elements. + /// Ignore X509 certificate elements and attributes. + /// + /// The first XML element to compare. + /// The second XML element to compare. + /// + /// True if the elements are considered equal, otherwise false. + private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareContext localContext) + { + // Ensure both elements exist; if one is null while the other isn't, they are not equal. + if (elem1 == null || elem2 == null) + return false; + + // Compare element names; if they are different, the elements are not equal. + if (elem1.Name != elem2.Name) + return false; + + // Ignore comparison for elements related to X509 certificates. + if (elem1.Name.ToString().Contains("X509")) + return true; + + // Retrieve and order attributes by name to ensure order-independent comparison. + var attrs1 = elem1.Attributes().OrderBy(a => a.Name.ToString()).ToList(); + var attrs2 = elem2.Attributes().OrderBy(a => a.Name.ToString()).ToList(); + + // If the number of attributes differs, the elements are not equal. + if (attrs1.Count != attrs2.Count) + return false; + + // Compare attributes + for (int i = 0; i < attrs1.Count; i++) + { + // Compare attribute names; if different, the elements are not equal. + if (attrs1[i].Name != attrs2[i].Name) + return false; + + // Ignore attributes related to X509 certificates. + if (attrs1[i].Name.ToString().Contains("X509")) + continue; + + // Compare attribute values using the specified string comparison method. + if (!string.Equals(attrs1[i].Value, attrs2[i].Value, localContext.StringComparison)) + return false; + } + + // Retrieve and order child elements by name to ensure order-independent comparison. + var children1 = elem1.Elements().OrderBy(e => e.Name.ToString()).ToList(); + var children2 = elem2.Elements().OrderBy(e => e.Name.ToString()).ToList(); + + // If the number of child elements differs, the elements are not equal. + if (children1.Count != children2.Count) + return false; + + // Recursively compare child elements. + for (int i = 0; i < children1.Count; i++) + { + if (!CompareXmlElements(children1[i], children2[i], localContext)) + return false; // Child elements mismatch + } + + // If the element has no children, compare its values. + if (children1.Count == 0 && !string.Equals(elem1.Value.Trim(), elem2.Value.Trim(), localContext.StringComparison)) + { + localContext.Diffs.Add(elem1.Value.Trim()); + localContext.Diffs.Add("!="); + localContext.Diffs.Add(elem2.Value.Trim()); + return false; + } + return true; + } public static bool AreStringsEqual(object object1, object object2, CompareContext context) { + return AreStringsEqual(object1, object2, "str1", "str2", context); } diff --git a/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs b/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs index 4f14f1c427..abbc7104a3 100644 --- a/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs +++ b/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs @@ -7,6 +7,7 @@ using System.Security.Cryptography; using System.Text; using System.Xml; +using System.Xml.Linq; using Microsoft.IdentityModel.TestUtils; using Microsoft.IdentityModel.Tokens; using Microsoft.IdentityModel.Xml; @@ -115,7 +116,11 @@ public void WriteKeyInfo(DSigSerializerTheoryData theoryData) theoryData.Serializer.WriteKeyInfo(writer, keyInfo); writer.Flush(); var xml = Encoding.UTF8.GetString(ms.ToArray()); - IdentityComparer.AreEqual(theoryData.Xml, xml); + + // Compare the original XML with the re-serialized XML. + // Parsing the XML strings into XDocument ensures that the comparison is based on + // structural and content equality rather than raw string differences (formatting, whitespace,...). + IdentityComparer.AreEqual(XDocument.Parse(theoryData.Xml), XDocument.Parse(xml), context); } catch (Exception ex) { From 3664823c350a8171730d42658771293846c4eed6 Mon Sep 17 00:00:00 2001 From: Mahmood Malekloo Date: Sat, 22 Mar 2025 20:42:33 +0430 Subject: [PATCH 2/5] fix: remove redundant null check --- test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs index e63a1a0555..ea02b56e8c 100644 --- a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs +++ b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs @@ -1186,12 +1186,6 @@ private static bool AreXmlsEqual(XDocument xml1, XDocument xml2, string name1, s if (ReferenceEquals(xml1, xml2)) return true; - if (xml1 == null) - localContext.Diffs.Add($"({name1} == null, {name2} == {xml2.ToString()}."); - - if (xml2 == null) - localContext.Diffs.Add($"({name1} == {xml1.ToString()}, {name2} == null."); - if (!CompareXmlElements(xml1.Root, xml2.Root, localContext)) { From 7ef0393a19129ed54b755d24c41386514fcd6486 Mon Sep 17 00:00:00 2001 From: Mahmood Malekloo Date: Sat, 22 Mar 2025 21:05:49 +0430 Subject: [PATCH 3/5] fix: add information about differences found in xml documents to compare context --- .../IdentityComparer.cs | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs index ea02b56e8c..2521ae1cb4 100644 --- a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs +++ b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs @@ -1188,14 +1188,12 @@ private static bool AreXmlsEqual(XDocument xml1, XDocument xml2, string name1, s if (!CompareXmlElements(xml1.Root, xml2.Root, localContext)) { - localContext.Diffs.Add($"'{name1}' != '{name2}', StringComparison: '{context.StringComparison}'"); localContext.Diffs.Add($"'{xml1.ToString()}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{xml2.ToString()}'"); } - return context.Merge(localContext); } @@ -1211,11 +1209,23 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo { // Ensure both elements exist; if one is null while the other isn't, they are not equal. if (elem1 == null || elem2 == null) + { + localContext.Diffs.Add($"one of the xml elements is null"); + localContext.Diffs.Add($"'{elem1.ToString()}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{elem2.ToString()}'"); return false; + } // Compare element names; if they are different, the elements are not equal. if (elem1.Name != elem2.Name) + { + localContext.Diffs.Add($"xml element name are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"'{elem1.Name.ToString()}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{elem2.Name.ToString()}'"); return false; + } // Ignore comparison for elements related to X509 certificates. if (elem1.Name.ToString().Contains("X509")) @@ -1227,14 +1237,26 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // If the number of attributes differs, the elements are not equal. if (attrs1.Count != attrs2.Count) + { + localContext.Diffs.Add($"number of xml element attribute are not the same"); + localContext.Diffs.Add($"'{attrs1.Count}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{attrs2.Count}'"); return false; + } // Compare attributes for (int i = 0; i < attrs1.Count; i++) { // Compare attribute names; if different, the elements are not equal. if (attrs1[i].Name != attrs2[i].Name) + { + localContext.Diffs.Add($"the xml element attribute name are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"'{attrs1[i].Name}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{attrs2[i].Name}'"); return false; + } // Ignore attributes related to X509 certificates. if (attrs1[i].Name.ToString().Contains("X509")) @@ -1242,7 +1264,13 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // Compare attribute values using the specified string comparison method. if (!string.Equals(attrs1[i].Value, attrs2[i].Value, localContext.StringComparison)) + { + localContext.Diffs.Add($"the xml element attribute value are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"'{attrs1[i].Value}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{attrs2[i].Value}'"); return false; + } } // Retrieve and order child elements by name to ensure order-independent comparison. @@ -1251,7 +1279,13 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // If the number of child elements differs, the elements are not equal. if (children1.Count != children2.Count) + { + localContext.Diffs.Add($"number of xml element children are not the same"); + localContext.Diffs.Add($"'{children1.Count}'"); + localContext.Diffs.Add($"!="); + localContext.Diffs.Add($"'{children2.Count}'"); return false; + } // Recursively compare child elements. for (int i = 0; i < children1.Count; i++) @@ -1263,6 +1297,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // If the element has no children, compare its values. if (children1.Count == 0 && !string.Equals(elem1.Value.Trim(), elem2.Value.Trim(), localContext.StringComparison)) { + localContext.Diffs.Add($"the xml element value are not equal, StringComparison: '{localContext.StringComparison}'"); localContext.Diffs.Add(elem1.Value.Trim()); localContext.Diffs.Add("!="); localContext.Diffs.Add(elem2.Value.Trim()); @@ -1273,7 +1308,6 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo public static bool AreStringsEqual(object object1, object object2, CompareContext context) { - return AreStringsEqual(object1, object2, "str1", "str2", context); } From 1438d3a6ecbe10285369a690c1ae4e9001cd0a3c Mon Sep 17 00:00:00 2001 From: Mahmood Malekloo Date: Sat, 22 Mar 2025 21:17:14 +0430 Subject: [PATCH 4/5] fix: add try catch block for xml document cast exception and fix typo --- .../IdentityComparer.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs index 2521ae1cb4..4e0676c92f 100644 --- a/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs +++ b/test/Microsoft.IdentityModel.TestUtils/IdentityComparer.cs @@ -1174,7 +1174,15 @@ public static bool AreStringDictionariesEqual(Object object1, Object object2, Co } public static bool AreXmlsEqual(object xml1, object xml2, CompareContext context) { - return AreXmlsEqual((XDocument)xml1, (XDocument)xml2, "xml1", "xml2", context); + try + { + return AreXmlsEqual((XDocument)xml1, (XDocument)xml2, "xml1", "xml2", context); + } + catch (InvalidCastException ex) + { + context.Diffs.Add($"unable to cast {xml1.ToString()} and/or {xml2.ToString()} to xml document. Exception: '{ex}'."); + return false; + } } private static bool AreXmlsEqual(XDocument xml1, XDocument xml2, string name1, string name2, CompareContext context) @@ -1220,7 +1228,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // Compare element names; if they are different, the elements are not equal. if (elem1.Name != elem2.Name) { - localContext.Diffs.Add($"xml element name are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"xml element names are not equal, StringComparison: '{localContext.StringComparison}'"); localContext.Diffs.Add($"'{elem1.Name.ToString()}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{elem2.Name.ToString()}'"); @@ -1238,7 +1246,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // If the number of attributes differs, the elements are not equal. if (attrs1.Count != attrs2.Count) { - localContext.Diffs.Add($"number of xml element attribute are not the same"); + localContext.Diffs.Add($"number of xml element attributes are not the same"); localContext.Diffs.Add($"'{attrs1.Count}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{attrs2.Count}'"); @@ -1251,7 +1259,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // Compare attribute names; if different, the elements are not equal. if (attrs1[i].Name != attrs2[i].Name) { - localContext.Diffs.Add($"the xml element attribute name are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"the xml element attribute names are not equal, StringComparison: '{localContext.StringComparison}'"); localContext.Diffs.Add($"'{attrs1[i].Name}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{attrs2[i].Name}'"); @@ -1265,7 +1273,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // Compare attribute values using the specified string comparison method. if (!string.Equals(attrs1[i].Value, attrs2[i].Value, localContext.StringComparison)) { - localContext.Diffs.Add($"the xml element attribute value are not equal, StringComparison: '{localContext.StringComparison}'"); + localContext.Diffs.Add($"the xml element attribute values are not equal, StringComparison: '{localContext.StringComparison}'"); localContext.Diffs.Add($"'{attrs1[i].Value}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{attrs2[i].Value}'"); @@ -1280,7 +1288,7 @@ private static bool CompareXmlElements(XElement elem1, XElement elem2, CompareCo // If the number of child elements differs, the elements are not equal. if (children1.Count != children2.Count) { - localContext.Diffs.Add($"number of xml element children are not the same"); + localContext.Diffs.Add($"number of xml element childrens are not the same"); localContext.Diffs.Add($"'{children1.Count}'"); localContext.Diffs.Add($"!="); localContext.Diffs.Add($"'{children2.Count}'"); From 658442d16ddcb44742504f3952832fa3d1393057 Mon Sep 17 00:00:00 2001 From: Mahmood Malekloo Date: Sat, 22 Mar 2025 21:22:25 +0430 Subject: [PATCH 5/5] fix: add exception handling for the current test case --- .../DSigSerializerTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs b/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs index abbc7104a3..6ddccde774 100644 --- a/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs +++ b/test/Microsoft.IdentityModel.Xml.Tests/DSigSerializerTests.cs @@ -122,6 +122,16 @@ public void WriteKeyInfo(DSigSerializerTheoryData theoryData) // structural and content equality rather than raw string differences (formatting, whitespace,...). IdentityComparer.AreEqual(XDocument.Parse(theoryData.Xml), XDocument.Parse(xml), context); } + catch (InvalidCastException ex) + { + context.Diffs.Add($"InvalidCastException: {ex.Message}"); + theoryData.ExpectedException.ProcessException(ex, context.Diffs); + } + catch (XmlException ex) + { + context.Diffs.Add($"XmlException: {ex.Message}"); + theoryData.ExpectedException.ProcessException(ex, context.Diffs); + } catch (Exception ex) { theoryData.ExpectedException.ProcessException(ex, context.Diffs);