Incorporate code review feedback.

5 files changed, 121 insertions, 61 deletions

diff --git a/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs b/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
index 5329bd30..5a2c19b8 100644
--- a/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
+++ b/src/System.Web.Http.OData/OData/Query/Expressions/FilterBinder.cs
@@ -31,7 +31,7 @@ namespace System.Web.Http.OData.Query.Expressions
         /// restrict the maximum number of expressions that we generate to prevent DoS attacks.
         /// </summary>
         private const int MaxBindCount = 100;
-        private int _currentBindCount = 0;
+        private int _currentBindCount;
         private int _currentLambdaNestingCount;
 
         private static readonly MethodInfo _stringCompareMethodInfo = typeof(string).GetMethod("Compare", new[] { typeof(string), typeof(string), typeof(StringComparison) });
@@ -806,38 +806,44 @@ namespace System.Web.Http.OData.Query.Expressions
         {
             EnterLambda();
 
-            ParameterExpression allIt = HandleLambdaParameters(allQueryNode.Parameters);
+            Expression returnValue;
 
-            Expression source;
-            Contract.Assert(allQueryNode.Source != null);
-            source = Bind(allQueryNode.Source);
+            try
+            {
+                ParameterExpression allIt = HandleLambdaParameters(allQueryNode.Parameters);
 
-            Expression body = source;
-            Contract.Assert(allQueryNode.Body != null);
+                Expression source;
+                Contract.Assert(allQueryNode.Source != null);
+                source = Bind(allQueryNode.Source);
 
-            body = Bind(allQueryNode.Body);
-            body = ApplyNullPropagationForFilterBody(body);
-            body = Expression.Lambda(body, allIt);
+                Expression body = source;
+                Contract.Assert(allQueryNode.Body != null);
 
-            Expression all = All(source, body);
-            Expression returnValue;
+                body = Bind(allQueryNode.Body);
+                body = ApplyNullPropagationForFilterBody(body);
+                body = Expression.Lambda(body, allIt);
 
-            if (_querySettings.HandleNullPropagation == HandleNullPropagationOption.True && IsNullable(source.Type))
-            {
-                // IFF(source == null) null; else Any(body);
-                all = ToNullable(all);
-                returnValue = Expression.Condition(
-                    test: Expression.Equal(source, _nullConstant),
-                    ifTrue: Expression.Constant(null, all.Type),
-                    ifFalse: all);
+                Expression all = All(source, body);
+
+                if (_querySettings.HandleNullPropagation == HandleNullPropagationOption.True && IsNullable(source.Type))
+                {
+                    // IFF(source == null) null; else Any(body);
+                    all = ToNullable(all);
+                    returnValue = Expression.Condition(
+                        test: Expression.Equal(source, _nullConstant),
+                        ifTrue: Expression.Constant(null, all.Type),
+                        ifFalse: all);
+                }
+                else
+                {
+                    returnValue = all;
+                }
             }
-            else
+            finally
             {
-                returnValue = all;
+                ExitLambda();
             }
 
-            ExitLambda();
-
             return returnValue;
         }
 
@@ -845,40 +851,46 @@ namespace System.Web.Http.OData.Query.Expressions
         {
             EnterLambda();
 
-            ParameterExpression anyIt = HandleLambdaParameters(anyQueryNode.Parameters);
-
-            Expression source;
-            Contract.Assert(anyQueryNode.Source != null);
-            source = Bind(anyQueryNode.Source);
+            Expression returnValue;
 
-            Expression body = null;
-            // uri parser places an Constant node with value true for empty any() body
-            if (anyQueryNode.Body != null && anyQueryNode.Body.Kind != QueryNodeKind.Constant)
+            try
             {
-                body = Bind(anyQueryNode.Body);
-                body = ApplyNullPropagationForFilterBody(body);
-                body = Expression.Lambda(body, anyIt);
-            }
+                ParameterExpression anyIt = HandleLambdaParameters(anyQueryNode.Parameters);
 
-            Expression any = Any(source, body);
-            Expression returnValue;
+                Expression source;
+                Contract.Assert(anyQueryNode.Source != null);
+                source = Bind(anyQueryNode.Source);
 
-            if (_querySettings.HandleNullPropagation == HandleNullPropagationOption.True && IsNullable(source.Type))
-            {
-                // IFF(source == null) null; else Any(body);
-                any = ToNullable(any);
-                returnValue = Expression.Condition(
-                    test: Expression.Equal(source, _nullConstant),
-                    ifTrue: Expression.Constant(null, any.Type),
-                    ifFalse: any);
+                Expression body = null;
+                // uri parser places an Constant node with value true for empty any() body
+                if (anyQueryNode.Body != null && anyQueryNode.Body.Kind != QueryNodeKind.Constant)
+                {
+                    body = Bind(anyQueryNode.Body);
+                    body = ApplyNullPropagationForFilterBody(body);
+                    body = Expression.Lambda(body, anyIt);
+                }
+
+                Expression any = Any(source, body);
+
+                if (_querySettings.HandleNullPropagation == HandleNullPropagationOption.True && IsNullable(source.Type))
+                {
+                    // IFF(source == null) null; else Any(body);
+                    any = ToNullable(any);
+                    returnValue = Expression.Condition(
+                        test: Expression.Equal(source, _nullConstant),
+                        ifTrue: Expression.Constant(null, any.Type),
+                        ifFalse: any);
+                }
+                else
+                {
+                    returnValue = any;
+                }
             }
-            else
+            finally
             {
-                returnValue = any;
+                ExitLambda();
             }
 
-            ExitLambda();
-
             return returnValue;
         }
 
@@ -922,26 +934,28 @@ namespace System.Web.Http.OData.Query.Expressions
 
         private void IncrementBindCount()
         {
-            if (++_currentBindCount > MaxBindCount)
+            if (_currentBindCount >= MaxBindCount)
             {
                 throw new ODataException(SRResources.RecursionLimitExceeded);
             }
+
+            _currentBindCount++;
         }
 
         private void EnterLambda()
         {
             if (_currentLambdaNestingCount >= _querySettings.LambdaNestingLimit)
             {
-                throw new ODataException(SRResources.LambdaNestingLimitExceeded);
+                throw new ODataException(Error.Format(SRResources.LambdaNestingLimitExceeded, _querySettings.LambdaNestingLimit));
             }
 
-            ++_currentLambdaNestingCount;
+            _currentBindCount++;
         }
 
         private void ExitLambda()
         {
             Contract.Assert(_currentLambdaNestingCount > 0);
-            --_currentLambdaNestingCount;
+            _currentLambdaNestingCount--;
         }
 
         // If the expression is of non-standard edm primitive type (like uint), convert the expression to its standard edm type.
diff --git a/src/System.Web.Http.OData/OData/Query/ODataQuerySettings.cs b/src/System.Web.Http.OData/OData/Query/ODataQuerySettings.cs
index afb43538..26f299aa 100644
--- a/src/System.Web.Http.OData/OData/Query/ODataQuerySettings.cs
+++ b/src/System.Web.Http.OData/OData/Query/ODataQuerySettings.cs
@@ -8,6 +8,8 @@ namespace System.Web.Http.OData.Query
     public class ODataQuerySettings
     {
         private HandleNullPropagationOption _handleNullPropagationOption = HandleNullPropagationOption.Default;
+        private int _lambdaNestingLimit = 1;
+        private int? _resultLimit;
 
         /// <summary>
         /// Instantiates a new instance of the <see cref="ODataQuerySettings"/> class
@@ -16,7 +18,6 @@ namespace System.Web.Http.OData.Query
         public ODataQuerySettings()
         {
             EnsureStableOrdering = true;
-            LambdaNestingLimit = 1;
         }
 
         /// <summary>
@@ -60,7 +61,22 @@ namespace System.Web.Http.OData.Query
         /// <value>
         /// The maxiumum depth of the Any or All elements nested inside the query.
         /// </value>
-        public int LambdaNestingLimit { get; set; }
+        public int LambdaNestingLimit
+        {
+            get
+            {
+                return _lambdaNestingLimit;
+            }
+            set
+            {
+                if (value <= 0)
+                {
+                    throw Error.ArgumentMustBeGreaterThanOrEqualTo("value", value, 1);
+                }
+
+                _lambdaNestingLimit = value;
+            }
+        }
 
         /// <summary>
         /// Gets or sets the maximum number of query results to return.
@@ -68,6 +84,21 @@ namespace System.Web.Http.OData.Query
         /// <value>
         /// The maximum number of query results to return, or <c>null</c> if there is no limit.
         /// </value>
-        public int? ResultLimit { get; set; }
+        public int? ResultLimit
+        {
+            get
+            {
+                return _resultLimit;
+            }
+            set
+            {
+                if (value.HasValue && value <= 0)
+                {
+                    throw Error.ArgumentMustBeGreaterThanOrEqualTo("value", value, 1);
+                }
+
+                _resultLimit = value;
+            }
+        }
     }
 }
