[Xaml] require escaping curly braces (#4723)
authorStephane Delcroix <stephane@delcroix.org>
Wed, 19 Dec 2018 15:14:21 +0000 (16:14 +0100)
committerGitHub <noreply@github.com>
Wed, 19 Dec 2018 15:14:21 +0000 (16:14 +0100)
This is a breaking change as we weren't throwing errors on that before.

- fixes #2756

Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/EmptyViewGalleries/EmptyViewTemplateGallery.xaml
Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml.cs [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Issues/Gh3539.xaml
Xamarin.Forms.Xaml.UnitTests/Issues/Issue1438.xaml
Xamarin.Forms.Xaml.UnitTests/Issues/Issue1549.cs
Xamarin.Forms.Xaml.UnitTests/MarkupExpressionParserTests.cs
Xamarin.Forms.Xaml/MarkupExpressionParser.cs
Xamarin.Forms.Xaml/MarkupExtensionParser.cs

index da35604..b94c268 100644 (file)
@@ -20,7 +20,7 @@
                                                <StackLayout> 
                                                        <Label FontAttributes="Bold" FontSize="18" Margin="10,25,10,10"
                                                                        HorizontalOptions="Fill" HorizontalTextAlignment="Center" 
-                                                                       Text="{Binding Filter, StringFormat='Your filter term of {0} did not match any records'}"></Label>
+                                                                       Text="{Binding Filter, StringFormat='{}Your filter term of {0} did not match any records'}"></Label>
                                                </StackLayout>
                                        </DataTemplate>
                                </CollectionView.EmptyViewTemplate>
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml
new file mode 100644 (file)
index 0000000..c9259f7
--- /dev/null
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+        xmlns="http://xamarin.com/schemas/2014/forms"
+        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+        x:Class="Xamarin.Forms.Xaml.UnitTests.Gh2756">
+    <Label Text="{Binding Path=StartTime, StringFormat='{0:yy-MM-dd}'}"/>
+</ContentPage>
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh2756.xaml.cs
new file mode 100644 (file)
index 0000000..eae564d
--- /dev/null
@@ -0,0 +1,34 @@
+using NUnit.Framework;
+
+using Xamarin.Forms.Core.UnitTests;
+
+using Xamarin.Forms;
+
+namespace Xamarin.Forms.Xaml.UnitTests
+{
+       [XamlCompilation(XamlCompilationOptions.Skip)]
+       public partial class Gh2756 : ContentPage
+       {
+               public Gh2756() => InitializeComponent();
+               public Gh2756(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 UnescapedBraces([Values(false, true)]bool useCompiledXaml)
+                       {
+                               if (useCompiledXaml)
+                                       Assert.Throws<XamlParseException>(() => MockCompiler.Compile(typeof(Gh2756)));
+                               else
+                                       Assert.Throws<XamlParseException>(() => new Gh2756(useCompiledXaml));
+                       }
+               }
+       }
+}
index a6b9da7..90a0b78 100644 (file)
         <StackLayout Margin="10, 0">
             <Label Text="{Binding Name}" />
             <Slider Value="{Binding Hue}" />
-            <Label Text="{Binding Hue, StringFormat='Hue = {0:F2}'}" />
+            <Label Text="{Binding Hue, StringFormat='{}Hue = {0:F2}'}" />
             <Slider Value="{Binding Saturation}" />
-            <Label Text="{Binding Saturation, StringFormat='Saturation = {0:F2}'}" />
+            <Label Text="{Binding Saturation, StringFormat='{}Saturation = {0:F2}'}" />
             <Slider Value="{Binding Luminosity}" />
-            <Label Text="{Binding Luminosity, StringFormat='Luminosity = {0:F2}'}" />
+            <Label Text="{Binding Luminosity, StringFormat='{}Luminosity = {0:F2}'}" />
         </StackLayout>
     </StackLayout> 
 </ContentPage>
index f31c52a..4fb3672 100644 (file)
@@ -1,11 +1,11 @@
-<?xml version="1.0" encoding="UTF-8"?>
+<?xml version="1.0" encoding="UTF-8"?>
 <ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
                         xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
                         x:Class="Xamarin.Forms.Xaml.UnitTests.Issue1438">
        <StackLayout>
                <Label BindingContext="{x:Reference slider}"
                           x:Name="label"
-                          Text="{Binding Value, StringFormat='Slider value is {0:F3}'}"
+                          Text="{Binding Value, StringFormat='{}Slider value is {0:F3}'}"
                           Font="Large"
                           HorizontalOptions="Center"
                           VerticalOptions="CenterAndExpand" />
index 8b5bf30..8e59a30 100644 (file)
@@ -82,7 +82,7 @@ xmlns=""http://xamarin.com/schemas/2014/forms""
 <local:SeverityColorConverter x:Key=""SeverityColorConverter"" />
 </ResourceDictionary>
 </ContentPage.Resources>
-                               <Label Text=""{Binding value, StringFormat='{0}'}"" 
+                               <Label Text=""{Binding value, StringFormat='{}{0}'}"" 
                                        WidthRequest=""50"" 
                                        TextColor=""Black""
                                        x:Name=""label""
index 37ba170..c62523b 100644 (file)
@@ -248,17 +248,25 @@ namespace Xamarin.Forms.Xaml.UnitTests
                }
 
                [Test]
-               public void BindingStringFormatWithoutEscaping ()
+               public void BracesNeedsToBeEscaped()
                {
-                       var bindingString = "{Binding Foo, StringFormat='{0,20}'}";
+                       var bindingString = "{Binding Foo, StringFormat='Hello {0}'}";
 
-                       var binding = (new MarkupExtensionParser ()).ParseExpression (ref bindingString, new Internals.XamlServiceProvider (null, null) {
+                       Assert.Throws<XamlParseException>(() => new MarkupExtensionParser().ParseExpression(ref bindingString, new Internals.XamlServiceProvider(null, null)
+                       {
                                IXamlTypeResolver = typeResolver,
-                       });
+                       }));
+               }
 
-                       Assert.That (binding, Is.InstanceOf<Binding> ());
-                       Assert.AreEqual ("Foo", ((Binding)binding).Path);
-                       Assert.AreEqual ("{0,20}", ((Binding)binding).StringFormat);
+               [Test]
+               public void BracesNeedsToBeEscaped2()
+               {
+                       var bindingString = "{Binding Foo, StringFormat='{0}'}";
+
+                       Assert.Throws<XamlParseException>(() => new MarkupExtensionParser().ParseExpression(ref bindingString, new Internals.XamlServiceProvider(null, null)
+                       {
+                               IXamlTypeResolver = typeResolver,
+                       }));
                }
 
                [Test]
index 9123a72..8473627 100644 (file)
@@ -46,7 +46,10 @@ namespace Xamarin.Forms.Xaml
                                return expression.Substring(2);
 
                        if (expression[expression.Length - 1] != '}')
-                               throw new Exception("Expression must end with '}'");
+                       {
+                               var lineInfo = (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) as IXmlLineInfoProvider != null) ? (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) as IXmlLineInfoProvider).XmlLineInfo : new XmlLineInfo();
+                               throw new XamlParseException("Expression must end with '}'", lineInfo);
+                       }
 
                        int len;
                        string match;
