Improving Table Filter parameter validation

Azure · Jan 23, 2017 · 50967cb · 50967cb
1 parent 5429d54
commit 50967cb
Show file tree

Hide file tree

Showing 16 changed files with 431 additions and 22 deletions.
diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/AsyncCollector/AsyncCollectorBinding.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/AsyncCollector/AsyncCollectorBinding.cs
@@ -28,6 +28,11 @@ protected override Task<IValueProvider> BuildAsync(TAttribute attrResolved, Valu
         {
             var value = _argumentBuilder(attrResolved, context);
             return Task.FromResult(value);
-        }      
+        }
+
+        protected override Task<IValueProvider> BuildAsync(TAttribute attrResolved, BindingContext context)
+        {
+            return BuildAsync(attrResolved, context.ValueContext);
+        }
     }
 }
diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingBase.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingBase.cs
@@ -42,12 +42,14 @@ public bool FromAttribute
             }
         }
 
+        protected abstract Task<IValueProvider> BuildAsync(TAttribute attrResolved, BindingContext context);
+
         protected abstract Task<IValueProvider> BuildAsync(TAttribute attrResolved, ValueBindingContext context);
 
         public async Task<IValueProvider> BindAsync(BindingContext context)
         {
             var attrResolved = await Cloner.ResolveFromBindingDataAsync(context);
-            return await BuildAsync(attrResolved, context.ValueContext);
+            return await BuildAsync(attrResolved, context);
         }
 
         public async Task<IValueProvider> BindAsync(object value, ValueBindingContext context)

diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingDataPathHelper.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingDataPathHelper.cs
@@ -17,7 +17,7 @@ internal static class BindingDataPathHelper
         /// </summary>
         /// <param name="bindingData">The binding data to convert.</param>
         /// <returns>A collection of path compatible parameters.</returns>
-        public static IReadOnlyDictionary<string, string> ConvertParameters(IReadOnlyDictionary<string, object> bindingData)
+        public static Dictionary<string, string> ConvertParameters(IReadOnlyDictionary<string, object> bindingData)
         {
             if (bindingData == null)
             {

diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingFactory.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingFactory.cs
@@ -168,15 +168,17 @@ public IBindingProvider BindToExactType<TAttribute, TUserType>(Func<TAttribute,
         /// <typeparam name="TUserType">The exact type of the user's parameter that this will bind to.</typeparam>
         /// <param name="buildFromAttribute">builder function to create the object that will get passed to the user function.</param>
         /// <param name="buildParameterDescriptor">An optional function to create a specific ParameterDescriptor object for the dashboard. If missing, a default ParameterDescriptor is created. </param>
-        /// <param name="postResolveHook">an advanced hook for translating the attribute. </param>
+        /// <param name="postResolveHook">an advanced hook for translating the attribute.</param>
+        /// <param name="preBindHook">an advanced hook for performing any per invocation attribute resolution.</param>
         /// <returns>A binding provider that applies these semantics.</returns>
         public IBindingProvider BindToExactAsyncType<TAttribute, TUserType>(
             Func<TAttribute, Task<TUserType>> buildFromAttribute,
             Func<TAttribute, ParameterInfo, INameResolver, ParameterDescriptor> buildParameterDescriptor = null,
-            Func<TAttribute, ParameterInfo, INameResolver, Task<TAttribute>> postResolveHook = null)
+            Func<TAttribute, ParameterInfo, INameResolver, Task<TAttribute>> postResolveHook = null,
+            Action<TAttribute, BindingContext> preBindHook = null)
             where TAttribute : Attribute
         {
-            var bindingProvider = new ExactTypeBindingProvider<TAttribute, TUserType>(_nameResolver, buildFromAttribute, buildParameterDescriptor, postResolveHook);
+            var bindingProvider = new ExactTypeBindingProvider<TAttribute, TUserType>(_nameResolver, buildFromAttribute, buildParameterDescriptor, postResolveHook, preBindHook);
             return bindingProvider;
         }
 

diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/ExactTypeBindingProvider.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/ExactTypeBindingProvider.cs
@@ -16,14 +16,17 @@ internal class ExactTypeBindingProvider<TAttribute, TUserType> : IBindingProvide
         private readonly Func<TAttribute, Task<TUserType>> _buildFromAttr;
         private readonly Func<TAttribute, ParameterInfo, INameResolver, ParameterDescriptor> _buildParameterDescriptor;
         private readonly Func<TAttribute, ParameterInfo, INameResolver, Task<TAttribute>> _postResolveHook;
+        private readonly Action<TAttribute, BindingContext> _preBindHook = null;
 
         public ExactTypeBindingProvider(
             INameResolver nameResolver,
             Func<TAttribute, Task<TUserType>> buildFromAttr,
             Func<TAttribute, ParameterInfo, INameResolver, ParameterDescriptor> buildParameterDescriptor = null,
-            Func<TAttribute, ParameterInfo, INameResolver, Task<TAttribute>> postResolveHook = null)
+            Func<TAttribute, ParameterInfo, INameResolver, Task<TAttribute>> postResolveHook = null,
+            Action<TAttribute, BindingContext> preBindHook = null)
         {
             this._postResolveHook = postResolveHook;
+            this._preBindHook = preBindHook;
             this._nameResolver = nameResolver;
             this._buildParameterDescriptor = buildParameterDescriptor;
             this._buildFromAttr = buildFromAttr;
@@ -72,21 +75,24 @@ public Task<IBinding> TryCreateAsync(BindingProviderContext context)
                 };
             }
 
