diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 40ac80ed..7132c0b2 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -48,6 +48,7 @@ namespace Ocelot.DependencyInjection using ServiceDiscovery.Providers; using Steeltoe.Common.Discovery; using Pivotal.Discovery.Client; + using Ocelot.Request.Creator; public class OcelotBuilder : IOcelotBuilder { @@ -161,6 +162,8 @@ namespace Ocelot.DependencyInjection _services.TryAddSingleton(); _services.TryAddSingleton(); _services.TryAddSingleton(); + _services.TryAddSingleton(); + _services.TryAddSingleton(); } public IOcelotAdministrationBuilder AddAdministration(string path, string secret) diff --git a/src/Ocelot/Infrastructure/FrameworkDescription.cs b/src/Ocelot/Infrastructure/FrameworkDescription.cs new file mode 100644 index 00000000..927d4059 --- /dev/null +++ b/src/Ocelot/Infrastructure/FrameworkDescription.cs @@ -0,0 +1,12 @@ +using System.Runtime.InteropServices; + +namespace Ocelot.Infrastructure +{ + public class FrameworkDescription : IFrameworkDescription + { + public string Get() + { + return RuntimeInformation.FrameworkDescription; + } + } +} diff --git a/src/Ocelot/Infrastructure/IFrameworkDescription.cs b/src/Ocelot/Infrastructure/IFrameworkDescription.cs new file mode 100644 index 00000000..e59ac794 --- /dev/null +++ b/src/Ocelot/Infrastructure/IFrameworkDescription.cs @@ -0,0 +1,7 @@ +namespace Ocelot.Infrastructure +{ + public interface IFrameworkDescription + { + string Get(); + } +} diff --git a/src/Ocelot/Request/Creator/DownstreamRequestCreator.cs b/src/Ocelot/Request/Creator/DownstreamRequestCreator.cs new file mode 100644 index 00000000..e4bdb1fc --- /dev/null +++ b/src/Ocelot/Request/Creator/DownstreamRequestCreator.cs @@ -0,0 +1,42 @@ +namespace Ocelot.Request.Creator +{ + using System.Net.Http; + using Ocelot.Request.Middleware; + using System.Runtime.InteropServices; + using Ocelot.Infrastructure; + + public class DownstreamRequestCreator : IDownstreamRequestCreator + { + private readonly IFrameworkDescription _framework; + private const string dotNetFramework = ".NET Framework"; + + public DownstreamRequestCreator(IFrameworkDescription framework) + { + _framework = framework; + } + + public DownstreamRequest Create(HttpRequestMessage request) + { + /** + * According to https://tools.ietf.org/html/rfc7231 + * GET,HEAD,DELETE,CONNECT,TRACE + * Can have body but server can reject the request. + * And MS HttpClient in Full Framework actually rejects it. + * see #366 issue + **/ + + if(_framework.Get().Contains(dotNetFramework)) + { + if (request.Method == HttpMethod.Get || + request.Method == HttpMethod.Head || + request.Method == HttpMethod.Delete || + request.Method == HttpMethod.Trace) + { + request.Content = null; + } + } + + return new DownstreamRequest(request); + } + } +} diff --git a/src/Ocelot/Request/Creator/IDownstreamRequestCreator.cs b/src/Ocelot/Request/Creator/IDownstreamRequestCreator.cs new file mode 100644 index 00000000..e6755399 --- /dev/null +++ b/src/Ocelot/Request/Creator/IDownstreamRequestCreator.cs @@ -0,0 +1,10 @@ +namespace Ocelot.Request.Creator +{ + using System.Net.Http; + using Ocelot.Request.Middleware; + + public interface IDownstreamRequestCreator + { + DownstreamRequest Create(HttpRequestMessage request); + } +} diff --git a/src/Ocelot/Request/Middleware/DownstreamRequest.cs b/src/Ocelot/Request/Middleware/DownstreamRequest.cs index fc1feb8a..75070bfd 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequest.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequest.cs @@ -48,22 +48,6 @@ namespace Ocelot.Request.Middleware Scheme = Scheme }; - /** - * According to https://tools.ietf.org/html/rfc7231 - * GET,HEAD,DELETE,CONNECT,TRACE - * Can have body but server can reject the request. - * And MS HttpClient in Full Framework actually rejects it. - * see #366 issue - **/ -#if NET461 || NET462 || NET47 || NET471 || NET472 - if (_request.Method == HttpMethod.Get || - _request.Method == HttpMethod.Head || - _request.Method == HttpMethod.Delete || - _request.Method == HttpMethod.Trace) - { - _request.Content = null; - } -#endif _request.RequestUri = uriBuilder.Uri; return _request; } diff --git a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs index 0cb7c7dd..ce226d98 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs @@ -1,39 +1,42 @@ -namespace Ocelot.Request.Middleware -{ - using System.Net.Http; - using System.Threading.Tasks; - using Microsoft.AspNetCore.Http; - using Ocelot.DownstreamRouteFinder.Middleware; - using Ocelot.Infrastructure.RequestData; - using Ocelot.Logging; - using Ocelot.Middleware; - - public class DownstreamRequestInitialiserMiddleware : OcelotMiddleware - { - private readonly OcelotRequestDelegate _next; - private readonly Mapper.IRequestMapper _requestMapper; - - public DownstreamRequestInitialiserMiddleware(OcelotRequestDelegate next, - IOcelotLoggerFactory loggerFactory, - Mapper.IRequestMapper requestMapper) - :base(loggerFactory.CreateLogger()) - { - _next = next; - _requestMapper = requestMapper; - } - - public async Task Invoke(DownstreamContext context) - { - var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request); - if (downstreamRequest.IsError) - { - SetPipelineError(context, downstreamRequest.Errors); - return; - } - - context.DownstreamRequest = new DownstreamRequest(downstreamRequest.Data); - - await _next.Invoke(context); - } - } -} +namespace Ocelot.Request.Middleware +{ + using System.Threading.Tasks; + using Microsoft.AspNetCore.Http; + using Ocelot.DownstreamRouteFinder.Middleware; + using Ocelot.Infrastructure.RequestData; + using Ocelot.Logging; + using Ocelot.Middleware; + using Ocelot.Request.Creator; + + public class DownstreamRequestInitialiserMiddleware : OcelotMiddleware + { + private readonly OcelotRequestDelegate _next; + private readonly Mapper.IRequestMapper _requestMapper; + private readonly IDownstreamRequestCreator _creator; + + public DownstreamRequestInitialiserMiddleware(OcelotRequestDelegate next, + IOcelotLoggerFactory loggerFactory, + Mapper.IRequestMapper requestMapper, + IDownstreamRequestCreator creator) + :base(loggerFactory.CreateLogger()) + { + _next = next; + _requestMapper = requestMapper; + _creator = creator; + } + + public async Task Invoke(DownstreamContext context) + { + var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request); + if (downstreamRequest.IsError) + { + SetPipelineError(context, downstreamRequest.Errors); + return; + } + + context.DownstreamRequest = _creator.Create(downstreamRequest.Data); + + await _next.Invoke(context); + } + } +} diff --git a/test/Ocelot.ManualTest/ocelot.json b/test/Ocelot.ManualTest/ocelot.json index c21f8abb..6f531414 100644 --- a/test/Ocelot.ManualTest/ocelot.json +++ b/test/Ocelot.ManualTest/ocelot.json @@ -114,7 +114,8 @@ "HttpHandlerOptions": { "AllowAutoRedirect": true, "UseCookieContainer": true, - "UseTracing": true + "UseTracing": true, + "UseProxy": true }, "QoSOptions": { "ExceptionsAllowedBeforeBreaking": 3, diff --git a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj index bfef63e0..a1908e7a 100644 --- a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj +++ b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj @@ -28,6 +28,15 @@ + + + PreserveNewest + + + PreserveNewest + + + diff --git a/test/Ocelot.UnitTests/Request/Creator/DownstreamRequestCreatorTests.cs b/test/Ocelot.UnitTests/Request/Creator/DownstreamRequestCreatorTests.cs new file mode 100644 index 00000000..f30f4cd8 --- /dev/null +++ b/test/Ocelot.UnitTests/Request/Creator/DownstreamRequestCreatorTests.cs @@ -0,0 +1,93 @@ +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; +using System.Threading.Tasks; +using Moq; +using Ocelot.Infrastructure; +using Ocelot.Request.Creator; +using Ocelot.Request.Middleware; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Request.Creator +{ + public class DownstreamRequestCreatorTests + { + private Mock _framework; + private DownstreamRequestCreator _downstreamRequestCreator; + private HttpRequestMessage _request; + private DownstreamRequest _result; + + public DownstreamRequestCreatorTests() + { + _framework = new Mock(); + _downstreamRequestCreator = new DownstreamRequestCreator(_framework.Object); + } + + [Fact] + public void should_create_downstream_request() + { + var request = new HttpRequestMessage(HttpMethod.Get, "http://www.test.com"); + var content = new StringContent("test"); + request.Content = content; + + this.Given(_ => GivenTheFrameworkIs("")) + .And(_ => GivenTheRequestIs(request)) + .When(_ => WhenICreate()) + .Then(_ => ThenTheDownstreamRequestHasABody()) + .BDDfy(); + } + + [Fact] + public void should_remove_body_for_http_methods() + { + var methods = new List { HttpMethod.Get, HttpMethod.Head, HttpMethod.Delete, HttpMethod.Trace }; + var request = new HttpRequestMessage(HttpMethod.Get, "http://www.test.com"); + var content = new StringContent("test"); + request.Content = content; + + methods.ForEach(m => { + this.Given(_ => GivenTheFrameworkIs(".NET Framework")) + .And(_ => GivenTheRequestIs(request)) + .When(_ => WhenICreate()) + .Then(_ => ThenTheDownstreamRequestDoesNotHaveABody()) + .BDDfy(); + }); + } + + private void GivenTheFrameworkIs(string framework) + { + _framework.Setup(x => x.Get()).Returns(framework); + } + + private void GivenTheRequestIs(HttpRequestMessage request) + { + _request = request; + } + + private void WhenICreate() + { + _result = _downstreamRequestCreator.Create(_request); + } + + private async Task ThenTheDownstreamRequestHasABody() + { + _result.ShouldNotBeNull(); + _result.Method.ToLower().ShouldBe("get"); + _result.Scheme.ToLower().ShouldBe("http"); + _result.Host.ToLower().ShouldBe("www.test.com"); + var resultContent = await _result.ToHttpRequestMessage().Content.ReadAsStringAsync(); + resultContent.ShouldBe("test"); + } + + private void ThenTheDownstreamRequestDoesNotHaveABody() + { + _result.ShouldNotBeNull(); + _result.Method.ToLower().ShouldBe("get"); + _result.Scheme.ToLower().ShouldBe("http"); + _result.Host.ToLower().ShouldBe("www.test.com"); + _result.ToHttpRequestMessage().Content.ShouldBeNull(); + } + } +} diff --git a/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs b/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs index d37efc42..d571a8b7 100644 --- a/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs @@ -1,143 +1,146 @@ -using Ocelot.Middleware; - -namespace Ocelot.UnitTests.Request -{ - using System.Net.Http; - using Microsoft.AspNetCore.Http; - using Moq; - using Ocelot.Logging; - using Ocelot.Request.Mapper; - using Ocelot.Request.Middleware; - using Ocelot.Infrastructure.RequestData; - using TestStack.BDDfy; - using Xunit; - using Ocelot.Responses; - using Ocelot.DownstreamRouteFinder.Middleware; - using Shouldly; - - public class DownstreamRequestInitialiserMiddlewareTests - { - readonly DownstreamRequestInitialiserMiddleware _middleware; - - readonly Mock _httpContext; - - readonly Mock _httpRequest; - - readonly Mock _next; - - readonly Mock _requestMapper; - - readonly Mock _loggerFactory; - - readonly Mock _logger; - - Response _mappedRequest; - private DownstreamContext _downstreamContext; - - public DownstreamRequestInitialiserMiddlewareTests() - { - _httpContext = new Mock(); - _httpRequest = new Mock(); - _requestMapper = new Mock(); - _next = new Mock(); - _logger = new Mock(); - - _loggerFactory = new Mock(); - _loggerFactory - .Setup(lf => lf.CreateLogger()) - .Returns(_logger.Object); - - _middleware = new DownstreamRequestInitialiserMiddleware( - _next.Object, - _loggerFactory.Object, - _requestMapper.Object); - - _downstreamContext = new DownstreamContext(_httpContext.Object); - } - - [Fact] - public void Should_handle_valid_httpRequest() - { - this.Given(_ => GivenTheHttpContextContainsARequest()) - .And(_ => GivenTheMapperWillReturnAMappedRequest()) - .When(_ => WhenTheMiddlewareIsInvoked()) - .Then(_ => ThenTheContexRequestIsMappedToADownstreamRequest()) - .And(_ => ThenTheDownstreamRequestIsStored()) - .And(_ => ThenTheNextMiddlewareIsInvoked()) - .BDDfy(); - } - - [Fact] - public void Should_handle_mapping_failure() - { - this.Given(_ => GivenTheHttpContextContainsARequest()) - .And(_ => GivenTheMapperWillReturnAnError()) - .When(_ => WhenTheMiddlewareIsInvoked()) - .And(_ => ThenTheDownstreamRequestIsNotStored()) - .And(_ => ThenAPipelineErrorIsStored()) - .And(_ => ThenTheNextMiddlewareIsNotInvoked()) - .BDDfy(); - } - - private void GivenTheHttpContextContainsARequest() - { - _httpContext - .Setup(hc => hc.Request) - .Returns(_httpRequest.Object); - } - - private void GivenTheMapperWillReturnAMappedRequest() - { - _mappedRequest = new OkResponse(new HttpRequestMessage(HttpMethod.Get, "http://www.bbc.co.uk")); - - _requestMapper - .Setup(rm => rm.Map(It.IsAny())) - .ReturnsAsync(_mappedRequest); - } - - private void GivenTheMapperWillReturnAnError() - { - _mappedRequest = new ErrorResponse(new UnmappableRequestError(new System.Exception("boooom!"))); - - _requestMapper - .Setup(rm => rm.Map(It.IsAny())) - .ReturnsAsync(_mappedRequest); - } - - private void WhenTheMiddlewareIsInvoked() - { - _middleware.Invoke(_downstreamContext).GetAwaiter().GetResult(); - } - - private void ThenTheContexRequestIsMappedToADownstreamRequest() - { - _requestMapper.Verify(rm => rm.Map(_httpRequest.Object), Times.Once); - } - - private void ThenTheDownstreamRequestIsStored() - { - _downstreamContext.DownstreamRequest.ShouldNotBeNull(); - } - - private void ThenTheDownstreamRequestIsNotStored() - { - _downstreamContext.DownstreamRequest.ShouldBeNull(); - } - - private void ThenAPipelineErrorIsStored() - { - _downstreamContext.IsError.ShouldBeTrue(); - _downstreamContext.Errors.ShouldBe(_mappedRequest.Errors); - } - - private void ThenTheNextMiddlewareIsInvoked() - { - _next.Verify(n => n(_downstreamContext), Times.Once); - } - - private void ThenTheNextMiddlewareIsNotInvoked() - { - _next.Verify(n => n(It.IsAny()), Times.Never); - } - } -} +using Ocelot.Middleware; + +namespace Ocelot.UnitTests.Request +{ + using System.Net.Http; + using Microsoft.AspNetCore.Http; + using Moq; + using Ocelot.Logging; + using Ocelot.Request.Mapper; + using Ocelot.Request.Middleware; + using Ocelot.Infrastructure.RequestData; + using TestStack.BDDfy; + using Xunit; + using Ocelot.Responses; + using Ocelot.DownstreamRouteFinder.Middleware; + using Shouldly; + using Ocelot.Request.Creator; + using Ocelot.Infrastructure; + + public class DownstreamRequestInitialiserMiddlewareTests + { + readonly DownstreamRequestInitialiserMiddleware _middleware; + + readonly Mock _httpContext; + + readonly Mock _httpRequest; + + readonly Mock _next; + + readonly Mock _requestMapper; + + readonly Mock _loggerFactory; + + readonly Mock _logger; + + Response _mappedRequest; + private DownstreamContext _downstreamContext; + + public DownstreamRequestInitialiserMiddlewareTests() + { + _httpContext = new Mock(); + _httpRequest = new Mock(); + _requestMapper = new Mock(); + _next = new Mock(); + _logger = new Mock(); + + _loggerFactory = new Mock(); + _loggerFactory + .Setup(lf => lf.CreateLogger()) + .Returns(_logger.Object); + + _middleware = new DownstreamRequestInitialiserMiddleware( + _next.Object, + _loggerFactory.Object, + _requestMapper.Object, + new DownstreamRequestCreator(new FrameworkDescription())); + + _downstreamContext = new DownstreamContext(_httpContext.Object); + } + + [Fact] + public void Should_handle_valid_httpRequest() + { + this.Given(_ => GivenTheHttpContextContainsARequest()) + .And(_ => GivenTheMapperWillReturnAMappedRequest()) + .When(_ => WhenTheMiddlewareIsInvoked()) + .Then(_ => ThenTheContexRequestIsMappedToADownstreamRequest()) + .And(_ => ThenTheDownstreamRequestIsStored()) + .And(_ => ThenTheNextMiddlewareIsInvoked()) + .BDDfy(); + } + + [Fact] + public void Should_handle_mapping_failure() + { + this.Given(_ => GivenTheHttpContextContainsARequest()) + .And(_ => GivenTheMapperWillReturnAnError()) + .When(_ => WhenTheMiddlewareIsInvoked()) + .And(_ => ThenTheDownstreamRequestIsNotStored()) + .And(_ => ThenAPipelineErrorIsStored()) + .And(_ => ThenTheNextMiddlewareIsNotInvoked()) + .BDDfy(); + } + + private void GivenTheHttpContextContainsARequest() + { + _httpContext + .Setup(hc => hc.Request) + .Returns(_httpRequest.Object); + } + + private void GivenTheMapperWillReturnAMappedRequest() + { + _mappedRequest = new OkResponse(new HttpRequestMessage(HttpMethod.Get, "http://www.bbc.co.uk")); + + _requestMapper + .Setup(rm => rm.Map(It.IsAny())) + .ReturnsAsync(_mappedRequest); + } + + private void GivenTheMapperWillReturnAnError() + { + _mappedRequest = new ErrorResponse(new UnmappableRequestError(new System.Exception("boooom!"))); + + _requestMapper + .Setup(rm => rm.Map(It.IsAny())) + .ReturnsAsync(_mappedRequest); + } + + private void WhenTheMiddlewareIsInvoked() + { + _middleware.Invoke(_downstreamContext).GetAwaiter().GetResult(); + } + + private void ThenTheContexRequestIsMappedToADownstreamRequest() + { + _requestMapper.Verify(rm => rm.Map(_httpRequest.Object), Times.Once); + } + + private void ThenTheDownstreamRequestIsStored() + { + _downstreamContext.DownstreamRequest.ShouldNotBeNull(); + } + + private void ThenTheDownstreamRequestIsNotStored() + { + _downstreamContext.DownstreamRequest.ShouldBeNull(); + } + + private void ThenAPipelineErrorIsStored() + { + _downstreamContext.IsError.ShouldBeTrue(); + _downstreamContext.Errors.ShouldBe(_mappedRequest.Errors); + } + + private void ThenTheNextMiddlewareIsInvoked() + { + _next.Verify(n => n(_downstreamContext), Times.Once); + } + + private void ThenTheNextMiddlewareIsNotInvoked() + { + _next.Verify(n => n(It.IsAny()), Times.Never); + } + } +} diff --git a/test/Ocelot.UnitTests/idsrv3test.pfx b/test/Ocelot.UnitTests/idsrv3test.pfx new file mode 100644 index 00000000..0247dea0 Binary files /dev/null and b/test/Ocelot.UnitTests/idsrv3test.pfx differ