Skip to content

Commit

Permalink
Add missing PathBase. (#1198)
Browse files Browse the repository at this point in the history 
Co-authored-by: Josh DeGraw <18509575+josh-degraw@users.noreply.github.com>
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
  • Loading branch information
@lucas-zimerman @josh-degraw
@bruno-garcia @getsentry-bot
4 people authored Sep 21, 2021
1 parent 3146b63 commit 130ad42
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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
View file
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
141 changes: 133 additions & 8 deletions test/Sentry.AspNetCore.Tests/Extensions/HttpContextExtensionsTests.cs
View file
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

0 comments on commit 130ad42

Please sign in to comment.