Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.OData/Query/ODataQueryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ private IDictionary<string, string> GetODataQueryParameters()
}
else
{
if (IsSupportedQueryOption(key))
if (!string.IsNullOrEmpty(key) && IsSupportedQueryOption(key))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it acceptable for the OData endpoint to allow an empty or whitespace-only query option, such as:
GET http://server/service/Customers?%20

Throwing an exception in this case seems reasonable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @WanjohiSammy. I think avoiding the throw is the right choice here, since this is something a client can trigger and it currently results in an internal server error in the controller action, which doesn’t feel appropriate. This also makes the behavior consistent with EnableNoDollarQueryOptions = false, where unknown or empty keys are already ignored.

{
// Normalized the supported system query key by adding the $-prefix if needed.
result.Add(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//-----------------------------------------------------------------------------
// <copyright file="IgnoreEmptyParamsTest.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//--

using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.OData.Routing.Controllers;
using Microsoft.AspNetCore.OData.TestCommon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData.Edm;
using Xunit;

namespace Microsoft.AspNetCore.OData.E2E.Tests.UriParserExtension;

public class IgnoreEmptyParamsTest : WebApiTestBase<IgnoreEmptyParamsTest>
{
public IgnoreEmptyParamsTest(WebApiTestFixture<IgnoreEmptyParamsTest> fixture)
: base(fixture)
{
}

protected static void UpdateConfigureServices(IServiceCollection services)
{
services.ConfigureControllers(typeof(CustomersController), typeof(OrdersController), typeof(MetadataController));

IEdmModel model = UriParserExtenstionEdmModel.GetEdmModel();

services.AddControllers().AddOData(opt =>
{
opt.AddRouteComponents("odata", model).Count().Filter().OrderBy().Select().Expand().SetMaxTop(null);
opt.EnableNoDollarQueryOptions = true;
});
}

public static TheoryDataSet<string, string, string> IgnoreEmptyParamsCases
{
get
{
return new TheoryDataSet<string, string, string>()
{
{ "Get", "Customers?$top=10&$skip=0", "Customers?$top=10&$skip=0&%20" },
{ "Get", "Customers?$select=Name,Id", "Customers?%20=foo&$select=Name,Id" },
{ "Get", "Customers?$orderby=Name", "Customers?%20=foo$orderby=Name" },
{ "Get", "Customers?$filter=Name eq 'test'", "Customers?$filter=Name eq 'test'&" },
{ "Get", "Customers?$count=true", "Customers?%20=%20&$count=true" },

{ "Get", "Customers?top=10&skip=0", "Customers?top=10&skip=0&%20" },
{ "Get", "Customers?select=Name,Id", "Customers?&%20=foo&select=Name,Id" },
{ "Get", "Customers?orderby=Name", "Customers?%20=foo&orderby=Name" },
{ "Get", "Customers?filter=Name eq 'test'", "Customers?filter=Name eq 'test'&" },
{ "Get", "Customers?count=true", "Customers?%20=%20&count=true" },

{ "Get", "Customers", "Customers?%20" },
{ "Get", "Customers(1)", "Customers(1)?=foo" },
};
}
}

[Theory]
[MemberData(nameof(IgnoreEmptyParamsCases))]
public async Task ParserIgnoresEmptyParamsTest(string method, string baselinePath, string emptyParamsPath)
{
// Baseline scenario: query params are all correct
HttpClient client = CreateClient();

var baselineUri = $"odata/{baselinePath}";
HttpRequestMessage request = new HttpRequestMessage(new HttpMethod(method), baselineUri);
HttpResponseMessage response = await client.SendAsync(request);

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
string baselineResponse = await response.Content.ReadAsStringAsync();

// Test scenario: some params are injected in the query string, such as empty spaces
var emptyParamsUri = $"odata/{emptyParamsPath}";
request = new HttpRequestMessage(new HttpMethod(method), emptyParamsUri);
response = await client.SendAsync(request);

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
string emptyParamsResponse = await response.Content.ReadAsStringAsync();

// Expected behavior: empty params are ignored and responses from both scenarios are equivalent
Assert.Equal(baselineResponse, emptyParamsResponse);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,27 @@ public async Task DuplicateUnsupportedQueryParametersIgnoredWithNoException()
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
}

[Theory]
[InlineData("$top=10&$skip=0&%20")]
[InlineData("$top=10&%20=foo&$skip=0")]
[InlineData("$top=10&$skip=0&")]
[InlineData("$top=10&&$skip=0")]
[InlineData("%20")]
public void ODataQueryOptions_CtorDoesNotThrowOnEmptyQueryStringKeys(string oDataQuery)
{
// Arrange
IEdmModel model = GetEdmModelWithoutKey();

string url = "http://server/service/Customers?" + oDataQuery;
HttpRequest request = RequestFactory.Create(HttpMethods.Get, url, opt => opt.EnableNoDollarQueryOptions = true);

// Act
var exception = Record.Exception(() => new ODataQueryOptions(new ODataQueryContext(model, typeof(Customer)), request));

// Assert
Assert.Null(exception);
}

private static HttpClient CreateClient()
{
var controllers = new[] { typeof(EntityModelsController), typeof(ProductsController) };
Expand Down