diff --git a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
index ac4718f4..f3b9c3a9 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
+++ b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
@@ -556,7 +556,7 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to The Any/All nesting limit has been exceeded..
+        ///   Looks up a localized string similar to The Any/All nesting limit of {0} has been exceeded. LambdaNestingLimit can be configured on ODataQuerySettings or QueryableAttribute..
         /// </summary>
         internal static string LambdaNestingLimitExceeded {
             get {
diff --git a/src/System.Web.Http.OData/Properties/SRResources.resx b/src/System.Web.Http.OData/Properties/SRResources.resx
index 9cc8edf2..9b0e5ca8 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.resx
+++ b/src/System.Web.Http.OData/Properties/SRResources.resx
@@ -439,7 +439,7 @@
     <value>Queries can not be applied to a response content of type '{0}'. The response content must be an ObjectContent.</value>
   </data>
   <data name="LambdaNestingLimitExceeded" xml:space="preserve">
-    <value>The Any/All nesting limit has been exceeded.</value>
+    <value>The Any/All nesting limit of {0} has been exceeded. LambdaNestingLimit can be configured on ODataQuerySettings or QueryableAttribute.</value>
   </data>
   <data name="NoODataFormatterForMetadata" xml:space="preserve">
     <value>No OData formatter was found to write the OData metadata. Consider registering an appropriate ODataMediaTypeFormatter on the configuration's formatter collection.</value>
diff --git a/src/System.Web.Http.OData/QueryableAttribute.cs b/src/System.Web.Http.OData/QueryableAttribute.cs
index bf974591..edc8cf13 100644
--- a/src/System.Web.Http.OData/QueryableAttribute.cs
+++ b/src/System.Web.Http.OData/QueryableAttribute.cs
@@ -22,6 +22,7 @@ namespace System.Web.Http
     public class QueryableAttribute : ActionFilterAttribute
     {
         private HandleNullPropagationOption _handleNullPropagationOption = HandleNullPropagationOption.Default;
+        private int _lambdaNestingLimit = 1;
         private int? _resultLimit;
 
         /// <summary>
@@ -30,7 +31,6 @@ namespace System.Web.Http
         public QueryableAttribute()
         {
             EnsureStableOrdering = true;
-            LambdaNestingLimit = 1;
         }
 
         /// <summary>
@@ -74,7 +74,22 @@ namespace System.Web.Http
         /// <value>
         /// The maxiumum depth of the Any or All elements nested inside the query.
         /// </value>
-        public int LambdaNestingLimit { get; set; }
+        public int LambdaNestingLimit
+        {
+            get
+            {
+                return _lambdaNestingLimit;
+            }
+            set
+            {
+                if (value <= 0)
+                {
+                    throw Error.ArgumentMustBeGreaterThanOrEqualTo("value", value, 1);
+                }
+
+                _lambdaNestingLimit = value;
+            }
+        }
 
         /// <summary>
         /// Gets or sets the maximum number of query results to send back to clients.