check which version of .net framework before creating http handler (#412)

* #405 needto check which version of .net we are using but cannot use compiler directives

* #405 started puttig abstraction around static method to get frameworks so we can test this logic

* #405 added test for all methods and tidied up tests

* #405 made contains as ms docs are wrong, thanks to davidni for the heads up

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<IConsulClientFactory, ConsulClientFactory>();
             _services.TryAddSingleton<IResponseAggregatorFactory, InMemoryResponseAggregatorFactory>();
             _services.TryAddSingleton<IDefinedAggregatorProvider, ServiceLocatorDefinedAggregatorProvider>();
+            _services.TryAddSingleton<IDownstreamRequestCreator, DownstreamRequestCreator>();
+            _services.TryAddSingleton<IFrameworkDescription, FrameworkDescription>();
         }
 
         public IOcelotAdministrationBuilder AddAdministration(string path, string secret)

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;
+        }
+    }
+}

src/Ocelot/Infrastructure/IFrameworkDescription.cs

@@ -0,0 +1,7 @@
+namespace Ocelot.Infrastructure
+{
+    public interface IFrameworkDescription
+    {
+        string Get();
+    }
+}

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);
+        }
+    }
+}

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);
+    }
+}

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;
         }

src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs

@@ -1,39 +1,42 @@
-namespace Ocelot.Request.Middleware
+namespace Ocelot.Request.Middleware
-{
+{
-    using System.Net.Http;
+    using System.Threading.Tasks;
-    using System.Threading.Tasks;
+    using Microsoft.AspNetCore.Http;
-    using Microsoft.AspNetCore.Http;
+    using Ocelot.DownstreamRouteFinder.Middleware;
-    using Ocelot.DownstreamRouteFinder.Middleware;
+    using Ocelot.Infrastructure.RequestData;
-    using Ocelot.Infrastructure.RequestData;
+    using Ocelot.Logging;
-    using Ocelot.Logging;
+    using Ocelot.Middleware;
-    using Ocelot.Middleware;
+    using Ocelot.Request.Creator;
-
+
-    public class DownstreamRequestInitialiserMiddleware : OcelotMiddleware
+    public class DownstreamRequestInitialiserMiddleware : OcelotMiddleware
-    {
+    {
-        private readonly OcelotRequestDelegate _next;
+        private readonly OcelotRequestDelegate _next;
-        private readonly Mapper.IRequestMapper _requestMapper;
+        private readonly Mapper.IRequestMapper _requestMapper;
-
+        private readonly IDownstreamRequestCreator _creator;
-        public DownstreamRequestInitialiserMiddleware(OcelotRequestDelegate next,
+
-            IOcelotLoggerFactory loggerFactory,
+        public DownstreamRequestInitialiserMiddleware(OcelotRequestDelegate next,
-            Mapper.IRequestMapper requestMapper)
+            IOcelotLoggerFactory loggerFactory,
-                :base(loggerFactory.CreateLogger<DownstreamRequestInitialiserMiddleware>())
+            Mapper.IRequestMapper requestMapper,
-        {
+            IDownstreamRequestCreator creator)
-            _next = next;
+                :base(loggerFactory.CreateLogger<DownstreamRequestInitialiserMiddleware>())
-            _requestMapper = requestMapper;
+        {
-        }
+            _next = next;
-
+            _requestMapper = requestMapper;
-        public async Task Invoke(DownstreamContext context)
+            _creator = creator;
-        {
+        }
-            var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request);
+
-            if (downstreamRequest.IsError)
+        public async Task Invoke(DownstreamContext context)
-            {
+        {
-                SetPipelineError(context, downstreamRequest.Errors);
+            var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request);
-                return;
+            if (downstreamRequest.IsError)
-            }
+            {
-
+                SetPipelineError(context, downstreamRequest.Errors);
-            context.DownstreamRequest = new DownstreamRequest(downstreamRequest.Data);
+                return;
-
+            }
-            await _next.Invoke(context);
+
-        }
+            context.DownstreamRequest = _creator.Create(downstreamRequest.Data);
-    }
+
-}
+            await _next.Invoke(context);
+        }
+    }
+}