-            var binding = new ExactBinding(cloner, param, _buildFromAttr);
+            var binding = new ExactBinding(cloner, param, _buildFromAttr, _preBindHook);
             return Task.FromResult<IBinding>(binding);
         }
 
         private class ExactBinding : BindingBase<TAttribute>
         {
             private readonly Func<TAttribute, Task<TUserType>> _buildFromAttr;
+            private readonly Action<TAttribute, BindingContext> _preBindHook = null;
 
             public ExactBinding(
                 AttributeCloner<TAttribute> cloner, 
                 ParameterDescriptor param, 
-                Func<TAttribute, Task<TUserType>> buildFromAttr)
+                Func<TAttribute, Task<TUserType>> buildFromAttr,
+                Action<TAttribute, BindingContext> preBindHook = null)
                 : base(cloner, param)
             {
                 _buildFromAttr = buildFromAttr;
+                _preBindHook = preBindHook;
             }
 
             protected override async Task<IValueProvider> BuildAsync(TAttribute attrResolved, ValueBindingContext context)
@@ -96,7 +102,14 @@ protected override async Task<IValueProvider> BuildAsync(TAttribute attrResolved
 
                 IValueProvider vp = new ConstantValueProvider(obj, typeof(TUserType), invokeString);
                 return vp;
-            }            
+            }
+
+            protected override Task<IValueProvider> BuildAsync(TAttribute attrResolved, BindingContext context)
+            {
+                _preBindHook?.Invoke(attrResolved, context);
+
+                return BuildAsync(attrResolved, context.ValueContext);
+            }
         }
     }
 }
diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/GenericItemBindingProvider.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/GenericItemBindingProvider.cs
@@ -67,6 +67,11 @@ protected override async Task<IValueProvider> BuildAsync(TAttribute attrResolved
                 IValueProvider vp = new ConstantValueProvider(obj, _parameter.ParameterType, invokeString);
                 return vp;
             }
+
+            protected override Task<IValueProvider> BuildAsync(TAttribute attrResolved, BindingContext context)
+            {
+                return BuildAsync(attrResolved, context.ValueContext);
+            }
         }
     }
 }
diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/ItemBindingProvider.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingProviders/ItemBindingProvider.cs
@@ -69,6 +69,11 @@ protected override async Task<IValueProvider> BuildAsync(TAttribute attrResolved
                 return new Wrapper(valueBinder, invokeString);
             }
 
+            protected override Task<IValueProvider> BuildAsync(TAttribute attrResolved, BindingContext context)
+            {
+                return BuildAsync(attrResolved, context.ValueContext);
+            }
+
             private class Wrapper : IValueBinder
             {
                 private readonly IValueBinder _inner;

diff --git a/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingTemplateExtensions.cs b/src/Microsoft.Azure.WebJobs.Host/Bindings/BindingTemplateExtensions.cs
@@ -53,16 +53,34 @@ public static string Bind(this BindingTemplate bindingTemplate, IReadOnlyDiction
                 throw new ArgumentNullException("bindingTemplate");
             }
 
-            if (bindingData == null ||
-                !bindingTemplate.ParameterNames.Any())
+            if (bindingData == null || !bindingTemplate.ParameterNames.Any())
             {
                 return bindingTemplate.Pattern;
             }
 
             IReadOnlyDictionary<string, string> parameters = BindingDataPathHelper.ConvertParameters(bindingData);
-            string path = bindingTemplate.Bind(parameters);
+            return bindingTemplate.Bind(parameters);
+        }
+
+        /// <summary>
+        /// Bind the <see cref="BindingTemplate"/> using the specified binding data.
+        /// </summary>
+        /// <param name="bindingTemplate">The binding template to validate.</param>
+        /// <param name="bindingData">The binding data to apply to the template.</param>
+        /// <returns>The bound template string.</returns>
+        public static string Bind(this BindingTemplate bindingTemplate, IReadOnlyDictionary<string, string> bindingData)
+        {
+            if (bindingTemplate == null)
+            {
+                throw new ArgumentNullException("bindingTemplate");
+            }
+
+            if (bindingData == null || !bindingTemplate.ParameterNames.Any())
+            {
+                return bindingTemplate.Pattern;
+            }
 
-            return path;
+            return bindingTemplate.Bind(bindingData);
         }
 
         private static void ValidateContractCompatibility(IEnumerable<string> parameterNames, IReadOnlyDictionary<string, Type> bindingDataContract)

