Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing PathBase. #1198

Merged
merged 17 commits into from
Sep 21, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

- Add missing PathBase from ASP.NET Core ([#1198](https://github.com/getsentry/sentry-dotnet/pull/1198))
- Use fallback if route pattern is MVC ([#1188](https://github.com/getsentry/sentry-dotnet/pull/1188))
- Move UseSentryTracing to different namespace ([#1200](https://github.com/getsentry/sentry-dotnet/pull/1200))
- Prevent duplicate package reporting ([#1197](https://github.com/getsentry/sentry-dotnet/pull/1197))
Expand Down
56 changes: 47 additions & 9 deletions src/Sentry.AspNetCore/Extensions/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Text;
using System.Text.RegularExpressions;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
Expand All @@ -18,16 +19,39 @@ internal static class HttpContextExtensions
var endpoint = context.Features.Get<IEndpointFeature?>()?.Endpoint as RouteEndpoint;
var routePattern = endpoint?.RoutePattern.RawText;

if (!string.IsNullOrWhiteSpace(routePattern))
if (NewRouteFormat(routePattern, context) is { } formattedRoute)
{
// Skip route pattern if it resembles to a MVC route or null e.g.
// {controller=Home}/{action=Index}/{id?}
return RouteHasMvcParameters(routePattern)
? ReplaceMvcParameters(routePattern, context)
: routePattern;
return formattedRoute;
}
#endif
return LegacyRouteFormat(context);
}

// Internal for testing.
internal static string? NewRouteFormat(string? routePattern, HttpContext context)
{
if (string.IsNullOrWhiteSpace(routePattern))
{
return null;
}

var builder = new StringBuilder();
if (context.Request.PathBase.HasValue)
{
builder.Append(context.Request.PathBase.Value?.TrimStart('/'))
.Append('/');
}

// Skip route pattern if it resembles to a MVC route or null e.g.
// {controller=Home}/{action=Index}/{id?}
return RouteHasMvcParameters(routePattern)
? builder.Append(ReplaceMvcParameters(routePattern, context)).ToString()
: builder.Append(routePattern).ToString();
}

// Internal for testing.
internal static string? LegacyRouteFormat(HttpContext context)
{
// Fallback for legacy .UseMvc().
// Uses context.Features.Get<IRoutingFeature?>() under the hood and CAN be null,
// despite the annotations claiming otherwise.
Expand All @@ -39,9 +63,23 @@ internal static class HttpContextExtensions

if (!string.IsNullOrWhiteSpace(action))
{
return !string.IsNullOrWhiteSpace(area)
? $"{area}.{controller}.{action}"
: $"{controller}.{action}";
var builder = new StringBuilder();
if (context.Request.PathBase.HasValue)
{
builder.Append(context.Request.PathBase.Value?.TrimStart('/'))
.Append('.');
}

if (!string.IsNullOrWhiteSpace(area))
{
builder.Append(area)
.Append('.');
}

builder.Append(controller)
.Append('.')
.Append(action);
return builder.ToString();
}

// If the handler doesn't use routing (i.e. it checks `context.Request.Path` directly),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,53 @@ namespace Sentry.AspNetCore.Tests.Extensions
{
public class HttpContextExtensionsTests
{
private static void AddRouteValuesIfNotNull(RouteValueDictionary route, string key, string value)
private class Fixture
{
if (value is not null)
public HttpContext GetSut(string pathBase = null)
{
route.Add(key, value);
var httpContext = new DefaultHttpContext();
if (pathBase is not null)
{
// pathBase must start with '/' otherwise the new PathString will throw an exception.
httpContext.Request.PathBase = new PathString(pathBase);
}
return httpContext;
}

public HttpContext GetMvcSut(
string area = null,
string controller = null,
string action = null,
string pathBase = null)
{
var httpContext = new DefaultHttpContext();
if (pathBase is not null)
{
// pathBase must start with '/' otherwise the new PathString will throw an exception.
httpContext.Request.PathBase = new PathString(pathBase);
}

AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "controller", controller);
AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "action", action);
AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "area", area);
return httpContext;
}

private static void AddRouteValuesIfNotNull(RouteValueDictionary route, string key, string value)
{
if (value is not null)
{
route.Add(key, value);
}
}
}

private readonly Fixture _fixture = new();

private string LegacyFormat(string controller, string action, string area)
=> !string.IsNullOrWhiteSpace(area) ?
$"{area}.{controller}.{action}" : $"{controller}.{action}";

[Theory]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "theArea/house/about/{id?}", "house", "about", "theArea")]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "{area=MyArea}/house/about/{id?}", "house", "about", null)]
Expand All @@ -26,13 +65,10 @@ private static void AddRouteValuesIfNotNull(RouteValueDictionary route, string k
[InlineData("{action=Index}/{id?}", "about/{id?}", null, "about", null)]
[InlineData("not/mvc/", "not/mvc/", "house", "about", "area")]
[InlineData("not/mvc/{controller}/{action}/{area}", "not/mvc/{controller}/{action}/{area}", "house", "about", "area")]
public void ReplaceMcvParameters_ParsedParameters(string routeInput, string assertOutput, string context, string action, string area)
public void ReplaceMcvParameters_ParsedParameters(string routeInput, string assertOutput, string controller, string action, string area)
{
// Arrange
var httpContext = new DefaultHttpContext();
AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "controller", context);
AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "action", action);
AddRouteValuesIfNotNull(httpContext.Request.RouteValues, "area", area);
var httpContext = _fixture.GetMvcSut(area, controller, action);

// Act
var filteredRoute = HttpContextExtensions.ReplaceMvcParameters(routeInput, httpContext);
Expand Down Expand Up @@ -67,6 +103,95 @@ public void RouteHasMvcParameters_RouteWithoutMvcParameters_False(string route)
// Assert
Assert.False(HttpContextExtensions.RouteHasMvcParameters(route));
}

[Theory]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "myPath/theArea/house/about/{id?}", "house", "about", "theArea")]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "myPath/{area=MyArea}/house/about/{id?}", "house", "about", null)]
[InlineData("{area=}/{controller=}/{action=}/{id?}", "myPath/{area=}/{controller=}/{action=}/{id?}", "house", "about", "theArea")]
[InlineData("{controller=Home}/{action=Index}/{id?}", "myPath/house/about/{id?}", "house", "about", null)]
[InlineData("{controller=Home}/{action=Index}", "myPath/house/about", "house", "about", null)]
[InlineData("{controller=Home}/{id?}", "myPath/house/{id?}", "house", "about", null)]
[InlineData("{action=Index}/{id?}", "myPath/about/{id?}", null, "about", null)]
public void NewRouteFormat_MvcRouteWithPathBase_ParsedParameters(string routeInput, string expectedOutput, string controller, string action, string area)
{
// Arrange
var httpContext = _fixture.GetMvcSut(area, controller, action, pathBase: "/myPath");

// Act
var filteredRoute = HttpContextExtensions.NewRouteFormat(routeInput, httpContext);

// Assert
Assert.Equal(expectedOutput, filteredRoute);
}

