From 079ec0f365d6076dda14671ef32536df9d27b358 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Sat, 21 Jul 2018 00:10:06 +0100 Subject: [PATCH] #474 tests to show how spaces in headers work (#483) --- .../Middleware/ExceptionHandlerMiddleware.cs | 208 +++++++++--------- test/Ocelot.AcceptanceTests/HeaderTests.cs | 72 ++++++ test/Ocelot.AcceptanceTests/Steps.cs | 2 +- 3 files changed, 177 insertions(+), 105 deletions(-) diff --git a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs index f1353346..feb64af0 100644 --- a/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs +++ b/src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs @@ -1,104 +1,104 @@ -using System; -using System.Linq; -using System.Threading.Tasks; -using Ocelot.Configuration.Repository; -using Ocelot.Infrastructure.Extensions; -using Ocelot.Infrastructure.RequestData; -using Ocelot.Logging; -using Ocelot.Middleware; - -namespace Ocelot.Errors.Middleware -{ - using Configuration; - - /// - /// Catches all unhandled exceptions thrown by middleware, logs and returns a 500 - /// - public class ExceptionHandlerMiddleware : OcelotMiddleware - { - private readonly OcelotRequestDelegate _next; - private readonly IInternalConfigurationRepository _configRepo; - private readonly IRequestScopedDataRepository _repo; - - public ExceptionHandlerMiddleware(OcelotRequestDelegate next, - IOcelotLoggerFactory loggerFactory, - IInternalConfigurationRepository configRepo, - IRequestScopedDataRepository repo) - : base(loggerFactory.CreateLogger()) - { - _configRepo = configRepo; - _repo = repo; - _next = next; - } - - public async Task Invoke(DownstreamContext context) - { - try - { - //try and get the global request id and set it for logs... - //should this basically be immutable per request...i guess it should! - //first thing is get config - var configuration = _configRepo.Get(); - - if (configuration.IsError) - { - throw new Exception($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); - } - - TrySetGlobalRequestId(context, configuration.Data); - - context.Configuration = configuration.Data; - - Logger.LogDebug("ocelot pipeline started"); - - await _next.Invoke(context); - } - catch (Exception e) - { - Logger.LogDebug("error calling middleware"); - - var message = CreateMessage(context, e); - - Logger.LogError(message, e); - - SetInternalServerErrorOnResponse(context); - } - - Logger.LogDebug("ocelot pipeline finished"); - } - - private void TrySetGlobalRequestId(DownstreamContext context, IInternalConfiguration configuration) - { - var key = configuration.RequestId; - - if (!string.IsNullOrEmpty(key) && context.HttpContext.Request.Headers.TryGetValue(key, out var upstreamRequestIds)) - { - context.HttpContext.TraceIdentifier = upstreamRequestIds.First(); - } - - _repo.Add("RequestId", context.HttpContext.TraceIdentifier); - } - - private void SetInternalServerErrorOnResponse(DownstreamContext context) - { - if (!context.HttpContext.Response.HasStarted) - { - context.HttpContext.Response.StatusCode = 500; - } - } - - private string CreateMessage(DownstreamContext context, Exception e) - { - var message = - $"Exception caught in global error handler, exception message: {e.Message}, exception stack: {e.StackTrace}"; - - if (e.InnerException != null) - { - message = - $"{message}, inner exception message {e.InnerException.Message}, inner exception stack {e.InnerException.StackTrace}"; - } - - return $"{message} RequestId: {context.HttpContext.TraceIdentifier}"; - } - } -} +using System; +using System.Linq; +using System.Threading.Tasks; +using Ocelot.Configuration.Repository; +using Ocelot.Infrastructure.Extensions; +using Ocelot.Infrastructure.RequestData; +using Ocelot.Logging; +using Ocelot.Middleware; + +namespace Ocelot.Errors.Middleware +{ + using Configuration; + + /// + /// Catches all unhandled exceptions thrown by middleware, logs and returns a 500 + /// + public class ExceptionHandlerMiddleware : OcelotMiddleware + { + private readonly OcelotRequestDelegate _next; + private readonly IInternalConfigurationRepository _configRepo; + private readonly IRequestScopedDataRepository _repo; + + public ExceptionHandlerMiddleware(OcelotRequestDelegate next, + IOcelotLoggerFactory loggerFactory, + IInternalConfigurationRepository configRepo, + IRequestScopedDataRepository repo) + : base(loggerFactory.CreateLogger()) + { + _configRepo = configRepo; + _repo = repo; + _next = next; + } + + public async Task Invoke(DownstreamContext context) + { + try + { + //try and get the global request id and set it for logs... + //should this basically be immutable per request...i guess it should! + //first thing is get config + var configuration = _configRepo.Get(); + + if (configuration.IsError) + { + throw new Exception($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); + } + + TrySetGlobalRequestId(context, configuration.Data); + + context.Configuration = configuration.Data; + + Logger.LogDebug("ocelot pipeline started"); + + await _next.Invoke(context); + } + catch (Exception e) + { + Logger.LogDebug("error calling middleware"); + + var message = CreateMessage(context, e); + + Logger.LogError(message, e); + + SetInternalServerErrorOnResponse(context); + } + + Logger.LogDebug("ocelot pipeline finished"); + } + + private void TrySetGlobalRequestId(DownstreamContext context, IInternalConfiguration configuration) + { + var key = configuration.RequestId; + + if (!string.IsNullOrEmpty(key) && context.HttpContext.Request.Headers.TryGetValue(key, out var upstreamRequestIds)) + { + context.HttpContext.TraceIdentifier = upstreamRequestIds.First(); + } + + _repo.Add("RequestId", context.HttpContext.TraceIdentifier); + } + + private void SetInternalServerErrorOnResponse(DownstreamContext context) + { + if (!context.HttpContext.Response.HasStarted) + { + context.HttpContext.Response.StatusCode = 500; + } + } + + private string CreateMessage(DownstreamContext context, Exception e) + { + var message = + $"Exception caught in global error handler, exception message: {e.Message}, exception stack: {e.StackTrace}"; + + if (e.InnerException != null) + { + message = + $"{message}, inner exception message {e.InnerException.Message}, inner exception stack {e.InnerException.StackTrace}"; + } + + return $"{message} RequestId: {context.HttpContext.TraceIdentifier}"; + } + } +} diff --git a/test/Ocelot.AcceptanceTests/HeaderTests.cs b/test/Ocelot.AcceptanceTests/HeaderTests.cs index fd8b2767..52ad0a06 100644 --- a/test/Ocelot.AcceptanceTests/HeaderTests.cs +++ b/test/Ocelot.AcceptanceTests/HeaderTests.cs @@ -313,6 +313,78 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + [Fact] + public void issue_474_should_not_put_spaces_in_header() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "http", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = 51879, + } + }, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51879", "/", 200, "Accept")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .And(x => _steps.GivenIAddAHeader("Accept", "text/html,application/xhtml+xml,application/xml;")) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("text/html,application/xhtml+xml,application/xml;")) + .BDDfy(); + } + + [Fact] + public void issue_474_should_put_spaces_in_header() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "http", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = 51879, + } + }, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51879", "/", 200, "Accept")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .And(x => _steps.GivenIAddAHeader("Accept", "text/html")) + .And(x => _steps.GivenIAddAHeader("Accept", "application/xhtml+xml")) + .And(x => _steps.GivenIAddAHeader("Accept", "application/xml")) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("text/html, application/xhtml+xml, application/xml")) + .BDDfy(); + } + private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode) { _builder = new WebHostBuilder() diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 66fcef2e..92f463f1 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -746,7 +746,7 @@ namespace Ocelot.AcceptanceTests public void GivenIAddAHeader(string key, string value) { - _ocelotClient.DefaultRequestHeaders.Add(key, value); + _ocelotClient.DefaultRequestHeaders.TryAddWithoutValidation(key, value); } public void WhenIGetUrlOnTheApiGatewayMultipleTimes(string url, int times)