diff --git a/src/Microsoft.Azure.WebJobs.Host/GlobalSuppressions.cs b/src/Microsoft.Azure.WebJobs.Host/GlobalSuppressions.cs
@@ -22,4 +22,8 @@
 [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "0", Scope = "member", Target = "Microsoft.Azure.WebJobs.IConverterManagerExtensions.#AddConverter`2(Microsoft.Azure.WebJobs.IConverterManager,System.Func`2<!!0,!!1>)")]
 [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "0", Scope = "member", Target = "Microsoft.Azure.WebJobs.IConverterManagerExtensions.#AddConverter`3(Microsoft.Azure.WebJobs.IConverterManager,System.Func`3<!!0,!!2,!!1>)")]
 [assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1005:AvoidExcessiveParametersOnGenericTypes", Scope = "type", Target = "Microsoft.Azure.WebJobs.FuncConverter`3")]
-[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "WindowsAzure", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.AzureStorageDeploymentValidator.#Validate()")]
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "WindowsAzure", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.AzureStorageDeploymentValidator.#Validate()")]
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "1", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.Bindings.AsyncCollectorBinding`2.#BuildAsync(!0,Microsoft.Azure.WebJobs.Host.Bindings.BindingContext)")]
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "1", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.Bindings.ExactTypeBindingProvider`2+ExactBinding.#BuildAsync(!0,Microsoft.Azure.WebJobs.Host.Bindings.BindingContext)")]
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "1", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.Bindings.GenericItemBindingProvider`1+Binding.#BuildAsync(!0,Microsoft.Azure.WebJobs.Host.Bindings.BindingContext)")]
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "1", Scope = "member", Target = "Microsoft.Azure.WebJobs.Host.Bindings.ItemBindingProvider`1+Binding.#BuildAsync(!0,Microsoft.Azure.WebJobs.Host.Bindings.BindingContext)")]
diff --git a/src/Microsoft.Azure.WebJobs.Host/Tables/TableAttributeBindingProvider.cs b/src/Microsoft.Azure.WebJobs.Host/Tables/TableAttributeBindingProvider.cs
@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.WebJobs.Host.Bindings;
+using Microsoft.Azure.WebJobs.Host.Bindings.Path;
 using Microsoft.Azure.WebJobs.Host.Executors;
 using Microsoft.Azure.WebJobs.Host.Storage;
 using Microsoft.Azure.WebJobs.Host.Storage.Table;
@@ -52,7 +53,7 @@ private TableAttributeBindingProvider(INameResolver nameResolver, IStorageAccoun
                 new PocoEntityArgumentBindingProvider()); // Supports all types; must come after other providers
         }
 
-        // [Table] has some pre-existing behavior where the storage account can be specified outside of the [Queue] attribute. 
+        // [Table] has some pre-existing behavior where the storage account can be specified outside of the [Table] attribute. 
         // The storage account is pulled from the ParameterInfo (which could pull in a [Storage] attribute on the container class)
         // Resolve everything back down to a single attribute so we can use the binding helpers. 
         // This pattern should be rare since other extensions can just keep everything directly on the primary attribute. 
