From: Stephane Delcroix Date: Sun, 1 Sep 2019 21:01:44 +0000 (+0200) Subject: [X] Detect empty property value in markups (#7190) X-Git-Tag: submit/tizen_5.5/20200420.234045~211^2~3 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1efe1311af0957790be18e01199d66007878c2f1;p=platform%2Fcore%2Fcsapi%2Fxsf.git [X] Detect empty property value in markups (#7190) - fixes #7187 --- diff --git a/Xamarin.Forms.Build.Tasks/ExpandMarkupsVisitor.cs b/Xamarin.Forms.Build.Tasks/ExpandMarkupsVisitor.cs index 8d216518..d8d7f957 100644 --- a/Xamarin.Forms.Build.Tasks/ExpandMarkupsVisitor.cs +++ b/Xamarin.Forms.Build.Tasks/ExpandMarkupsVisitor.cs @@ -8,7 +8,7 @@ namespace Xamarin.Forms.Build.Tasks { class ExpandMarkupsVisitor : IXamlNodeVisitor { - readonly IList skips = new List + readonly IList _skips = new List { XmlName.xKey, XmlName.xTypeArguments, @@ -16,10 +16,7 @@ namespace Xamarin.Forms.Build.Tasks XmlName.xName, }; - public ExpandMarkupsVisitor(ILContext context) - { - Context = context; - } + public ExpandMarkupsVisitor(ILContext context) => Context = context; ILContext Context { get; } @@ -42,17 +39,14 @@ namespace Xamarin.Forms.Build.Tasks public void Visit(MarkupNode markupnode, INode parentNode) { - XmlName propertyName; - if (!TryGetProperyName(markupnode, parentNode, out propertyName)) + if (!TryGetProperyName(markupnode, parentNode, out XmlName propertyName)) return; - if (skips.Contains(propertyName)) + if (_skips.Contains(propertyName)) return; if (parentNode is IElementNode && ((IElementNode)parentNode).SkipProperties.Contains (propertyName)) return; var markupString = markupnode.MarkupString; - var node = ParseExpression(ref markupString, Context, markupnode.NamespaceResolver, markupnode) as IElementNode; - if (node != null) - { + if (ParseExpression(ref markupString, Context, markupnode.NamespaceResolver, markupnode) is IElementNode node) { ((IElementNode)parentNode).Properties[propertyName] = node; node.Accept(new XamlNodeVisitor((n, parent) => n.Parent = parent), parentNode); } @@ -73,11 +67,9 @@ namespace Xamarin.Forms.Build.Tasks public static bool TryGetProperyName(INode node, INode parentNode, out XmlName name) { name = default(XmlName); - var parentElement = parentNode as IElementNode; - if (parentElement == null) + if (!(parentNode is IElementNode parentElement)) return false; - foreach (var kvp in parentElement.Properties) - { + foreach (var kvp in parentElement.Properties) { if (kvp.Value != node) continue; name = kvp.Key; @@ -95,9 +87,7 @@ namespace Xamarin.Forms.Build.Tasks if (expression[expression.Length - 1] != '}') throw new XamlParseException("Markup expression missing its closing tag", xmlLineInfo); - int len; - string match; - if (!MarkupExpressionParser.MatchMarkup(out match, expression, out len)) + if (!MarkupExpressionParser.MatchMarkup(out var match, expression, out var len)) throw new XamlParseException("Error while parsing markup expression", xmlLineInfo); expression = expression.Substring(len).TrimStart(); if (expression.Length == 0) @@ -113,31 +103,23 @@ namespace Xamarin.Forms.Build.Tasks class ILContextProvider { - public ILContextProvider(ILContext context) - { - Context = context; - } + public ILContextProvider(ILContext context) => Context = context; public ILContext Context { get; } } class MarkupExpansionParser : MarkupExpressionParser, IExpressionParser { - IElementNode node; + IElementNode _node; - object IExpressionParser.Parse(string match, ref string remaining, IServiceProvider serviceProvider) - { - return Parse(match, ref remaining, serviceProvider); - } + object IExpressionParser.Parse(string match, ref string remaining, IServiceProvider serviceProvider) => Parse(match, ref remaining, serviceProvider); public INode Parse(string match, ref string remaining, IServiceProvider serviceProvider) { - var nsResolver = serviceProvider.GetService(typeof (IXmlNamespaceResolver)) as IXmlNamespaceResolver; - if (nsResolver == null) + if (!(serviceProvider.GetService(typeof(IXmlNamespaceResolver)) is IXmlNamespaceResolver nsResolver)) throw new ArgumentException(); IXmlLineInfo xmlLineInfo = null; - var xmlLineInfoProvider = serviceProvider.GetService(typeof (IXmlLineInfoProvider)) as IXmlLineInfoProvider; - if (xmlLineInfoProvider != null) + if (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) is IXmlLineInfoProvider xmlLineInfoProvider) xmlLineInfo = xmlLineInfoProvider.XmlLineInfo; var contextProvider = serviceProvider.GetService(typeof (ILContextProvider)) as ILContextProvider; @@ -146,13 +128,11 @@ namespace Xamarin.Forms.Build.Tasks throw new ArgumentException(); string prefix, name; - if (split.Length == 2) - { + if (split.Length == 2) { prefix = split[0]; name = split[1]; } - else - { + else { prefix = ""; name = split[0]; } @@ -162,47 +142,44 @@ namespace Xamarin.Forms.Build.Tasks throw new XamlParseException($"Undeclared xmlns prefix '{prefix}'", xmlLineInfo); //The order of lookup is to look for the Extension-suffixed class name first and then look for the class name without the Extension suffix. XmlType type; - try - { + try { type = new XmlType(namespaceuri, name + "Extension", null); type.GetTypeReference(contextProvider.Context.Module, null); } - catch (XamlParseException) - { + catch (XamlParseException) { type = new XmlType(namespaceuri, name, null); } if (type == null) throw new NotSupportedException(); - node = xmlLineInfo == null + _node = xmlLineInfo == null ? new ElementNode(type, "", nsResolver) : new ElementNode(type, "", nsResolver, xmlLineInfo.LineNumber, xmlLineInfo.LinePosition); - if (remaining.StartsWith("}", StringComparison.Ordinal)) - { + if (remaining.StartsWith("}", StringComparison.Ordinal)) { remaining = remaining.Substring(1); - return node; + return _node; } - char next; string piece; - while ((piece = GetNextPiece(ref remaining, out next)) != null) + while ((piece = GetNextPiece(ref remaining, out var next)) != null) HandleProperty(piece, serviceProvider, ref remaining, next != '='); - return node; + return _node; } protected override void SetPropertyValue(string prop, string strValue, object value, IServiceProvider serviceProvider) { + if (value == null && strValue == null) + throw new XamlParseException($"No value found for property '{prop}' in markup expression", serviceProvider); var nsResolver = serviceProvider.GetService(typeof(IXmlNamespaceResolver)) as IXmlNamespaceResolver; - if (prop != null) - { - var name = new XmlName(node.NamespaceURI, prop); - node.Properties[name] = value as INode ?? new ValueNode(strValue, nsResolver); + if (prop != null) { + var name = new XmlName(_node.NamespaceURI, prop); + _node.Properties[name] = value as INode ?? new ValueNode(strValue, nsResolver); } else //ContentProperty - node.CollectionItems.Add(value as INode ?? new ValueNode(strValue, nsResolver)); + _node.CollectionItems.Add(value as INode ?? new ValueNode(strValue, nsResolver)); } } } diff --git a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs index a17411e2..a226367d 100644 --- a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs +++ b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs @@ -137,11 +137,9 @@ namespace Xamarin.Forms.Build.Tasks { var module = context.Body.Method.Module; var str = (string)node.Value; - //If the TypeConverter has a ProvideCompiledAttribute that can be resolved, shortcut this - var compiledConverterName = typeConverter?.GetCustomAttribute(module, ("Xamarin.Forms.Core", "Xamarin.Forms.Xaml", "ProvideCompiledAttribute"))?.ConstructorArguments?.First().Value as string; Type compiledConverterType; - if (compiledConverterName != null && (compiledConverterType = Type.GetType (compiledConverterName)) != null) { + if (typeConverter?.GetCustomAttribute(module, ("Xamarin.Forms.Core", "Xamarin.Forms.Xaml", "ProvideCompiledAttribute"))?.ConstructorArguments?.First().Value is string compiledConverterName && (compiledConverterType = Type.GetType (compiledConverterName)) != null) { var compiledConverter = Activator.CreateInstance (compiledConverterType); var converter = typeof(ICompiledTypeConverter).GetMethods ().FirstOrDefault (md => md.Name == "ConvertFromString"); IEnumerable instructions; @@ -159,8 +157,7 @@ namespace Xamarin.Forms.Build.Tasks } //If there's a [TypeConverter], use it - if (typeConverter != null) - { + if (typeConverter != null) { var isExtendedConverter = typeConverter.ImplementsInterface(module.ImportReference(("Xamarin.Forms.Core", "Xamarin.Forms", "IExtendedTypeConverter"))); var typeConverterCtorRef = module.ImportCtorReference(typeConverter, paramCount: 0); var convertFromInvariantStringDefinition = isExtendedConverter @@ -172,8 +169,8 @@ namespace Xamarin.Forms.Build.Tasks .FirstOrDefault(md => md.methodDef.Name == "ConvertFromInvariantString" && md.methodDef.Parameters.Count == 1).methodDef; var convertFromInvariantStringReference = module.ImportReference(convertFromInvariantStringDefinition); - yield return Instruction.Create(OpCodes.Newobj, typeConverterCtorRef); - yield return Instruction.Create(OpCodes.Ldstr, node.Value as string); + yield return Create(Newobj, typeConverterCtorRef); + yield return Create(Ldstr, node.Value as string); if (isExtendedConverter) { diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml new file mode 100644 index 00000000..65cdea96 --- /dev/null +++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml @@ -0,0 +1,4 @@ + + + diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml.cs new file mode 100644 index 00000000..9ad6f303 --- /dev/null +++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh7187.xaml.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using NUnit.Framework; +using Xamarin.Forms; +using Xamarin.Forms.Core.UnitTests; + +namespace Xamarin.Forms.Xaml.UnitTests +{ + [XamlCompilation(XamlCompilationOptions.Skip)] + public partial class Gh7187 : ContentPage + { + public Gh7187() => InitializeComponent(); + public Gh7187(bool useCompiledXaml) + { + //this stub will be replaced at compile time + } + + [TestFixture] + class Tests + { + [SetUp] public void Setup() => Device.PlatformServices = new MockPlatformServices(); + [TearDown] public void TearDown() => Device.PlatformServices = null; + + [Test] + public void InvalidMarkupAssignmentThrowsXPE([Values(false, true)]bool useCompiledXaml) + { + if (useCompiledXaml) + Assert.Throws(() => MockCompiler.Compile(typeof(Gh7187))); + else + Assert.Throws(() => new Gh7187(useCompiledXaml)); + } + } + } +} diff --git a/Xamarin.Forms.Xaml/ExpandMarkupsVisitor.cs b/Xamarin.Forms.Xaml/ExpandMarkupsVisitor.cs index 6e60dcfa..8cf27cf7 100644 --- a/Xamarin.Forms.Xaml/ExpandMarkupsVisitor.cs +++ b/Xamarin.Forms.Xaml/ExpandMarkupsVisitor.cs @@ -7,10 +7,7 @@ namespace Xamarin.Forms.Xaml { class ExpandMarkupsVisitor : IXamlNodeVisitor { - public ExpandMarkupsVisitor(HydrationContext context) - { - Context = context; - } + public ExpandMarkupsVisitor(HydrationContext context) => Context = context; public static readonly IList Skips = new List { @@ -93,8 +90,7 @@ namespace Xamarin.Forms.Xaml expression = expression.Substring(len).TrimStart(); if (expression.Length == 0) { var ex = new XamlParseException("Expression did not end in '}'", xmlLineInfo); - if (Context.ExceptionHandler != null) - { + if (Context.ExceptionHandler != null) { Context.ExceptionHandler(ex); return null; } @@ -108,7 +104,7 @@ namespace Xamarin.Forms.Xaml public class MarkupExpansionParser : MarkupExpressionParser, IExpressionParser { - IElementNode node; + IElementNode _node; internal Action ExceptionHandler { get; set; } object IExpressionParser.Parse(string match, ref string remaining, IServiceProvider serviceProvider) { @@ -117,12 +113,10 @@ namespace Xamarin.Forms.Xaml public INode Parse(string match, ref string remaining, IServiceProvider serviceProvider) { - var nsResolver = serviceProvider.GetService(typeof (IXmlNamespaceResolver)) as IXmlNamespaceResolver; - if (nsResolver == null) + if (!(serviceProvider.GetService(typeof(IXmlNamespaceResolver)) is IXmlNamespaceResolver nsResolver)) throw new ArgumentException(); IXmlLineInfo xmlLineInfo = null; - var xmlLineInfoProvider = serviceProvider.GetService(typeof (IXmlLineInfoProvider)) as IXmlLineInfoProvider; - if (xmlLineInfoProvider != null) + if (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) is IXmlLineInfoProvider xmlLineInfoProvider) xmlLineInfo = xmlLineInfoProvider.XmlLineInfo; var split = match.Split(':'); @@ -130,23 +124,19 @@ namespace Xamarin.Forms.Xaml throw new ArgumentException(); string prefix; //, name; - if (split.Length == 2) - { + if (split.Length == 2) { prefix = split[0]; // name = split [1]; } - else - { + else { prefix = ""; // name = split [0]; } Type type; - var typeResolver = serviceProvider.GetService(typeof (IXamlTypeResolver)) as IXamlTypeResolver; - if (typeResolver == null) + if (!(serviceProvider.GetService(typeof(IXamlTypeResolver)) is IXamlTypeResolver typeResolver)) type = null; - else - { + else { //The order of lookup is to look for the Extension-suffixed class name first and then look for the class name without the Extension suffix. if (!typeResolver.TryResolve(match + "Extension", out type) && !typeResolver.TryResolve(match, out type)) { var ex = new XamlParseException($"MarkupExtension not found for {match}", serviceProvider); @@ -164,37 +154,43 @@ namespace Xamarin.Forms.Xaml if (type == null) throw new NotSupportedException(); - node = xmlLineInfo == null + _node = xmlLineInfo == null ? new ElementNode(xmltype, null, nsResolver) : new ElementNode(xmltype, null, nsResolver, xmlLineInfo.LineNumber, xmlLineInfo.LinePosition); - if (remaining.StartsWith("}", StringComparison.Ordinal)) - { + if (remaining.StartsWith("}", StringComparison.Ordinal)) { remaining = remaining.Substring(1); - return node; + return _node; } - char next; string piece; - while ((piece = GetNextPiece(ref remaining, out next)) != null) + while ((piece = GetNextPiece(ref remaining, out var next)) != null) HandleProperty(piece, serviceProvider, ref remaining, next != '='); - return node; + return _node; } protected override void SetPropertyValue(string prop, string strValue, object value, IServiceProvider serviceProvider) { + if (value == null && strValue == null) { + var xpe = new XamlParseException($"No value found for property '{prop}' in markup expression", serviceProvider); + if (ExceptionHandler != null) { + ExceptionHandler(xpe); + return; + } + throw xpe; + } + var nsResolver = serviceProvider.GetService(typeof (IXmlNamespaceResolver)) as IXmlNamespaceResolver; var childnode = value as INode ?? new ValueNode(strValue, nsResolver); - childnode.Parent = node; - if (prop != null) - { - var name = new XmlName(node.NamespaceURI, prop); - node.Properties[name] = childnode; + childnode.Parent = _node; + if (prop != null) { + var name = new XmlName(_node.NamespaceURI, prop); + _node.Properties[name] = childnode; } else //ContentProperty - node.CollectionItems.Add(childnode); + _node.CollectionItems.Add(childnode); } } }