Skip to content

Commit

Permalink
Change query validation to throw ODataException
Browse files Browse the repository at this point in the history
  • Loading branch information
@xuzhg
xuzhg committed Feb 1, 2022
1 parent a2cb05f commit 1360a59
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.OData/Deltas/DeltaOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ private void InitializeProperties(IEnumerable<string> updatableProperties)
}
}

private bool IsIgnoredProperty(bool isTypeDataContract, PropertyInfo propertyInfo)
private static bool IsIgnoredProperty(bool isTypeDataContract, PropertyInfo propertyInfo)
{
//This is for Ignoring the property that matches below criteria
//1. Its marked as NotMapped
Expand Down
35 changes: 21 additions & 14 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1089,20 +1089,6 @@
<param name="clrType">The type to test.</param>
<returns>True if the type is a DateTime; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsDateOnly(System.Type)">
<summary>
Determine if a type is a <see cref="T:System.DateOnly"/>.
</summary>
<param name="clrType">The type to test.</param>
<returns>True if the type is a DateOnly; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsTimeOnly(System.Type)">
<summary>
Determine if a type is a <see cref="T:System.TimeOnly"/>.
</summary>
<param name="clrType">The type to test.</param>
<returns>True if the type is a TimeOnly; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsTimeSpan(System.Type)">
<summary>
Determine if a type is a TimeSpan.
Expand Down Expand Up @@ -6553,6 +6539,11 @@
Looks up a localized string similar to {0} does not support CreateODataValue..
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.SRResources.CustomQueryOptionNotSupportedWithDollarSign">
<summary>
Looks up a localized string similar to Custom query option &apos;{0}&apos; that starts with &apos;$&apos; is not supported..
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.SRResources.DeltaEntityTypeNotAssignable">
<summary>
Looks up a localized string similar to The actual entity type &apos;{0}&apos; is not assignable to the expected type &apos;{1}&apos;..
Expand Down Expand Up @@ -14102,3 +14093,19 @@
</member>
</members>
</doc>
ummary>
<param name="segment">The value segment.</param>
</member>
<member name="P:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.Segment">
<summary>
Gets the value segment.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.GetTemplates(Microsoft.AspNetCore.OData.Routing.ODataRouteOptions)">
<inheritdoc />
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.TryTranslate(Microsoft.AspNetCore.OData.Routing.Template.ODataTemplateTranslateContext)">
<inheritdoc />
</member>
</members>
</doc>
9 changes: 9 additions & 0 deletions src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Microsoft.AspNetCore.OData/Properties/SRResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,9 @@
<data name="NonSelectExpandOnSingleEntity" xml:space="preserve">
<value>The requested resource is not a collection. Query options $filter, $orderby, $count, $skip, and $top can be applied only on collections.</value>
</data>
<data name="CustomQueryOptionNotSupportedWithDollarSign" xml:space="preserve">
<value>Custom query option '{0}' that starts with '$' is not supported.</value>
</data>
<data name="SingleResultHasMoreThanOneEntity" xml:space="preserve">
<value>The action '{0}' on controller '{1}' returned a {2} containing more than one element. {2} must have zero or one elements.</value>
</data>
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.AspNetCore.OData/Query/EnableQueryAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,12 @@ public virtual void ValidateQuery(HttpRequest request, ODataQueryOptions queryOp
{
if (request == null)
{
throw Error.ArgumentNull("request");
throw Error.ArgumentNull(nameof(request));
}

if (queryOptions == null)
{
throw Error.ArgumentNull("queryOptions");
throw Error.ArgumentNull(nameof(queryOptions));
}

IQueryCollection queryParameters = request.Query;
Expand All @@ -700,7 +700,7 @@ public virtual void ValidateQuery(HttpRequest request, ODataQueryOptions queryOp
{
// we don't support any custom query options that start with $
// this should be caught be OnActionExecuted().
throw new ArgumentOutOfRangeException(kvp.Key);
throw new ODataException(Error.Format(SRResources.CustomQueryOptionNotSupportedWithDollarSign, kvp.Key));
}
}

Expand Down
76 changes: 40 additions & 36 deletions test/Microsoft.AspNetCore.OData.Tests/Query/EnableQueryAttributeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,23 @@
//------------------------------------------------------------------------------

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.OData.Extensions;
using Microsoft.AspNetCore.OData.Query;
using Microsoft.AspNetCore.OData.TestCommon;
using Microsoft.AspNetCore.OData.Tests.Commons;
using Microsoft.AspNetCore.OData.Tests.Extensions;
using Microsoft.AspNetCore.OData.Tests.Models;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;
using Microsoft.OData.UriParser;
using Moq;
using Xunit;
Expand All @@ -38,6 +31,8 @@ namespace Microsoft.AspNetCore.OData.Tests.Query
{
public class EnableQueryAttributeTests
{
private static IEdmModel _model = GetEdmModel();

#if false
public static List<Customer> CustomerList = new List<Customer>()
{
Expand Down Expand Up @@ -575,16 +570,15 @@ public void Primitives_Can_Be_Used_For_Top_And_Skip(string filter)
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal(expectedResponse, response.Content);
}
#endif

[Fact]
public void ValidateQuery_Throws_With_Null_Request()
{
// Arrange
EnableQueryAttribute attribute = new EnableQueryAttribute();
var request = RequestFactory.Create();
request.EnableHttpDependencyInjectionSupport();
var model = new ODataModelBuilder().Add_Customer_EntityType().Add_Customers_EntitySet().GetEdmModel();
var options = new ODataQueryOptions(new ODataQueryContext(model, typeof(Builder.TestModels.Customer)), request);
HttpRequest request = new DefaultHttpContext().Request;
var options = new ODataQueryOptions(new ODataQueryContext(EdmCoreModel.Instance, typeof(int)), request);

// Act & Assert
ExceptionAssert.ThrowsArgumentNull(() => attribute.ValidateQuery(null, options), "request");
Expand All @@ -595,9 +589,10 @@ public void ValidateQuery_Throws_WithNullQueryOptions()
{
// Arrange
EnableQueryAttribute attribute = new EnableQueryAttribute();
HttpRequest request = new DefaultHttpContext().Request;

// Act & Assert
ExceptionAssert.ThrowsArgumentNull(() => attribute.ValidateQuery(new HttpRequestMessage(), null), "queryOptions");
ExceptionAssert.ThrowsArgumentNull(() => attribute.ValidateQuery(request, null), "queryOptions");
}

[Theory]
Expand All @@ -609,50 +604,47 @@ public void ValidateQuery_Accepts_All_Supported_QueryNames(string query)
{
// Arrange
EnableQueryAttribute attribute = new EnableQueryAttribute();
HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/?" + query);
request.EnableHttpDependencyInjectionSupport();
DefaultQuerySettings defaultQuerySettings = request.GetConfiguration().GetDefaultQuerySettings();
defaultQuerySettings.EnableFilter = true;
defaultQuerySettings.EnableOrderBy = true;
defaultQuerySettings.MaxTop = null;
HttpRequest request = RequestFactory.Create("Get", "http://localhost/?" + query);

var model = new ODataModelBuilder().Add_Customer_EntityType().Add_Customers_EntitySet().GetEdmModel();
var context = new ODataQueryContext(model, typeof(Builder.TestModels.Customer), null);
var context = new ODataQueryContext(_model, typeof(QCustomer));
context.DefaultQuerySettings.EnableFilter = true;
context.DefaultQuerySettings.EnableOrderBy = true;
context.DefaultQuerySettings.MaxTop = null;
var options = new ODataQueryOptions(context, request);

// Act & Assert
ExceptionAssert.DoesNotThrow(() => attribute.ValidateQuery(request, options));
}

[Fact]
public void ValidateQuery_Sends_BadRequest_For_Unrecognized_QueryNames()
public void ValidateQuery_ThrowsODataException_For_Unrecognized_QueryNames()
{
// Arrange
EnableQueryAttribute attribute = new EnableQueryAttribute();
HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/?$xxx");
request.EnableHttpDependencyInjectionSupport();
var model = new ODataModelBuilder().Add_Customer_EntityType().Add_Customers_EntitySet().GetEdmModel();
var options = new ODataQueryOptions(new ODataQueryContext(model, typeof(Builder.TestModels.Customer)), request);
HttpRequest request = RequestFactory.Create("Get", "http://localhost/?$xxx");
var options = new ODataQueryOptions(new ODataQueryContext(_model, typeof(QCustomer)), request);

// Act & Assert
HttpResponseException responseException = ExceptionAssert.Throws<HttpResponseException>(
() => attribute.ValidateQuery(request, options));
ODataException ex = ExceptionAssert.Throws<ODataException>(
() => attribute.ValidateQuery(request, options));

Assert.Equal(HttpStatusCode.BadRequest, responseException.Response.StatusCode);
Assert.Equal("Custom query option '$xxx' that starts with '$' is not supported.", ex.Message);
}

[Fact]
public void ValidateQuery_Can_Override_Base()
{
// Arrange
Mock<EnableQueryAttribute> mockAttribute = new Mock<EnableQueryAttribute>();
mockAttribute.Setup(m => m.ValidateQuery(It.IsAny<HttpRequestMessage>(), It.IsAny<ODataQueryOptions>())).Callback(() => { }).Verifiable();
mockAttribute.Setup(
m => m.ValidateQuery(It.IsAny<HttpRequest>(), It.IsAny<ODataQueryOptions>())).Callback(() => { }).Verifiable();

// Act & Assert
mockAttribute.Object.ValidateQuery(null, null);
mockAttribute.Verify();
}

#if false
[Fact]
public void ApplyQuery_Throws_With_Null_Queryable()
{
Expand Down Expand Up @@ -1159,7 +1151,7 @@ public async Task OnActionExecuted_SingleResult_WithMoreThanASingleQueryResult_R
"returned a SingleResult containing more than one element. SingleResult must have zero or one elements.",
responseString);
}

#endif
[Theory]
[InlineData("$filter=ID eq 1")]
[InlineData("$orderby=ID")]
Expand All @@ -1168,18 +1160,18 @@ public async Task OnActionExecuted_SingleResult_WithMoreThanASingleQueryResult_R
[InlineData("$top=0")]
public void ValidateSelectExpandOnly_ThrowsODataException_IfODataQueryOptionsHasNonSelectExpand(string parameter)
{
CustomersModelWithInheritance model = new CustomersModelWithInheritance();
model.Model.SetAnnotationValue(model.Customer, new ClrTypeAnnotation(typeof(Customer)));
HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://localhost?" + parameter);
request.EnableHttpDependencyInjectionSupport();
ODataQueryContext context = new ODataQueryContext(model.Model, typeof(Customer));
// Arrange
HttpRequest request = RequestFactory.Create("Get", "http://localhost?" + parameter);
ODataQueryContext context = new ODataQueryContext(_model, typeof(QCustomer));
ODataQueryOptions queryOptions = new ODataQueryOptions(context, request);

// Act & Assert
ExceptionAssert.Throws<ODataException>(
() => EnableQueryAttribute.ValidateSelectExpandOnly(queryOptions),
"The requested resource is not a collection. Query options $filter, $orderby, $count, $skip, and $top can be applied only on collections.");
}

#if false
[Fact]
public void OnActionExecuted_Works_WithPath()
{
Expand Down Expand Up @@ -1249,5 +1241,17 @@ private static HttpActionExecutedContext GetActionExecutedContext<TResponse>(str
return actionExecutedContext;
}
#endif
private static IEdmModel GetEdmModel()
{
var builder = new ODataConventionModelBuilder();
builder.EntitySet<QCustomer>("Customers");
return builder.GetEdmModel();
}

private class QCustomer
{
public int Id { get; set; }
public string Name { get; set; }
}
}
}

0 comments on commit 1360a59

Please sign in to comment.