@@ -79,7 +80,24 @@ public static IBindingProvider Build(INameResolver nameResolver, IConverterManag
             var bindAsyncCollector = bindingFactory.BindToAsyncCollector<TableAttribute, ITableEntity>(original.BuildFromTableAttribute, null, original.CollectAttributeInfo);
 
             var bindToJobject = bindingFactory.BindToExactAsyncType<TableAttribute, JObject>(original.BuildJObject, null, original.CollectAttributeInfo);
-            var bindToJArray = bindingFactory.BindToExactAsyncType<TableAttribute, JArray>(original.BuildJArray, null, original.CollectAttributeInfo);
+
+            BindingTemplate bindingTemplate = null;
+            Func<TableAttribute, ParameterInfo, INameResolver, Task<TableAttribute>> postResolveHook = (attribute, parameterInfo, resolver) =>
+            {
+                if (!string.IsNullOrEmpty(attribute.Filter))
+                {
+                    bindingTemplate = BindingTemplate.FromString(attribute.Filter);
+                }
+                return original.CollectAttributeInfo(attribute, parameterInfo, nameResolver);
+            };
+            Action<TableAttribute, BindingContext> preBindHook = (attribute, context) =>
+            {
+                if (bindingTemplate != null)
+                {
+                    attribute.Filter = TableFilterFormatter.Format(bindingTemplate, context.BindingData);
+                }
+            };
+            var bindToJArray = bindingFactory.BindToExactAsyncType<TableAttribute, JArray>(original.BuildJArray, null, postResolveHook, preBindHook);
 
             // Filter to just support JObject, and use legacy bindings for everything else. 
             // Once we have ITableEntity converters for pocos, we can remove the filter. 
@@ -93,7 +111,7 @@ public static IBindingProvider Build(INameResolver nameResolver, IConverterManag
 
             return bindingProvider;
         }
-        
+
         private async Task<JObject> BuildJObject(TableAttribute attribute)
         {
             IStorageTable table = GetTable(attribute);

diff --git a/src/Microsoft.Azure.WebJobs.Host/Tables/TableFilterFormatter.cs b/src/Microsoft.Azure.WebJobs.Host/Tables/TableFilterFormatter.cs
@@ -0,0 +1,88 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using System.Text;
+using Microsoft.Azure.WebJobs.Host.Bindings;
+using Microsoft.Azure.WebJobs.Host.Bindings.Path;
+
+namespace Microsoft.Azure.WebJobs.Host.Tables
+{
+    internal static class TableFilterFormatter
+    {
+        private static char[] _specialChars;
+
+        static TableFilterFormatter()
+        {
+            _specialChars = new char[] { ' ', '\'', ')', '(' };
+        }
+
+        public static string Format(BindingTemplate template, IReadOnlyDictionary<string, object> bindingData)
+        {
+            if (!template.ParameterNames.Any())
+            {
+                return template.Pattern;
+            }
+
+            var convertedBindingData = BindingDataPathHelper.ConvertParameters(bindingData);
+
+            // each distinct parameter can occur one or more times in the template
+            var parameterGroups = template.ParameterNames.GroupBy(p => p);
+
+            // for each parameter, classify it as a string literal or other
+            // and perform value validation
+            foreach (var parameterGroup in parameterGroups)
+            {
+                // to classify as a string literal, ALL occurrences in the template
+                // must be string literals (e.g. of the form '{p}')
+                // note that this will also capture OData expressions of the form
+                // datetime'{p}', guid'{p}', X'{p}'
+                bool isStringLiteral = true;
+                string parameterName = parameterGroup.Key;
+                string stringParameterFormat = $"'{{{parameterName}}}'";
+                int count = 0, idx = 0; 
+                while (idx >= 0 && idx < template.Pattern.Length && count++ < parameterGroup.Count())
+                {
+                    idx = template.Pattern.IndexOf(stringParameterFormat, idx, StringComparison.OrdinalIgnoreCase);
+                    if (idx < 0)
+                    {
+                        isStringLiteral = false;
+                        break;
+                    }
+                    idx++;
+                }
+
+                // validate and format the value based on its classification
+                string value = null;
+                if (convertedBindingData.TryGetValue(parameterName, out value))
+                {
+                    if (isStringLiteral)
+                    {
+                        // ensure we escape any single quotes
+                        convertedBindingData[parameterName] = value.Replace("'", "''");
+                    }
+                    else if (!TryValidateNonStringLiteral(value))
+                    {
+                        throw new InvalidOperationException($"An invalid parameter value was specified for filter parameter '{parameterName}'.");
+                    }
+                }
+            }
+
+            return template.Bind(convertedBindingData);
+        }
+
+        internal static bool TryValidateNonStringLiteral(string value)
+        {
+            // verify that there are no special characters
+            if (_specialChars.Any(p => value.Contains(p)))
+            {
+                return false;
+            }
+
+            return true;
+        }
+    }
+}
diff --git a/src/Microsoft.Azure.WebJobs.Host/WebJobs.Host.csproj b/src/Microsoft.Azure.WebJobs.Host/WebJobs.Host.csproj
@@ -423,6 +423,7 @@
     <Compile Include="Storage\IStorageAccountExtensions.cs" />
     <Compile Include="Storage\StorageAccountType.cs" />
     <Compile Include="Queues\PoisonQueueMessageEventArgs.cs" />
+    <Compile Include="Tables\TableFilterFormatter.cs" />
     <Compile Include="Triggers\StrategyTriggerBinding.cs" />
     <Compile Include="Bindings\ConstantValueProvider.cs" />
     <Compile Include="Bindings\ConverterManager.cs" />

diff --git a/src/Microsoft.Azure.WebJobs/TableAttribute.cs b/src/Microsoft.Azure.WebJobs/TableAttribute.cs
@@ -91,7 +91,6 @@ public string RowKey
         /// <summary>
         /// Allow arbitrary table filter. RowKey should be null. 
         /// </summary>
-        [AutoResolve]
         public string Filter
         {
             get; set;