From d8dfbec04b82b034dab8410ec2fbc4d154915a4d Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 4 Sep 2026 11:19:13 +0200 Subject: [PATCH] Write XAML a XAML parser can read back Two of the defects reported on issue #2253 come from the decompiler writing text that means something else when it is read again. A markup extension is written as a single attribute value, and its grammar gives ',' '=' '{' '}' and the quote characters a meaning. Values went out unquoted, so an argument carrying any of them was read back as further name/value pairs: {DXBinding Expr='Price - Prev > 0 ? ...'}, the reported case, no longer compiles at all (MC3042, MC3045). Values without such a character stay unquoted, because quoting them would rewrite every document that never needed it. A clr-namespace declaration names the CLR namespace it maps, but nothing read that name out of it, so no lookup by namespace could match a declaration the document itself had made. Every type in such a namespace then got a second prefix declared on the element that used it. The assembly is the second half of the same lookup, and there the document records the name it was written against while a well-known type carries the assembly it resolves to now - "mscorlib" against "System.Private.CoreLib" - so the two are also accepted as the same when the recorded assembly forwards the type. Assisted-by: Claude:claude-opus-5:Claude Code --- .../Handlers/Records/XmlnsPropertyHandler.cs | 12 +- .../Xaml/NamespaceMap.cs | 35 +++++ .../Xaml/XamlExtension.cs | 2 +- ICSharpCode.BamlDecompiler/Xaml/XamlType.cs | 4 +- ICSharpCode.BamlDecompiler/Xaml/XamlUtils.cs | 79 +++++++++++ ICSharpCode.BamlDecompiler/XamlContext.cs | 2 +- ICSharpCode.BamlDecompiler/XmlnsDictionary.cs | 10 +- .../Cases/EscapeSequence.xaml | 6 +- .../Cases/MarkupExtension.xaml | 2 +- .../ILSpy.BamlDecompiler.Tests.csproj | 2 + .../MarkupExtensionQuotingTests.cs | 133 ++++++++++++++++++ .../XmlnsDeclarationPlacementTests.cs | 118 ++++++++++++++++ 12 files changed, 390 insertions(+), 15 deletions(-) create mode 100644 ILSpy.BamlDecompiler.Tests/MarkupExtensionQuotingTests.cs create mode 100644 ILSpy.BamlDecompiler.Tests/XmlnsDeclarationPlacementTests.cs diff --git a/ICSharpCode.BamlDecompiler/Handlers/Records/XmlnsPropertyHandler.cs b/ICSharpCode.BamlDecompiler/Handlers/Records/XmlnsPropertyHandler.cs index 7eb462ffb..ac5d8a268 100644 --- a/ICSharpCode.BamlDecompiler/Handlers/Records/XmlnsPropertyHandler.cs +++ b/ICSharpCode.BamlDecompiler/Handlers/Records/XmlnsPropertyHandler.cs @@ -58,12 +58,20 @@ namespace ICSharpCode.BamlDecompiler.Handlers foreach (var asmId in record.AssemblyIds) { var assembly = ctx.Baml.ResolveAssembly(asmId); - ctx.XmlNs.Add(new NamespaceMap(record.Prefix, assembly.FullAssemblyName, record.XmlNamespace)); + // A clr-namespace declaration names its CLR namespace itself. Leaving that unread + // means no lookup by namespace can ever find the declaration the document made, + // and every type in it gets a second prefix of its own (issue #2253). + XamlUtils.TryParseClrNamespace(record.XmlNamespace, out string declaredClrNamespace); + ctx.XmlNs.Add(new NamespaceMap(record.Prefix, assembly.FullAssemblyName, record.XmlNamespace, declaredClrNamespace) { + Assembly = assembly.Assembly + }); if (assembly.Assembly?.IsMainModule == true) { foreach (var clrNs in ResolveCLRNamespaces(assembly.Assembly, record.XmlNamespace)) - ctx.XmlNs.Add(new NamespaceMap(record.Prefix, assembly.FullAssemblyName, record.XmlNamespace, clrNs)); + ctx.XmlNs.Add(new NamespaceMap(record.Prefix, assembly.FullAssemblyName, record.XmlNamespace, clrNs) { + Assembly = assembly.Assembly + }); } } diff --git a/ICSharpCode.BamlDecompiler/Xaml/NamespaceMap.cs b/ICSharpCode.BamlDecompiler/Xaml/NamespaceMap.cs index a81bec5cd..33411ccff 100644 --- a/ICSharpCode.BamlDecompiler/Xaml/NamespaceMap.cs +++ b/ICSharpCode.BamlDecompiler/Xaml/NamespaceMap.cs @@ -31,6 +31,13 @@ namespace ICSharpCode.BamlDecompiler.Xaml { public string XmlnsPrefix { get; set; } public string FullAssemblyName { get; set; } + + /// + /// The assembly resolves to, where it could be resolved. + /// The name is the one the document was written against, which is not always the name of + /// the assembly the types actually come from. + /// + public IModule Assembly { get; set; } public string XMLNamespace { get; set; } public string CLRNamespace { get; set; } @@ -47,6 +54,34 @@ namespace ICSharpCode.BamlDecompiler.Xaml CLRNamespace = clrNs; } + /// + /// Whether is the declaration to use for a type named + /// in of + /// . + /// + public static bool Matches(NamespaceMap map, string fullAssemblyName, string clrNs, string typeName) + { + if (map.CLRNamespace != clrNs) + return false; + if (map.FullAssemblyName == fullAssemblyName) + return true; + // The document records the assembly it was written against, while a well-known type + // carries the assembly it resolves to now - "mscorlib" against "System.Private.CoreLib" + // on .NET, say. The two name the same type when the recorded assembly forwards it, and + // then the declaration the document made is the one to use. + return typeName != null && ForwardsOrDeclares(map.Assembly, clrNs, typeName); + } + + static bool ForwardsOrDeclares(IModule assembly, string clrNs, string typeName) + { + if (assembly == null) + return false; + var name = new TopLevelTypeName(clrNs, typeName); + if (assembly.GetTypeDefinition(name) != null) + return true; + return assembly.MetadataFile?.GetTypeForwarder(new FullTypeName(name)).IsNil == false; + } + public override string ToString() => $"{XmlnsPrefix}:[{FullAssemblyName}|{CLRNamespace ?? XMLNamespace}]"; } } \ No newline at end of file diff --git a/ICSharpCode.BamlDecompiler/Xaml/XamlExtension.cs b/ICSharpCode.BamlDecompiler/Xaml/XamlExtension.cs index cd3e664ca..0d45d7a5b 100644 --- a/ICSharpCode.BamlDecompiler/Xaml/XamlExtension.cs +++ b/ICSharpCode.BamlDecompiler/Xaml/XamlExtension.cs @@ -43,7 +43,7 @@ namespace ICSharpCode.BamlDecompiler.Xaml if (value is XamlExtension) sb.Append(((XamlExtension)value).ToString(ctx, ctxElement)); else - sb.Append(value.ToString()); + sb.Append(XamlUtils.QuoteMarkupExtensionValue(value.ToString())); } public string ToString(XamlContext ctx, XElement ctxElement) diff --git a/ICSharpCode.BamlDecompiler/Xaml/XamlType.cs b/ICSharpCode.BamlDecompiler/Xaml/XamlType.cs index 09d287c18..a5c2be0f3 100644 --- a/ICSharpCode.BamlDecompiler/Xaml/XamlType.cs +++ b/ICSharpCode.BamlDecompiler/Xaml/XamlType.cs @@ -64,9 +64,9 @@ namespace ICSharpCode.BamlDecompiler.Xaml string xmlNs = null; if (elem.Annotation() != null) - xmlNs = elem.Annotation().LookupXmlns(FullAssemblyName, TypeNamespace); + xmlNs = elem.Annotation().LookupXmlns(FullAssemblyName, TypeNamespace, TypeName); if (xmlNs == null) - xmlNs = ctx.XmlNs.LookupXmlns(FullAssemblyName, TypeNamespace); + xmlNs = ctx.XmlNs.LookupXmlns(FullAssemblyName, TypeNamespace, TypeName); // Sometimes there's no reference to System.Xaml even if x:Type is used if (xmlNs == null) xmlNs = XamlContext.TryGetXmlNamespace(Assembly, TypeNamespace, elem); diff --git a/ICSharpCode.BamlDecompiler/Xaml/XamlUtils.cs b/ICSharpCode.BamlDecompiler/Xaml/XamlUtils.cs index 572667f1d..34a8b7ce8 100644 --- a/ICSharpCode.BamlDecompiler/Xaml/XamlUtils.cs +++ b/ICSharpCode.BamlDecompiler/Xaml/XamlUtils.cs @@ -20,6 +20,7 @@ THE SOFTWARE. */ +using System; using System.IO; using System.Text; using System.Xml; @@ -29,6 +30,84 @@ namespace ICSharpCode.BamlDecompiler.Xaml { internal static class XamlUtils { + static readonly char[] markupExtensionSpecialCharacters = { ',', '=', '\'', '"', '\\' }; + + /// + /// Quotes an argument of a markup extension if the parser reading the document again would + /// take part of it for grammar: ',' and '=' separate arguments from one another, a quote + /// character starts a quoted value, '\' escapes whatever follows it, and whitespace at + /// either end is dropped. A value carrying none of those is left as it is, because quoting + /// every value would rewrite every document that never needed it. + /// + /// Braces are grammar only where they are unbalanced: a stray '{' opens an extension and a + /// stray '}' closes the surrounding one, while a matched pair inside a value ("{0:C}", + /// "Element[{ns}Name]") is read as text and stays unquoted. A value beginning with '{' is + /// a nested extension that is already written as one, so it is left alone; the "{}" that + /// escapes a leading brace is not, because inside an extension it would open one. + /// + /// + public static string QuoteMarkupExtensionValue(string value) + { + if (value == null) + return null; + if (value.StartsWith("{", StringComparison.Ordinal) && !value.StartsWith("{}", StringComparison.Ordinal)) + { + return value; // a nested markup extension, already written as one + } + if (value.Length > 0 + && !value.StartsWith("{}", StringComparison.Ordinal) + && value.IndexOfAny(markupExtensionSpecialCharacters) < 0 + && BracesAreBalanced(value) + && !char.IsWhiteSpace(value[0]) + && !char.IsWhiteSpace(value[value.Length - 1])) + { + return value; + } + + var quoted = new StringBuilder(value.Length + 2); + quoted.Append('\''); + foreach (char c in value) + { + if (c == '\'' || c == '\\') + quoted.Append('\\'); + quoted.Append(c); + } + quoted.Append('\''); + return quoted.ToString(); + } + + static bool BracesAreBalanced(string value) + { + int depth = 0; + foreach (char c in value) + { + if (c == '{') + depth++; + else if (c == '}' && --depth < 0) + return false; + } + return depth == 0; + } + + /// + /// Reads the CLR namespace out of a "clr-namespace:Some.Namespace;assembly=Some.Assembly" + /// declaration. Such a declaration names its CLR namespace itself; the other form of XML + /// namespace ("http://...") maps to CLR namespaces through XmlnsDefinition attributes + /// instead, and has none of its own. + /// + public static bool TryParseClrNamespace(string xmlNamespace, out string clrNamespace) + { + const string prefix = "clr-namespace:"; + clrNamespace = null; + if (xmlNamespace == null || !xmlNamespace.StartsWith(prefix, StringComparison.Ordinal)) + return false; + clrNamespace = xmlNamespace.Substring(prefix.Length); + int assembly = clrNamespace.IndexOf(';'); + if (assembly >= 0) + clrNamespace = clrNamespace.Substring(0, assembly); + return true; + } + public static string Escape(string value) { if (value.Length == 0) diff --git a/ICSharpCode.BamlDecompiler/XamlContext.cs b/ICSharpCode.BamlDecompiler/XamlContext.cs index 5fc1752aa..2d6e5c104 100644 --- a/ICSharpCode.BamlDecompiler/XamlContext.cs +++ b/ICSharpCode.BamlDecompiler/XamlContext.cs @@ -126,7 +126,7 @@ namespace ICSharpCode.BamlDecompiler } var clrNs = type.Namespace; - var xmlNs = XmlNs.LookupXmlns(fullAssemblyName, clrNs); + var xmlNs = XmlNs.LookupXmlns(fullAssemblyName, clrNs, type.Name); typeMap[id] = xamlType = new XamlType(assembly, fullAssemblyName, clrNs, type.Name, GetXmlNamespace(xmlNs)) { ResolvedType = type diff --git a/ICSharpCode.BamlDecompiler/XmlnsDictionary.cs b/ICSharpCode.BamlDecompiler/XmlnsDictionary.cs index 8f90d47a8..fcb5ccbfe 100644 --- a/ICSharpCode.BamlDecompiler/XmlnsDictionary.cs +++ b/ICSharpCode.BamlDecompiler/XmlnsDictionary.cs @@ -40,11 +40,11 @@ namespace ICSharpCode.BamlDecompiler Element = elem; } - public string LookupXmlns(string fullAssemblyName, string clrNs) + public string LookupXmlns(string fullAssemblyName, string clrNs, string typeName = null) { foreach (var ns in this) { - if (fullAssemblyName == ns.FullAssemblyName && ns.CLRNamespace == clrNs) + if (NamespaceMap.Matches(ns, fullAssemblyName, clrNs, typeName)) return ns.XMLNamespace; } @@ -119,11 +119,11 @@ namespace ICSharpCode.BamlDecompiler return null; } - public string LookupXmlns(string fullAssemblyName, string clrNs) + public string LookupXmlns(string fullAssemblyName, string clrNs, string typeName = null) { foreach (var map in piMappings) { - if (fullAssemblyName == map.Value.FullAssemblyName && map.Value.CLRNamespace == clrNs) + if (NamespaceMap.Matches(map.Value, fullAssemblyName, clrNs, typeName)) return map.Key; } @@ -132,7 +132,7 @@ namespace ICSharpCode.BamlDecompiler { foreach (var ns in scope) { - if (fullAssemblyName == ns.FullAssemblyName && ns.CLRNamespace == clrNs) + if (NamespaceMap.Matches(ns, fullAssemblyName, clrNs, typeName)) return ns.XMLNamespace; } diff --git a/ILSpy.BamlDecompiler.Tests.Windows/Cases/EscapeSequence.xaml b/ILSpy.BamlDecompiler.Tests.Windows/Cases/EscapeSequence.xaml index 004ac0d22..8dd4c16b9 100644 --- a/ILSpy.BamlDecompiler.Tests.Windows/Cases/EscapeSequence.xaml +++ b/ILSpy.BamlDecompiler.Tests.Windows/Cases/EscapeSequence.xaml @@ -7,17 +7,17 @@ - + - + - + diff --git a/ILSpy.BamlDecompiler.Tests.Windows/Cases/MarkupExtension.xaml b/ILSpy.BamlDecompiler.Tests.Windows/Cases/MarkupExtension.xaml index ab059da44..0ba8c34c5 100644 --- a/ILSpy.BamlDecompiler.Tests.Windows/Cases/MarkupExtension.xaml +++ b/ILSpy.BamlDecompiler.Tests.Windows/Cases/MarkupExtension.xaml @@ -1,4 +1,4 @@ -