test/Ocelot.ManualTest/ocelot.json

@@ -114,7 +114,8 @@
       "HttpHandlerOptions": {
         "AllowAutoRedirect": true,
         "UseCookieContainer": true,
-        "UseTracing": true
+        "UseTracing": true,
+        "UseProxy": true
       },
       "QoSOptions": {
         "ExceptionsAllowedBeforeBreaking": 3,

test/Ocelot.UnitTests/Ocelot.UnitTests.csproj

@@ -28,6 +28,15 @@
   <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
   </ItemGroup>
 
+    <ItemGroup>
+    <None Update="appsettings.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+    <None Update="idsrv3test.pfx">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.8" />
     <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="2.0.3" />

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<IFrameworkDescription> _framework;
+        private DownstreamRequestCreator _downstreamRequestCreator;
+        private HttpRequestMessage _request;
+        private DownstreamRequest _result;
+
+        public DownstreamRequestCreatorTests()
+        {
+            _framework = new Mock<IFrameworkDescription>();
+            _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> { 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();
+        }
+    }
+}

test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs

@@ -1,143 +1,146 @@
-using Ocelot.Middleware;
+using Ocelot.Middleware;
-
+
-namespace Ocelot.UnitTests.Request
+namespace Ocelot.UnitTests.Request
-{
+{
-    using System.Net.Http;
+    using System.Net.Http;
-    using Microsoft.AspNetCore.Http;
+    using Microsoft.AspNetCore.Http;
-    using Moq;
+    using Moq;
-    using Ocelot.Logging;
+    using Ocelot.Logging;
-    using Ocelot.Request.Mapper;
+    using Ocelot.Request.Mapper;
-    using Ocelot.Request.Middleware;
+    using Ocelot.Request.Middleware;
-    using Ocelot.Infrastructure.RequestData;
+    using Ocelot.Infrastructure.RequestData;
-    using TestStack.BDDfy;
+    using TestStack.BDDfy;
-    using Xunit;
+    using Xunit;
-    using Ocelot.Responses;
+    using Ocelot.Responses;
-    using Ocelot.DownstreamRouteFinder.Middleware;
+    using Ocelot.DownstreamRouteFinder.Middleware;
-    using Shouldly;
+    using Shouldly;
-
+    using Ocelot.Request.Creator;
-    public class DownstreamRequestInitialiserMiddlewareTests
+    using Ocelot.Infrastructure;
-    {
+
-        readonly DownstreamRequestInitialiserMiddleware _middleware;
+    public class DownstreamRequestInitialiserMiddlewareTests
-
+    {
-        readonly Mock<HttpContext> _httpContext;
+        readonly DownstreamRequestInitialiserMiddleware _middleware;
-
+
-        readonly Mock<HttpRequest> _httpRequest;
+        readonly Mock<HttpContext> _httpContext;
-
+
-        readonly Mock<OcelotRequestDelegate> _next;
+        readonly Mock<HttpRequest> _httpRequest;
-
+
-        readonly Mock<IRequestMapper> _requestMapper;
+        readonly Mock<OcelotRequestDelegate> _next;
-
+
-        readonly Mock<IOcelotLoggerFactory> _loggerFactory;
+        readonly Mock<IRequestMapper> _requestMapper;
-
+
-        readonly Mock<IOcelotLogger> _logger;
+        readonly Mock<IOcelotLoggerFactory> _loggerFactory;
-
+
-        Response<HttpRequestMessage> _mappedRequest;
+        readonly Mock<IOcelotLogger> _logger;
-        private DownstreamContext _downstreamContext;
+
-
+        Response<HttpRequestMessage> _mappedRequest;
-        public DownstreamRequestInitialiserMiddlewareTests()
+        private DownstreamContext _downstreamContext;
-        {
+
-            _httpContext = new Mock<HttpContext>();
+        public DownstreamRequestInitialiserMiddlewareTests()
-            _httpRequest = new Mock<HttpRequest>();
+        {
-            _requestMapper = new Mock<IRequestMapper>();
+            _httpContext = new Mock<HttpContext>();
-            _next = new Mock<OcelotRequestDelegate>();
+            _httpRequest = new Mock<HttpRequest>();
-            _logger = new Mock<IOcelotLogger>();
+            _requestMapper = new Mock<IRequestMapper>();
-
+            _next = new Mock<OcelotRequestDelegate>();
-            _loggerFactory = new Mock<IOcelotLoggerFactory>();
+            _logger = new Mock<IOcelotLogger>();
-            _loggerFactory
+
-                .Setup(lf => lf.CreateLogger<DownstreamRequestInitialiserMiddleware>())
+            _loggerFactory = new Mock<IOcelotLoggerFactory>();
-                .Returns(_logger.Object);
+            _loggerFactory
-
+                .Setup(lf => lf.CreateLogger<DownstreamRequestInitialiserMiddleware>())
-            _middleware = new DownstreamRequestInitialiserMiddleware(
+                .Returns(_logger.Object);
-                _next.Object, 
+
-                _loggerFactory.Object, 
+            _middleware = new DownstreamRequestInitialiserMiddleware(
-                _requestMapper.Object);
+                _next.Object, 
-
+                _loggerFactory.Object, 
-            _downstreamContext = new DownstreamContext(_httpContext.Object);
+                _requestMapper.Object,
-        }
+                new DownstreamRequestCreator(new FrameworkDescription()));
-
+
-        [Fact]
+            _downstreamContext = new DownstreamContext(_httpContext.Object);
-        public void Should_handle_valid_httpRequest()
+        }
-        {
+
-            this.Given(_ => GivenTheHttpContextContainsARequest())
+        [Fact]
-                .And(_ => GivenTheMapperWillReturnAMappedRequest())
+        public void Should_handle_valid_httpRequest()
-                .When(_ => WhenTheMiddlewareIsInvoked())
+        {
-                .Then(_ => ThenTheContexRequestIsMappedToADownstreamRequest())
+            this.Given(_ => GivenTheHttpContextContainsARequest())
-                .And(_ => ThenTheDownstreamRequestIsStored())
+                .And(_ => GivenTheMapperWillReturnAMappedRequest())
-                .And(_ => ThenTheNextMiddlewareIsInvoked())
+                .When(_ => WhenTheMiddlewareIsInvoked())
-                .BDDfy();
+                .Then(_ => ThenTheContexRequestIsMappedToADownstreamRequest())
-        }
+                .And(_ => ThenTheDownstreamRequestIsStored())
-
+                .And(_ => ThenTheNextMiddlewareIsInvoked())
-        [Fact]
+                .BDDfy();
-        public void Should_handle_mapping_failure()
+        }
-        {
+
-            this.Given(_ => GivenTheHttpContextContainsARequest())
+        [Fact]
-                .And(_ => GivenTheMapperWillReturnAnError())
+        public void Should_handle_mapping_failure()
-                .When(_ => WhenTheMiddlewareIsInvoked())
+        {
-                .And(_ => ThenTheDownstreamRequestIsNotStored())
+            this.Given(_ => GivenTheHttpContextContainsARequest())
-                .And(_ => ThenAPipelineErrorIsStored())
+                .And(_ => GivenTheMapperWillReturnAnError())
-                .And(_ => ThenTheNextMiddlewareIsNotInvoked())
+                .When(_ => WhenTheMiddlewareIsInvoked())
-                .BDDfy();
+                .And(_ => ThenTheDownstreamRequestIsNotStored())
-        }
+                .And(_ => ThenAPipelineErrorIsStored())
-
+                .And(_ => ThenTheNextMiddlewareIsNotInvoked())
-        private void GivenTheHttpContextContainsARequest()
+                .BDDfy();
-        {
+        }
-            _httpContext
+
-                .Setup(hc => hc.Request)
+        private void GivenTheHttpContextContainsARequest()
-                .Returns(_httpRequest.Object);
+        {
-        }
+            _httpContext
-
+                .Setup(hc => hc.Request)
-        private void GivenTheMapperWillReturnAMappedRequest()
+                .Returns(_httpRequest.Object);
-        {
+        }
-            _mappedRequest = new OkResponse<HttpRequestMessage>(new HttpRequestMessage(HttpMethod.Get, "http://www.bbc.co.uk"));
+
-
+        private void GivenTheMapperWillReturnAMappedRequest()
-            _requestMapper
+        {
-                .Setup(rm => rm.Map(It.IsAny<HttpRequest>()))
+            _mappedRequest = new OkResponse<HttpRequestMessage>(new HttpRequestMessage(HttpMethod.Get, "http://www.bbc.co.uk"));
-                .ReturnsAsync(_mappedRequest);
+
-        }
+            _requestMapper
-
+                .Setup(rm => rm.Map(It.IsAny<HttpRequest>()))
-        private void GivenTheMapperWillReturnAnError()
+                .ReturnsAsync(_mappedRequest);
-        {
+        }
-            _mappedRequest = new ErrorResponse<HttpRequestMessage>(new UnmappableRequestError(new System.Exception("boooom!")));
+
-
+        private void GivenTheMapperWillReturnAnError()
-            _requestMapper
+        {
-                .Setup(rm => rm.Map(It.IsAny<HttpRequest>()))
+            _mappedRequest = new ErrorResponse<HttpRequestMessage>(new UnmappableRequestError(new System.Exception("boooom!")));
-                .ReturnsAsync(_mappedRequest);
+
-        }
+            _requestMapper
-
+                .Setup(rm => rm.Map(It.IsAny<HttpRequest>()))
-        private void WhenTheMiddlewareIsInvoked()
+                .ReturnsAsync(_mappedRequest);
-        {
+        }
-           _middleware.Invoke(_downstreamContext).GetAwaiter().GetResult();
+
-        }
+        private void WhenTheMiddlewareIsInvoked()
-
+        {
-        private void ThenTheContexRequestIsMappedToADownstreamRequest()
+           _middleware.Invoke(_downstreamContext).GetAwaiter().GetResult();
-        {
+        }
-            _requestMapper.Verify(rm => rm.Map(_httpRequest.Object), Times.Once);
+
-        }
+        private void ThenTheContexRequestIsMappedToADownstreamRequest()
-
+        {
-        private void ThenTheDownstreamRequestIsStored()
+            _requestMapper.Verify(rm => rm.Map(_httpRequest.Object), Times.Once);
-        {
+        }
-            _downstreamContext.DownstreamRequest.ShouldNotBeNull();
+
-        }
+        private void ThenTheDownstreamRequestIsStored()
-
+        {
-        private void ThenTheDownstreamRequestIsNotStored()
+            _downstreamContext.DownstreamRequest.ShouldNotBeNull();
-        {
+        }
-            _downstreamContext.DownstreamRequest.ShouldBeNull();
+
-        }
+        private void ThenTheDownstreamRequestIsNotStored()
-
+        {
-        private void ThenAPipelineErrorIsStored()
+            _downstreamContext.DownstreamRequest.ShouldBeNull();
-        {
+        }
-            _downstreamContext.IsError.ShouldBeTrue();
+
-            _downstreamContext.Errors.ShouldBe(_mappedRequest.Errors);
+        private void ThenAPipelineErrorIsStored()
-        }
+        {
-
+            _downstreamContext.IsError.ShouldBeTrue();
-        private void ThenTheNextMiddlewareIsInvoked()
+            _downstreamContext.Errors.ShouldBe(_mappedRequest.Errors);
-        {
+        }
-            _next.Verify(n => n(_downstreamContext), Times.Once);
+
-        }
+        private void ThenTheNextMiddlewareIsInvoked()
-
+        {
-        private void ThenTheNextMiddlewareIsNotInvoked()
+            _next.Verify(n => n(_downstreamContext), Times.Once);
-        {
+        }
-            _next.Verify(n => n(It.IsAny<DownstreamContext>()), Times.Never);
+
-        }
+        private void ThenTheNextMiddlewareIsNotInvoked()
-    }
+        {
-}
+            _next.Verify(n => n(It.IsAny<DownstreamContext>()), Times.Never);
+        }
+    }
+}