Skip to content
Merged
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
7 changes: 7 additions & 0 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6190,6 +6190,13 @@
<param name="setupAction">The setup config.</param>
<returns>The built service provider.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.ODataOptions.SanitizeRoutePrefix(System.String)">
Comment thread
gathogojr marked this conversation as resolved.
<summary>
Sanitizes the route prefix by stripping leading and trailing forward slashes.
</summary>
<param name="routePrefix">Route prefix to sanitize.</param>
<returns>Sanitized route prefix.</returns>
</member>
<member name="T:Microsoft.AspNetCore.OData.ODataOptionsSetup">
<summary>
Sets up default options for <see cref="T:Microsoft.AspNetCore.OData.ODataOptions"/>.
Expand Down
43 changes: 38 additions & 5 deletions src/Microsoft.AspNetCore.OData/ODataOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.Contracts;
using Microsoft.AspNetCore.OData.Abstracts;
using Microsoft.AspNetCore.OData.Batch;
Expand Down Expand Up @@ -129,14 +130,22 @@ public ODataOptions AddRouteComponents(string routePrefix, IEdmModel model, Acti
throw Error.ArgumentNull(nameof(model));
}

if (RouteComponents.ContainsKey(routePrefix))
if (routePrefix == null)

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.

Don't we have cases where the prefix is null?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have a case where the prefix is empty (the default case). Not sure about null though.

In any case, you can't add a null key to a dictionary. So I think throwing on null here is the right thing to do.

{
throw Error.InvalidOperation(SRResources.ModelPrefixAlreadyUsed, routePrefix);
throw Error.ArgumentNull(nameof(routePrefix));
}

string sanitizedRoutePrefix = SanitizeRoutePrefix(routePrefix);

if (RouteComponents.ContainsKey(sanitizedRoutePrefix))
{
throw Error.InvalidOperation(SRResources.ModelPrefixAlreadyUsed, sanitizedRoutePrefix);
}


// Consider to use Lazy<IServiceProvider> ?
IServiceProvider serviceProvider = BuildRouteContainer(model, configureServices);
RouteComponents[routePrefix] = (model, serviceProvider);
RouteComponents[sanitizedRoutePrefix] = (model, serviceProvider);
Comment thread
gathogojr marked this conversation as resolved.
return this;
}

Expand All @@ -147,9 +156,16 @@ public ODataOptions AddRouteComponents(string routePrefix, IEdmModel model, Acti
/// <returns>The root service provider for the route (prefix) name.</returns>
public IServiceProvider GetRouteServices(string routePrefix)
{
if (routePrefix != null && RouteComponents.ContainsKey(routePrefix))
if (routePrefix == null)
{
return null;
}

string sanitizedRoutePrefix = SanitizeRoutePrefix(routePrefix);

if (RouteComponents.TryGetValue(sanitizedRoutePrefix, out var components))
{
return RouteComponents[routePrefix].ServiceProvider;
return components.ServiceProvider;
}

return null;
Expand Down Expand Up @@ -300,5 +316,22 @@ private IServiceProvider BuildRouteContainer(IEdmModel model, Action<IServiceCol

return builder.BuildContainer();
}

/// <summary>
/// Sanitizes the route prefix by stripping leading and trailing forward slashes.
/// </summary>
/// <param name="routePrefix">Route prefix to sanitize.</param>
/// <returns>Sanitized route prefix.</returns>
private string SanitizeRoutePrefix(string routePrefix)
{
Debug.Assert(routePrefix != null);

if (routePrefix.Length > 0 && routePrefix[0] != '/' && routePrefix[^1] != '/')
{
return routePrefix;
}

return routePrefix.Trim('/');
}
}
}
53 changes: 53 additions & 0 deletions test/Microsoft.AspNetCore.OData.Tests/ODataOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,25 @@ public void AddRouteComponents_WithDependencyInjection_SetModelAndServices()
Assert.IsType<ODataFeature>(actual);
}

[Theory]
[InlineData("/odata", "odata")]
[InlineData("/odata/", "odata")]
[InlineData("odata/", "odata")]
[InlineData("/", "")]
public void AddRouteComponents_Strips_RoutePrefix_Leading_And_Trailing_Slashes(string routePrefix, string expectedRoutePrefix)
{
// Arrange
ODataOptions options = new ODataOptions();
IEdmModel edmModel = EdmCoreModel.Instance;

// Act
options.AddRouteComponents(routePrefix, edmModel, services => services.AddSingleton<IODataFeature, ODataFeature>());

// Assert
Assert.False(options.RouteComponents.ContainsKey(routePrefix));
Assert.True(options.RouteComponents.ContainsKey(expectedRoutePrefix));
}

[Fact]
public void AddRouteComponents_Throws_IfModelNull()
{
Expand All @@ -150,6 +169,16 @@ public void AddRouteComponents_Throws_IfModelNull()
ExceptionAssert.ThrowsArgumentNull(() => options.AddRouteComponents("odata", null, builder => { }), "model");
}

[Fact]
public void AddRouteComponents_Throws_IfRoutePrefixNull()
{
// Arrange
ODataOptions options = new ODataOptions();

// Act & Assert
ExceptionAssert.ThrowsArgumentNull(() => options.AddRouteComponents(null, EdmCoreModel.Instance, builder => { }), "routePrefix");
}

[Fact]
public void AddRouteComponents_Throws_IfPrefixExisted()
{
Expand Down Expand Up @@ -192,6 +221,30 @@ public void GetRouteServices_ReturnsCorrectServiceProvider()
Assert.NotNull(sp);
}

[Theory]
[InlineData("/odata")]
[InlineData("/odata/")]
[InlineData("odata/")]
public void GetRouteServices_ReturnsCorrectServiceProvider_When_Leading_Or_Trailing_Slashes(string routePrefix)
{
// Arrange
ODataOptions options = new ODataOptions();
IEdmModel edmModel = EdmCoreModel.Instance;

// Act
options.AddRouteComponents(routePrefix, edmModel);

// & Assert
// can retrieve service provider using original routePrefix
IServiceProvider sp = options.GetRouteServices(routePrefix);
Assert.NotNull(sp);

// can retrieve service provider using sanitized routePrefix
string sanitizedRoutePrefix = "odata";
IServiceProvider sp2 = options.GetRouteServices(sanitizedRoutePrefix);
Assert.NotNull(sp2);
}

#region QuerySetting
[Fact]
public void SetMaxTop_Throws_ForWrongValue()
Expand Down