From dfb579d45c7ba9f095b10505be57178897bf13ba Mon Sep 17 00:00:00 2001 From: Javier Calvarro Nelson Date: Tue, 21 Aug 2018 17:01:20 -0700 Subject: [PATCH] [Fixes #8021] Copy the request headers before sending the request on the RedirectHandler If another handler modifies the request headers the modified headers get applied on subsequent requests, which is not correct. This change copies the headers before sending the request and uses the original headers for the redirect request instead of the potentially modified ones. --- .../Handlers/RedirectHandler.cs | 42 ++++++++++------- .../TestingInfrastructureTests.cs | 47 ++++++++++++++++++- .../Controllers/TestingController.cs | 26 ++++++++++ 3 files changed, 97 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Testing/Handlers/RedirectHandler.cs b/src/Microsoft.AspNetCore.Mvc.Testing/Handlers/RedirectHandler.cs index 63158713d3..71198d3d75 100644 --- a/src/Microsoft.AspNetCore.Mvc.Testing/Handlers/RedirectHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Testing/Handlers/RedirectHandler.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Threading; using System.Threading.Tasks; @@ -49,13 +50,14 @@ namespace Microsoft.AspNetCore.Mvc.Testing.Handlers protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { var remainingRedirects = MaxRedirects; - + var redirectRequest = new HttpRequestMessage(); var originalRequestContent = HasBody(request) ? await DuplicateRequestContent(request) : null; + CopyRequestHeaders(request.Headers, redirectRequest.Headers); var response = await base.SendAsync(request, cancellationToken); while (IsRedirect(response) && remainingRedirects >= 0) { remainingRedirects--; - var redirectRequest = GetRedirectRequest(response, originalRequestContent); + UpdateRedirectRequest(response, redirectRequest, originalRequestContent); originalRequestContent = HasBody(redirectRequest) ? await DuplicateRequestContent(redirectRequest) : null; response = await base.SendAsync(redirectRequest, cancellationToken); } @@ -95,6 +97,16 @@ namespace Microsoft.AspNetCore.Mvc.Testing.Handlers } } + private static void CopyRequestHeaders( + HttpRequestHeaders originalRequestHeaders, + HttpRequestHeaders newRequestHeaders) + { + foreach (var header in originalRequestHeaders) + { + newRequestHeaders.Add(header.Key, header.Value); + } + } + private static async Task<(Stream originalBody, Stream copy)> CopyBody(HttpRequestMessage request) { var originalBody = await request.Content.ReadAsStreamAsync(); @@ -116,8 +128,9 @@ namespace Microsoft.AspNetCore.Mvc.Testing.Handlers return (originalBody, bodyCopy); } - private static HttpRequestMessage GetRedirectRequest( + private static void UpdateRedirectRequest( HttpResponseMessage response, + HttpRequestMessage redirect, HttpContent originalContent) { var location = response.Headers.Location; @@ -128,34 +141,31 @@ namespace Microsoft.AspNetCore.Mvc.Testing.Handlers location); } - var redirect = !ShouldKeepVerb(response) ? - new HttpRequestMessage(HttpMethod.Get, location) : - new HttpRequestMessage(response.RequestMessage.Method, location) - { - Content = originalContent - }; - - foreach (var header in response.RequestMessage.Headers) + redirect.RequestUri = location; + if (!ShouldKeepVerb(response)) { - redirect.Headers.Add(header.Key, header.Value); + redirect.Method = HttpMethod.Get; + } + else + { + redirect.Method = response.RequestMessage.Method; + redirect.Content = originalContent; } foreach (var property in response.RequestMessage.Properties) { redirect.Properties.Add(property.Key, property.Value); } - - return redirect; } private static bool ShouldKeepVerb(HttpResponseMessage response) => response.StatusCode == HttpStatusCode.RedirectKeepVerb || (int)response.StatusCode == 308; - private bool IsRedirect(HttpResponseMessage response) => + private bool IsRedirect(HttpResponseMessage response) => response.StatusCode == HttpStatusCode.MovedPermanently || response.StatusCode == HttpStatusCode.Redirect || response.StatusCode == HttpStatusCode.RedirectKeepVerb || (int)response.StatusCode == 308; } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TestingInfrastructureTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TestingInfrastructureTests.cs index 9e5be478f6..afa57b5092 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TestingInfrastructureTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TestingInfrastructureTests.cs @@ -2,11 +2,13 @@ using System.Net; using System.Net.Http; using System.Net.Http.Formatting; +using System.Threading; using System.Threading.Tasks; using BasicWebSite; using BasicWebSite.Controllers; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.Mvc.Testing.Handlers; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -17,13 +19,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests { public TestingInfrastructureTests(WebApplicationFactory fixture) { - var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); - Client = factory.CreateClient(); + Factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + Client = Factory.CreateClient(); } private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => builder.ConfigureTestServices(s => s.AddSingleton()); + public WebApplicationFactory Factory { get; } public HttpClient Client { get; } [Fact] @@ -57,6 +60,22 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(5, handlerResponse.Body); } + [Fact] + public async Task TestingInfrastructure_RedirectHandlerUsesOriginalRequestHeaders() + { + // Act + var request = new HttpRequestMessage(HttpMethod.Get, "Testing/RedirectHandler/Headers"); + var client = Factory.CreateDefaultClient( + new RedirectHandler(), new TestHandler()); + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var modifiedHeaderWasSent = await response.Content.ReadAsStringAsync(); + + Assert.Equal("false", modifiedHeaderWasSent); + } + [Fact] public async Task TestingInfrastructure_PostRedirectGetWorksWithCookies() { @@ -93,5 +112,29 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Message = "Test"; } } + + private class TestHandler : DelegatingHandler + { + public TestHandler() + { + } + + public TestHandler(HttpMessageHandler innerHandler) : base(innerHandler) + { + } + + public bool HeaderAdded { get; set; } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (!HeaderAdded) + { + request.Headers.Add("X-Added-Header", "true"); + HeaderAdded = true; + } + + return base.SendAsync(request, cancellationToken); + } + } } } diff --git a/test/WebSites/BasicWebSite/Controllers/TestingController.cs b/test/WebSites/BasicWebSite/Controllers/TestingController.cs index 07e76a3d6b..882ed935c9 100644 --- a/test/WebSites/BasicWebSite/Controllers/TestingController.cs +++ b/test/WebSites/BasicWebSite/Controllers/TestingController.cs @@ -37,6 +37,32 @@ namespace BasicWebSite.Controllers return Ok(new RedirectHandlerResponse { Url = value, Body = number.Value }); } + [HttpGet("Testing/RedirectHandler/Headers")] + public IActionResult RedirectHandlerHeaders() + { + if (!Request.Headers.TryGetValue("X-Added-Header", out var value)) + { + return Content("No header present"); + } + else + { + return RedirectToAction(nameof(RedirectHandlerHeadersRedirect)); + } + } + + [HttpGet("Testing/RedirectHandler/Headers/Redirect")] + public IActionResult RedirectHandlerHeadersRedirect() + { + if (Request.Headers.TryGetValue("X-Added-Header", out var value)) + { + return Content("true"); + } + else + { + return Content("false"); + } + } + [HttpGet("Testing/AntiforgerySimulator/{value}")] public IActionResult AntiforgerySimulator([FromRoute]int value) {