@@ -114,7 +117,6 @@ namespace Xamarin.Forms.Xaml
 
                protected void HandleProperty(string prop, IServiceProvider serviceProvider, ref string remaining, bool isImplicit)
                {
-                       char next;
                        object value = null;
                        string str_value;
 
@@ -137,7 +139,13 @@ namespace Xamarin.Forms.Xaml
                                str_value = value as string;
                        }
                        else
-                               str_value = GetNextPiece(ref remaining, out next);
+                               str_value = GetNextPiece(ref remaining, out var next);
+
+                       if (str_value != null && !str_value.StartsWith("{}", StringComparison.Ordinal) && str_value.Length > 2 && str_value.IndexOf('{') != -1)
+                       {
+                               var lineInfo = (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) as IXmlLineInfoProvider != null) ? (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) as IXmlLineInfoProvider).XmlLineInfo : new XmlLineInfo();
+                               throw new XamlParseException("Strings containing `{` needs to be escaped. Start the string with `{}`", lineInfo);
+                       }
 
                        SetPropertyValue(prop, str_value, value, serviceProvider);
                }
index ca30e3f..39684b2 100644 (file)
@@ -5,27 +5,31 @@ namespace Xamarin.Forms.Xaml
 {
        internal sealed class MarkupExtensionParser : MarkupExpressionParser, IExpressionParser<object>
        {
-               IMarkupExtension markupExtension;
+               IMarkupExtension _markupExtension;
 
                public object Parse(string match, ref string remaining, IServiceProvider serviceProvider)
                {
                        var typeResolver = serviceProvider.GetService(typeof (IXamlTypeResolver)) as IXamlTypeResolver;
-
-                       //shortcut for Binding and StaticResource, to avoid too many reflection calls.
-                       if (match == "Binding")
-                               markupExtension = new BindingExtension();
-                       else if (match == "TemplateBinding")
-                               markupExtension = new TemplateBindingExtension();
-                       else if (match == "StaticResource")
-                               markupExtension = new StaticResourceExtension();
-                       else if (match == "OnPlatform")
-                               markupExtension = new OnPlatformExtension();
-                       else if (match == "OnIdiom")
-                               markupExtension = new OnIdiomExtension();
-                       else if (match == "DataTemplate")
-                               markupExtension = new DataTemplateExtension();
-                       else
-                       {
+                       switch (match) {
+                       case "Binding":
+                               _markupExtension = new BindingExtension();
+                               break;
+                       case "TemplateBinding":
+                               _markupExtension = new TemplateBindingExtension();
+                               break;
+                       case "StaticResource":
+                               _markupExtension = new StaticResourceExtension();
+                               break;
+                       case "OnPlatform":
+                               _markupExtension = new OnPlatformExtension();
+                               break;
+                       case "OnIdiom":
+                               _markupExtension = new OnIdiomExtension();
+                               break;
+                       case "DataTemplate":
+                               _markupExtension = new DataTemplateExtension();
+                               break;
+                       default:
                                if (typeResolver == null)
                                        return null;
                                Type type;
@@ -33,20 +37,21 @@ namespace Xamarin.Forms.Xaml
                                //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))
                                        throw new XamlParseException($"MarkupExtension not found for {match}", serviceProvider);
-                               markupExtension = Activator.CreateInstance(type) as IMarkupExtension;
+                               _markupExtension = Activator.CreateInstance(type) as IMarkupExtension;
+                               break;
                        }
 
-                       if (markupExtension == null)
+                       if (_markupExtension == null)
                                throw new XamlParseException($"Missing public default constructor for MarkupExtension {match}", serviceProvider);
 
                        if (remaining == "}")
-                               return markupExtension.ProvideValue(serviceProvider);
+                               return _markupExtension.ProvideValue(serviceProvider);
 
                        string piece;
                        while ((piece = GetNextPiece(ref remaining, out char next)) != null)
                                HandleProperty(piece, serviceProvider, ref remaining, next != '=');
 
-                       return markupExtension.ProvideValue(serviceProvider);
+                       return _markupExtension.ProvideValue(serviceProvider);
                }
 
                protected override void SetPropertyValue(string prop, string strValue, object value, IServiceProvider serviceProvider)
@@ -54,7 +59,7 @@ namespace Xamarin.Forms.Xaml
                        MethodInfo setter;
                        if (prop == null) {
                                //implicit property
-                               var t = markupExtension.GetType();
+                               var t = _markupExtension.GetType();
                                prop = ApplyPropertiesVisitor.GetContentPropertyName(t.GetTypeInfo());
                                if (prop == null)
                                        return;
@@ -67,23 +72,24 @@ namespace Xamarin.Forms.Xaml
                        }
                        else {
                                try {
-                                       setter = markupExtension.GetType().GetRuntimeProperty(prop).SetMethod;
+                                       setter = _markupExtension.GetType().GetRuntimeProperty(prop).SetMethod;
                                }
                                catch (AmbiguousMatchException e) {
-                                       throw new XamlParseException($"Multiple properties with name  '{markupExtension.GetType()}.{prop}' found.", serviceProvider, innerException: e);
+                                       throw new XamlParseException($"Multiple properties with name  '{_markupExtension.GetType()}.{prop}' found.", serviceProvider, innerException: e);
                                }
 
                        }
                        if (value == null && strValue != null) {
                                try {
-                                       value = strValue.ConvertTo(markupExtension.GetType().GetRuntimeProperty(prop).PropertyType, (Func<TypeConverter>)null, serviceProvider);
+                                       value = strValue.ConvertTo(_markupExtension.GetType().GetRuntimeProperty(prop).PropertyType,
+                                               (Func<TypeConverter>)null, serviceProvider);
                                }
                                catch (AmbiguousMatchException e) {
-                                       throw new XamlParseException($"Multiple properties with name  '{markupExtension.GetType()}.{prop}' found.", serviceProvider, innerException: e);
+                                       throw new XamlParseException($"Multiple properties with name  '{_markupExtension.GetType()}.{prop}' found.", serviceProvider, innerException: e);
                                }
                        }
 
-                       setter.Invoke(markupExtension, new[] { value });
+                       setter.Invoke(_markupExtension, new[] { value });
                }
        }
-}
\ No newline at end of file
+}