From ca7c48c520f2baae032e0429f19552abed77e009 Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Mon, 7 Jan 2019 00:42:36 +0000 Subject: [PATCH] Fix ArgumentException from duplicate key (#6416) --- .../src/Matching/HttpMethodMatcherPolicy.cs | 11 ++++--- .../HttpMethodMatcherPolicyIntegrationTest.cs | 32 ++++++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs b/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs index f1ec26d67d..af3981521a 100644 --- a/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Http/Routing/src/Matching/HttpMethodMatcherPolicy.cs @@ -1,13 +1,10 @@ -// 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; using System.Collections.Generic; -using System.IO; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Primitives; @@ -274,7 +271,11 @@ namespace Microsoft.AspNetCore.Routing.Matching (context) => { context.Response.StatusCode = 405; - context.Response.Headers.Add("Allow", allow); + + // Prevent ArgumentException from duplicate key if header already added, such as when the + // request is re-executed by an error handler (see https://github.com/aspnet/AspNetCore/issues/6415) + context.Response.Headers["Allow"] = allow; + return Task.CompletedTask; }, EndpointMetadataCollection.Empty, diff --git a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs index 1aca0d5a64..fd223d9b7e 100644 --- a/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/HttpMethodMatcherPolicyIntegrationTest.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; @@ -288,6 +288,36 @@ namespace Microsoft.AspNetCore.Routing.Matching MatcherAssert.AssertMatch(context, httpContext, endpoint2, ignoreValues: true); } + [Fact] // See https://github.com/aspnet/AspNetCore/issues/6415 + public async Task NotMatch_HttpMethod_Returns405Endpoint_ReExecute() + { + // Arrange + var endpoint1 = CreateEndpoint("/hello", httpMethods: new string[] { "GET", "PUT" }); + var endpoint2 = CreateEndpoint("/hello", httpMethods: new string[] { "DELETE" }); + + var matcher = CreateMatcher(endpoint1, endpoint2); + var (httpContext, context) = CreateContext("/hello", "POST"); + + // Act + await matcher.MatchAsync(httpContext, context); + + // Assert + Assert.NotSame(endpoint1, context.Endpoint); + Assert.NotSame(endpoint2, context.Endpoint); + + Assert.Same(HttpMethodMatcherPolicy.Http405EndpointDisplayName, context.Endpoint.DisplayName); + + // Invoke the endpoint + await context.Endpoint.RequestDelegate(httpContext); + Assert.Equal(405, httpContext.Response.StatusCode); + Assert.Equal("DELETE, GET, PUT", httpContext.Response.Headers["Allow"]); + + // Invoke the endpoint again to verify headers not duplicated + await context.Endpoint.RequestDelegate(httpContext); + Assert.Equal(405, httpContext.Response.StatusCode); + Assert.Equal("DELETE, GET, PUT", httpContext.Response.Headers["Allow"]); + } + private static Matcher CreateMatcher(params RouteEndpoint[] endpoints) { var services = new ServiceCollection()