[Theory]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "theArea/house/about/{id?}", "house", "about", "theArea")]
[InlineData("{area=MyArea}/{controller=Home}/{action=Index}/{id?}", "{area=MyArea}/house/about/{id?}", "house", "about", null)]
[InlineData("{area=}/{controller=}/{action=}/{id?}", "{area=}/{controller=}/{action=}/{id?}", "house", "about", "theArea")]
[InlineData("{controller=Home}/{action=Index}/{id?}", "house/about/{id?}", "house", "about", null)]
[InlineData("{controller=Home}/{action=Index}", "house/about", "house", "about", null)]
[InlineData("{controller=Home}/{id?}", "house/{id?}", "house", "about", null)]
[InlineData("{action=Index}/{id?}", "about/{id?}", null, "about", null)]
public void NewRouteFormat_MvcRouteWithoutPathBase_ParsedParameters(string routeInput, string expectedOutput, string controller, string action, string area)
{
// Arrange
var httpContext = _fixture.GetMvcSut(area, controller, action);

// Act
var filteredRoute = HttpContextExtensions.NewRouteFormat(routeInput, httpContext);

// Assert
Assert.Equal(expectedOutput, filteredRoute);
}

[Theory]
[InlineData("myPath/some/Path", "/myPath", "some/Path")]
[InlineData("some/Path", null, "some/Path")]
[InlineData(null, null, "")]
[InlineData(null, null, null)]
public void NewRouteFormat_WithPathBase_MatchesExpectedRoute(string expectedRoute, string pathBase, string rawRoute)
{
// Arrange
var httpContext = _fixture.GetSut(pathBase);

// Act
var filteredRoute = HttpContextExtensions.NewRouteFormat(rawRoute, httpContext);

// Assert
Assert.Equal(expectedRoute, filteredRoute);
}

[Theory]
[InlineData("myPath.myArea.myController.myAction", "/myPath", "myController", "myAction", "myArea")]
[InlineData("myArea.myController.myAction", null, "myController", "myAction", "myArea")]
[InlineData("myController.myAction", null, "myController", "myAction", null)]
[InlineData(null, null, null, null, null)]
public void LegacyRouteFormat_WithPathBase_MatchesExcpectedRoute(string expectedRoute, string pathBase, string controller, string action, string area)
{
// Arrange
var httpContext = _fixture.GetMvcSut(area, controller, action, pathBase);

// Act
var filteredRoute = HttpContextExtensions.LegacyRouteFormat(httpContext);

// Assert
Assert.Equal(expectedRoute, filteredRoute);
}

[Theory]
[InlineData("myController", "myAction", "myArea")]
[InlineData("myController", "myAction", null)]
public void LegacyRouteFormat_ValidRoutes_MatchPreviousImplementationResult(string controller, string action, string area)
{
// Arrange
var httpContext = _fixture.GetMvcSut(area, controller, action);

// Act
var filteredRoute = HttpContextExtensions.LegacyRouteFormat(httpContext);

// Assert
Assert.Equal(LegacyFormat(controller, action, area), filteredRoute);
}
}
}
#endif