diff --git a/eng/Baseline.Designer.props b/eng/Baseline.Designer.props index 44b6452388..78f2952da7 100644 --- a/eng/Baseline.Designer.props +++ b/eng/Baseline.Designer.props @@ -2,7 +2,7 @@ $(MSBuildAllProjects);$(MSBuildThisFileFullPath) - 3.1.3 + 3.1.4 @@ -16,92 +16,92 @@ - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - - - + + + - + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 @@ -109,39 +109,39 @@ - 3.1.3 + 3.1.4 - - - + + + - - - + + + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + @@ -184,277 +184,277 @@ - 3.1.3 + 3.1.4 - - - + + + - - - + + + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - - + + - - + + - 3.1.3 + 3.1.4 - + - + - 3.1.3 + 3.1.4 - - - - + + + + - - - - + + + + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - + - - + + - + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - + - + - + - 3.1.3 + 3.1.4 - - - - - - + + + + + + - - - - - - + + + + + + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - - + + - - + + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - - - + + + - - - + + + - 3.1.3 + 3.1.4 - + - - + + - 3.1.3 + 3.1.4 - + - + - 3.1.3 + 3.1.4 - - + + - - + + - 3.1.3 + 3.1.4 - - - + + + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 @@ -462,239 +462,239 @@ - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - - - + + + - 3.1.3 + 3.1.4 - - - + + + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - + - - + + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - - - - - - + + + + + + - - - - - + + + + + - 3.1.3 + 3.1.4 - - + + - - - + + + - 3.1.3 + 3.1.4 - + - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - - - - + + + + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - + - 3.1.3 + 3.1.4 - - + + - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - 3.1.3 + 3.1.4 - - - + + + - 3.1.3 + 3.1.4 - - - + + + - - - + + + - 3.1.3 + 3.1.4 - - - + + + - - - + + + \ No newline at end of file diff --git a/eng/Baseline.xml b/eng/Baseline.xml index df8a80748c..5a5123aa86 100644 --- a/eng/Baseline.xml +++ b/eng/Baseline.xml @@ -4,86 +4,86 @@ This file contains a list of all the packages and their versions which were rele Update this list when preparing for a new patch. --> - + - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/eng/targets/ResolveReferences.targets b/eng/targets/ResolveReferences.targets index 4724ec029f..3439271a92 100644 --- a/eng/targets/ResolveReferences.targets +++ b/eng/targets/ResolveReferences.targets @@ -132,7 +132,9 @@ This target resolves remaining Referene items to Packages, if possible. If not, they are left as Reference items fo the SDK to resolve. This executes on NuGet restore and during DesignTimeBuild. It should not run in the outer, cross-targeting build. --> - + @@ -214,6 +216,53 @@ <_CompileTfmUsingReferenceAssemblies Condition=" '$(CompileUsingReferenceAssemblies)' != false AND '$(TargetFramework)' == '$(DefaultNetCoreTargetFramework)' ">true + + <_ReferenceProjectFile>$(MSBuildProjectDirectory)/../ref/$(MSBuildProjectFile) + + + + $(GetTargetPathWithTargetPlatformMonikerDependsOn);AddReferenceProjectMetadata + + RemoveReferenceProjectMetadata;$(PrepareForRunDependsOn) + + + + + false + + + + + + + + + + + + @(IntermediateAssembly) + + + + + + true + @(ReferenceProjectTargetPathMetadata) + + + + + + + + + diff --git a/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj b/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj index ede671fb99..5216c69c07 100644 --- a/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj +++ b/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj @@ -18,6 +18,7 @@ Microsoft.AspNetCore.Http.HttpResponse + diff --git a/src/Http/Routing/src/Matching/DfaMatcherBuilder.cs b/src/Http/Routing/src/Matching/DfaMatcherBuilder.cs index a259712109..628adf4b74 100644 --- a/src/Http/Routing/src/Matching/DfaMatcherBuilder.cs +++ b/src/Http/Routing/src/Matching/DfaMatcherBuilder.cs @@ -41,6 +41,15 @@ namespace Microsoft.AspNetCore.Routing.Matching _parameterPolicyFactory = parameterPolicyFactory; _selector = selector; + if (AppContext.TryGetSwitch("Microsoft.AspNetCore.Routing.UseCorrectCatchAllBehavior", out var enabled)) + { + UseCorrectCatchAllBehavior = enabled; + } + else + { + UseCorrectCatchAllBehavior = false; // default to bugged behavior + } + var (nodeBuilderPolicies, endpointComparerPolicies, endpointSelectorPolicies) = ExtractPolicies(policies.OrderBy(p => p.Order)); _endpointSelectorPolicies = endpointSelectorPolicies; _nodeBuilders = nodeBuilderPolicies; @@ -56,6 +65,9 @@ namespace Microsoft.AspNetCore.Routing.Matching // Used in tests internal EndpointComparer Comparer => _comparer; + // Used in tests + internal bool UseCorrectCatchAllBehavior { get; set; } + public override void AddEndpoint(RouteEndpoint endpoint) { _endpoints.Add(endpoint); @@ -63,6 +75,15 @@ namespace Microsoft.AspNetCore.Routing.Matching public DfaNode BuildDfaTree(bool includeLabel = false) { + if (!UseCorrectCatchAllBehavior) + { + // In 3.0 we did a global sort of the endpoints up front. This was a bug, because we actually want + // do do the sort at each level of the tree based on precedence. + // + // _useLegacy30Behavior enables opt-out via an AppContext switch. + _endpoints.Sort(_comparer); + } + // Since we're doing a BFS we will process each 'level' of the tree in stages // this list will hold the set of items we need to process at the current // stage. @@ -116,8 +137,13 @@ namespace Microsoft.AspNetCore.Routing.Matching nextWork = previousWork; } - // See comments on precedenceDigitComparer - work.Sort(0, workCount, precedenceDigitComparer); + if (UseCorrectCatchAllBehavior) + { + // The fix for the 3.0 sorting behavior bug. + + // See comments on precedenceDigitComparer + work.Sort(0, workCount, precedenceDigitComparer); + } for (var i = 0; i < workCount; i++) { @@ -460,7 +486,10 @@ namespace Microsoft.AspNetCore.Routing.Matching candidates, endpointSelectorPolicies?.ToArray() ?? Array.Empty(), JumpTableBuilder.Build(currentDefaultDestination, currentExitDestination, pathEntries), - BuildPolicy(currentExitDestination, node.NodeBuilder, policyEntries)); + // Use the final exit destination when building the policy state. + // We don't want to use either of the current destinations because they refer routing states, + // and a policy state should never transition back to a routing state. + BuildPolicy(exitDestination, node.NodeBuilder, policyEntries)); return currentStateIndex; diff --git a/src/Http/Routing/test/UnitTests/GlobalSuppressions.cs b/src/Http/Routing/test/UnitTests/GlobalSuppressions.cs new file mode 100644 index 0000000000..1fbf994418 --- /dev/null +++ b/src/Http/Routing/test/UnitTests/GlobalSuppressions.cs @@ -0,0 +1,11 @@ +// This file is used by Code Analysis to maintain SuppressMessage +// attributes that are applied to this project. +// Project-level suppressions either have no target or are given +// a specific target and scoped to a namespace, type, member, etc. + +[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage( + "Build", + "xUnit1013:Public method 'Quirks_CatchAllParameter' on test class 'FullFeaturedMatcherConformanceTest' should be marked as a Theory.", + Justification = "This is a bug in the xUnit analyzer. This method is already marked as a theory.", + Scope = "member", + Target = "~M:Microsoft.AspNetCore.Routing.Matching.FullFeaturedMatcherConformanceTest.Quirks_CatchAllParameter(System.String,System.String,System.String[],System.String[])~System.Threading.Tasks.Task")] diff --git a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherBuilderTest.cs b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherBuilderTest.cs index 71b7068f00..6ac360a207 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherBuilderTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherBuilderTest.cs @@ -460,12 +460,40 @@ namespace Microsoft.AspNetCore.Routing.Matching } // Regression test for https://github.com/dotnet/aspnetcore/issues/16579 + // + // This case behaves the same for all combinations. [Fact] - public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1() + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_CorrectBehavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = true; + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_Core(builder); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/16579 + // + // This case behaves the same for all combinations. + [Fact] + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_DefaultBehavior() + { + var builder = CreateDfaMatcherBuilder(); + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_Core(builder); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/16579 + // + // This case behaves the same for all combinations. + [Fact] + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_LegacyBehavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = false; + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_Core(builder); + } + + private void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order1_Core(DfaMatcherBuilder builder) { // Arrange - var builder = CreateDfaMatcherBuilder(); - var endpoint1 = CreateEndpoint("a/{b}", order: 0); builder.AddEndpoint(endpoint1); @@ -504,11 +532,31 @@ namespace Microsoft.AspNetCore.Routing.Matching // Regression test for https://github.com/dotnet/aspnetcore/issues/16579 [Fact] - public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2() + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_CorrectBehavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = true; + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_CorrectBehavior_Core(builder); + } + + [Fact] + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_DefaultBehavior() + { + var builder = CreateDfaMatcherBuilder(); + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_LegacyBehavior_Core(builder); + } + + [Fact] + public void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_LegacyBehavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = false; + BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_LegacyBehavior_Core(builder); + } + + private void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_CorrectBehavior_Core(DfaMatcherBuilder builder) { // Arrange - var builder = CreateDfaMatcherBuilder(); - var endpoint1 = CreateEndpoint("a/{*b}", order: 0); builder.AddEndpoint(endpoint1); @@ -545,12 +593,49 @@ namespace Microsoft.AspNetCore.Routing.Matching Assert.Same(catchAll, catchAll.CatchAll); } + private void BuildDfaTree_MultipleEndpoint_ParameterAndCatchAll_OnSameNode_Order2_LegacyBehavior_Core(DfaMatcherBuilder builder) + { + // Arrange + var endpoint1 = CreateEndpoint("a/{*b}", order: 0); + builder.AddEndpoint(endpoint1); + + var endpoint2 = CreateEndpoint("a/{b}", order: 1); + builder.AddEndpoint(endpoint2); + + // Act + var root = builder.BuildDfaTree(); + + // Assert + Assert.Null(root.Matches); + Assert.Null(root.Parameters); + + var next = Assert.Single(root.Literals); + Assert.Equal("a", next.Key); + + var a = next.Value; + Assert.Same(endpoint1, Assert.Single(a.Matches)); + Assert.Null(a.Literals); + + var b = a.Parameters; + Assert.Same(endpoint1, Assert.Single(a.Matches)); + Assert.Null(b.Literals); + Assert.Null(b.Parameters); + Assert.Null(b.CatchAll); + + var catchAll = a.CatchAll; + Assert.Same(endpoint1, Assert.Single(catchAll.Matches)); + Assert.Null(catchAll.Literals); + Assert.Same(catchAll, catchAll.Parameters); + Assert.Same(catchAll, catchAll.CatchAll); + } + // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 [Fact] - public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1() + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_CorrectBehavior() { // Arrange var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = true; var endpoint1 = CreateEndpoint("{a}/{b}", order: 0); builder.AddEndpoint(endpoint1); @@ -601,10 +686,11 @@ namespace Microsoft.AspNetCore.Routing.Matching // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 [Fact] - public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2() + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_CorrectBehavior() { // Arrange var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = true; var endpoint1 = CreateEndpoint("a/{*b}", order: 0); builder.AddEndpoint(endpoint1); @@ -653,6 +739,123 @@ namespace Microsoft.AspNetCore.Routing.Matching Assert.Null(b2.CatchAll); } + // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 + [Fact] + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_DefaultBehavior() + { + var builder = CreateDfaMatcherBuilder(); + BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_Legacy30Behavior_Core(builder); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 + [Fact] + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_Legacy30Behavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = false; + BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_Legacy30Behavior_Core(builder); + } + + private void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order1_Legacy30Behavior_Core(DfaMatcherBuilder builder) + { + // Arrange + var endpoint1 = CreateEndpoint("{a}/{b}", order: 0); + builder.AddEndpoint(endpoint1); + + var endpoint2 = CreateEndpoint("a/{*b}", order: 1); + builder.AddEndpoint(endpoint2); + + // Act + var root = builder.BuildDfaTree(); + + // Assert + Assert.Null(root.Matches); + + var next = Assert.Single(root.Literals); + Assert.Equal("a", next.Key); + + var a1 = next.Value; + Assert.Same(endpoint2, Assert.Single(a1.Matches)); + Assert.Null(a1.Literals); + Assert.Null(a1.Parameters); + + var catchAll1 = a1.CatchAll; + Assert.Same(endpoint2, Assert.Single(catchAll1.Matches)); + Assert.Null(catchAll1.Literals); + Assert.Same(catchAll1, catchAll1.Parameters); + Assert.Same(catchAll1, catchAll1.CatchAll); + + var a2 = root.Parameters; + Assert.Null(a2.Matches); + Assert.Null(a2.Literals); + + var b2 = a2.Parameters; + Assert.Collection( + b2.Matches, + e => Assert.Same(endpoint1, e)); + Assert.Null(b2.Literals); + Assert.Null(b2.Parameters); + Assert.Null(b2.CatchAll); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 + [Fact] + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_DefaultBehavior() + { + var builder = CreateDfaMatcherBuilder(); + BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_Legacy30Behavior_Core(builder); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/18677 + [Fact] + public void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_Legacy30Behavior() + { + var builder = CreateDfaMatcherBuilder(); + builder.UseCorrectCatchAllBehavior = false; + BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_Legacy30Behavior_Core(builder); + } + + private void BuildDfaTree_MultipleEndpoint_CatchAllWithHigherPrecedenceThanParameter_Order2_Legacy30Behavior_Core(DfaMatcherBuilder builder) + { + // Arrange + var endpoint1 = CreateEndpoint("a/{*b}", order: 0); + builder.AddEndpoint(endpoint1); + + var endpoint2 = CreateEndpoint("{a}/{b}", order: 1); + builder.AddEndpoint(endpoint2); + + // Act + var root = builder.BuildDfaTree(); + + // Assert + Assert.Null(root.Matches); + + var next = Assert.Single(root.Literals); + Assert.Equal("a", next.Key); + + var a1 = next.Value; + Assert.Same(endpoint1, Assert.Single(a1.Matches)); + Assert.Null(a1.Literals); + + var b1 = a1.Parameters; + Assert.Same(endpoint2, Assert.Single(b1.Matches)); + Assert.Null(b1.Literals); + Assert.Null(b1.Parameters); + Assert.Null(b1.CatchAll); + + var a2 = root.Parameters; + Assert.Null(a2.Matches); + Assert.Null(a2.Literals); + + var b2 = a2.Parameters; + Assert.Collection( + b2.Matches, + e => Assert.Same(endpoint2, e)); + Assert.Null(b2.Literals); + Assert.Null(b2.Parameters); + Assert.Null(b2.CatchAll); + } + [Fact] public void BuildDfaTree_WithPolicies() { @@ -941,6 +1144,8 @@ namespace Microsoft.AspNetCore.Routing.Matching new TestMetadata2MatcherPolicy(), }; + var comparer = new EndpointComparer(policies.OrderBy(p => p.Order).OfType().ToArray()); + var builder = CreateDfaMatcherBuilder(policies); ((TestMetadata1MatcherPolicy)policies[0]).OnGetEdges = VerifyOrder; @@ -961,7 +1166,7 @@ namespace Microsoft.AspNetCore.Routing.Matching void VerifyOrder(IReadOnlyList endpoints) { // The list should already be in sorted order, every time build is called. - Assert.Equal(endpoints, endpoints.OrderBy(e => e, builder.Comparer)); + Assert.Equal(endpoints, endpoints.OrderBy(e => e, comparer)); } } diff --git a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherConformanceTest.cs b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherConformanceTest.cs index 46ca9fdb2c..66fb02c03a 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherConformanceTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherConformanceTest.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Xunit; namespace Microsoft.AspNetCore.Routing.Matching { @@ -23,7 +24,132 @@ namespace Microsoft.AspNetCore.Routing.Matching MatcherAssert.AssertMatch(httpContext, endpoint, keys, values); } + // https://github.com/dotnet/aspnetcore/issues/18677 + [Theory] + [InlineData("/middleware", 1)] + [InlineData("/middleware/test", 1)] + [InlineData("/middleware/test1/test2", 1)] + [InlineData("/bill/boga", 0)] + public async Task Match_Regression_1867_CorrectBehavior(string path, int endpointIndex) + { + var endpoints = new RouteEndpoint[] + { + EndpointFactory.CreateRouteEndpoint( + "{firstName}/{lastName}", + order: 0, + defaults: new { controller = "TestRoute", action = "Index", }), + + EndpointFactory.CreateRouteEndpoint( + "middleware/{**_}", + order: 0), + }; + + var expected = endpoints[endpointIndex]; + + var matcher = CreateMatcher(useCorrectCatchAllBehavior: true, endpoints); + var httpContext = CreateContext(path); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); + } + + // https://github.com/dotnet/aspnetcore/issues/18677 + // + [Theory] + [InlineData("/middleware", 1)] + [InlineData("/middleware/test", 0)] + [InlineData("/middleware/test1/test2", -1)] + [InlineData("/bill/boga", 0)] + public async Task Match_Regression_1867_DefaultBehavior(string path, int endpointIndex) + { + var endpoints = new RouteEndpoint[] + { + EndpointFactory.CreateRouteEndpoint( + "{firstName}/{lastName}", + order: 0, + defaults: new { controller = "TestRoute", action = "Index", }), + + EndpointFactory.CreateRouteEndpoint( + "middleware/{**_}", + order: 0), + }; + + var expected = endpointIndex switch + { + -1 => null, + _ => endpoints[endpointIndex], + }; + + var matcher = CreateMatcher(useCorrectCatchAllBehavior: default, endpoints); + var httpContext = CreateContext(path); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + if (expected == null) + { + MatcherAssert.AssertNotMatch(httpContext); + } + else + { + MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); + } + } + + // https://github.com/dotnet/aspnetcore/issues/18677 + // + [Theory] + [InlineData("/middleware", 1)] + [InlineData("/middleware/test", 0)] + [InlineData("/middleware/test1/test2", -1)] + [InlineData("/bill/boga", 0)] + public async Task Match_Regression_1867_LegacyBehavior(string path, int endpointIndex) + { + var endpoints = new RouteEndpoint[] + { + EndpointFactory.CreateRouteEndpoint( + "{firstName}/{lastName}", + order: 0, + defaults: new { controller = "TestRoute", action = "Index", }), + + EndpointFactory.CreateRouteEndpoint( + "middleware/{**_}", + order: 0), + }; + + var expected = endpointIndex switch + { + -1 => null, + _ => endpoints[endpointIndex], + }; + + var matcher = CreateMatcher(useCorrectCatchAllBehavior: false, endpoints); + var httpContext = CreateContext(path); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + if (expected == null) + { + MatcherAssert.AssertNotMatch(httpContext); + } + else + { + MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); + } + } + internal override Matcher CreateMatcher(params RouteEndpoint[] endpoints) + { + return CreateMatcher(useCorrectCatchAllBehavior: default, endpoints); + } + + internal Matcher CreateMatcher(bool? useCorrectCatchAllBehavior, params RouteEndpoint[] endpoints) { var services = new ServiceCollection() .AddLogging() @@ -32,6 +158,11 @@ namespace Microsoft.AspNetCore.Routing.Matching .BuildServiceProvider(); var builder = services.GetRequiredService(); + if (useCorrectCatchAllBehavior.HasValue) + { + builder.UseCorrectCatchAllBehavior = useCorrectCatchAllBehavior.Value; + } + for (var i = 0; i < endpoints.Length; i++) { builder.AddEndpoint(endpoints[i]); diff --git a/src/Http/Routing/test/UnitTests/Matching/FullFeaturedMatcherConformanceTest.cs b/src/Http/Routing/test/UnitTests/Matching/FullFeaturedMatcherConformanceTest.cs index 10cdbc7e36..970a4e92ba 100644 --- a/src/Http/Routing/test/UnitTests/Matching/FullFeaturedMatcherConformanceTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/FullFeaturedMatcherConformanceTest.cs @@ -504,37 +504,5 @@ namespace Microsoft.AspNetCore.Routing.Matching // Assert MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); } - - // https://github.com/dotnet/aspnetcore/issues/18677 - [Theory] - [InlineData("/middleware", 1)] - [InlineData("/middleware/test", 1)] - [InlineData("/middleware/test1/test2", 1)] - [InlineData("/bill/boga", 0)] - public virtual async Task Match_Regression_18677(string path, int endpointIndex) - { - var endpoints = new RouteEndpoint[] - { - EndpointFactory.CreateRouteEndpoint( - "{firstName}/{lastName}", - order: 0, - defaults: new { controller = "TestRoute", action = "Index", }), - - EndpointFactory.CreateRouteEndpoint( - "middleware/{**_}", - order: 0), - }; - - var expected = endpoints[endpointIndex]; - - var matcher = CreateMatcher(endpoints); - var httpContext = CreateContext(path); - - // Act - await matcher.MatchAsync(httpContext); - - // Assert - MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); - } } } diff --git a/src/Http/Routing/test/UnitTests/Matching/HostMatcherPolicyIntegrationTestBase.cs b/src/Http/Routing/test/UnitTests/Matching/HostMatcherPolicyIntegrationTestBase.cs index 0e9d6da6c0..580da7ff90 100644 --- a/src/Http/Routing/test/UnitTests/Matching/HostMatcherPolicyIntegrationTestBase.cs +++ b/src/Http/Routing/test/UnitTests/Matching/HostMatcherPolicyIntegrationTestBase.cs @@ -337,6 +337,38 @@ namespace Microsoft.AspNetCore.Routing.Matching MatcherAssert.AssertMatch(httpContext, endpoint); } + [Fact] + public async Task Match_CatchAllRouteWithMatchingHost_Success() + { + // Arrange + var endpoint = CreateEndpoint("/{**path}", hosts: new string[] { "contoso.com", }); + + var matcher = CreateMatcher(endpoint); + var httpContext = CreateContext("/hello", "contoso.com"); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + MatcherAssert.AssertMatch(httpContext, endpoint, new { path = "hello" }); + } + + [Fact] + public async Task Match_CatchAllRouteFailureHost_NoMatch() + { + // Arrange + var endpoint = CreateEndpoint("/{**path}", hosts: new string[] { "contoso.com", }); + + var matcher = CreateMatcher(endpoint); + var httpContext = CreateContext("/hello", "nomatch.com"); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + MatcherAssert.AssertNotMatch(httpContext); + } + private static Matcher CreateMatcher(params RouteEndpoint[] endpoints) { var services = new ServiceCollection() diff --git a/src/Http/Routing/test/UnitTests/Matching/RouteMatcherConformanceTest.cs b/src/Http/Routing/test/UnitTests/Matching/RouteMatcherConformanceTest.cs index 1690696951..fad408f5a1 100644 --- a/src/Http/Routing/test/UnitTests/Matching/RouteMatcherConformanceTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/RouteMatcherConformanceTest.cs @@ -1,16 +1,52 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Threading.Tasks; +using Xunit; + namespace Microsoft.AspNetCore.Routing.Matching { public class RouteMatcherConformanceTest : FullFeaturedMatcherConformanceTest { + // https://github.com/dotnet/aspnetcore/issues/18677 + // + [Theory] + [InlineData("/middleware", 1)] + [InlineData("/middleware/test", 1)] + [InlineData("/middleware/test1/test2", 1)] + [InlineData("/bill/boga", 0)] + public async Task Match_Regression_1867(string path, int endpointIndex) + { + var endpoints = new RouteEndpoint[] + { + EndpointFactory.CreateRouteEndpoint( + "{firstName}/{lastName}", + order: 0, + defaults: new { controller = "TestRoute", action = "Index", }), + + EndpointFactory.CreateRouteEndpoint( + "middleware/{**_}", + order: 0), + }; + + var expected = endpoints[endpointIndex]; + + var matcher = CreateMatcher(endpoints); + var httpContext = CreateContext(path); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); + } + internal override Matcher CreateMatcher(params RouteEndpoint[] endpoints) { var builder = new RouteMatcherBuilder(); for (var i = 0; i < endpoints.Length; i++) { - builder.AddEndpoint(endpoints[i]); + builder.AddEndpoint(endpoints[i]); } return builder.Build(); } diff --git a/src/Http/Routing/test/UnitTests/Matching/TreeRouterMatcherConformanceTest.cs b/src/Http/Routing/test/UnitTests/Matching/TreeRouterMatcherConformanceTest.cs index e2c3a73108..dcf0d7e5ff 100644 --- a/src/Http/Routing/test/UnitTests/Matching/TreeRouterMatcherConformanceTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/TreeRouterMatcherConformanceTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Threading.Tasks; @@ -22,6 +22,39 @@ namespace Microsoft.AspNetCore.Routing.Matching return Task.CompletedTask; } + // https://github.com/dotnet/aspnetcore/issues/18677 + // + [Theory] + [InlineData("/middleware", 1)] + [InlineData("/middleware/test", 1)] + [InlineData("/middleware/test1/test2", 1)] + [InlineData("/bill/boga", 0)] + public async Task Match_Regression_1867(string path, int endpointIndex) + { + var endpoints = new RouteEndpoint[] + { + EndpointFactory.CreateRouteEndpoint( + "{firstName}/{lastName}", + order: 0, + defaults: new { controller = "TestRoute", action = "Index", }), + + EndpointFactory.CreateRouteEndpoint( + "middleware/{**_}", + order: 0), + }; + + var expected = endpoints[endpointIndex]; + + var matcher = CreateMatcher(endpoints); + var httpContext = CreateContext(path); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + MatcherAssert.AssertMatch(httpContext, expected, ignoreValues: true); + } + internal override Matcher CreateMatcher(params RouteEndpoint[] endpoints) { var builder = new TreeRouterMatcherBuilder(); diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index 73dc60e49d..d184f54b21 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -20,10 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { private readonly IArrayPool _charPool; private readonly MvcOptions _mvcOptions; - - // Perf: JsonSerializers are relatively expensive to create, and are thread safe. We cache - // the serializer and invalidate it when the settings change. - private JsonSerializer _serializer; + private JsonSerializerSettings _serializerSettings; /// /// Initializes a new instance. @@ -99,12 +96,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// The used during serialization and deserialization. protected virtual JsonSerializer CreateJsonSerializer() { - if (_serializer == null) + if (_serializerSettings == null) { - _serializer = JsonSerializer.Create(SerializerSettings); + // Lock the serializer settings once the first serialization has been initiated. + _serializerSettings = ShallowCopy(SerializerSettings); } - return _serializer; + return JsonSerializer.Create(_serializerSettings); } /// @@ -166,5 +164,43 @@ namespace Microsoft.AspNetCore.Mvc.Formatters } } } + + private static JsonSerializerSettings ShallowCopy(JsonSerializerSettings settings) + { + var copiedSettings = new JsonSerializerSettings + { + FloatParseHandling = settings.FloatParseHandling, + FloatFormatHandling = settings.FloatFormatHandling, + DateParseHandling = settings.DateParseHandling, + DateTimeZoneHandling = settings.DateTimeZoneHandling, + DateFormatHandling = settings.DateFormatHandling, + Formatting = settings.Formatting, + MaxDepth = settings.MaxDepth, + DateFormatString = settings.DateFormatString, + Context = settings.Context, + Error = settings.Error, + SerializationBinder = settings.SerializationBinder, + TraceWriter = settings.TraceWriter, + Culture = settings.Culture, + ReferenceResolverProvider = settings.ReferenceResolverProvider, + EqualityComparer = settings.EqualityComparer, + ContractResolver = settings.ContractResolver, + ConstructorHandling = settings.ConstructorHandling, + TypeNameAssemblyFormatHandling = settings.TypeNameAssemblyFormatHandling, + MetadataPropertyHandling = settings.MetadataPropertyHandling, + TypeNameHandling = settings.TypeNameHandling, + PreserveReferencesHandling = settings.PreserveReferencesHandling, + Converters = settings.Converters, + DefaultValueHandling = settings.DefaultValueHandling, + NullValueHandling = settings.NullValueHandling, + ObjectCreationHandling = settings.ObjectCreationHandling, + MissingMemberHandling = settings.MissingMemberHandling, + ReferenceLoopHandling = settings.ReferenceLoopHandling, + CheckAdditionalContent = settings.CheckAdditionalContent, + StringEscapeHandling = settings.StringEscapeHandling, + }; + + return copiedSettings; + } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs index d4bfbfa828..e6524e62bf 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs @@ -326,6 +326,40 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Assert.NotNull(outputFormatterContext.HttpContext.Response.ContentLength); } + [Fact] + public async Task SerializingWithPreserveReferenceHandling() + { + // Arrange + var expected = "{\"$id\":\"1\",\"fullName\":\"John\",\"age\":35}"; + var user = new User { FullName = "John", age = 35 }; + + var settings = new JsonSerializerSettings + { + ContractResolver = new DefaultContractResolver + { + NamingStrategy = new CamelCaseNamingStrategy(), + }, + PreserveReferencesHandling = PreserveReferencesHandling.All, + }; + var formatter = new TestableJsonOutputFormatter(settings); + + for (var i = 0; i < 3; i++) + { + // Act + var context = GetOutputFormatterContext(user, typeof(User)); + await formatter.WriteResponseBodyAsync(context, Encoding.UTF8); + + // Assert + var body = context.HttpContext.Response.Body; + + Assert.NotNull(body); + body.Position = 0; + + var content = new StreamReader(body, Encoding.UTF8).ReadToEnd(); + Assert.Equal(expected, content); + } + } + private class TestableJsonOutputFormatter : NewtonsoftJsonOutputFormatter { public TestableJsonOutputFormatter(JsonSerializerSettings serializerSettings)