#1971 #928 Avoid content if original request has no content and avoid…

… Transfer-Encoding: chunked if Content-Length is known (#1972) * Avoid content if original request has no content and avoid Transfer-Encoding: chunked if Content-Length is known * * Optimized mapping of empty Content * Added tests for mapping requests * Added tests to verify that mapped content is streamed * Changes requested by review * Changes requested by review * Be a little more conservative in streaming test and use 25GB instead if 100GB test data * Reduced streaming test content to 1GB as requested * Convert to file-scoped namespace * Move `Dispose` closer to the constructor * Move `ChunkedContent` class out * IDE1006: Naming rule violation * More private helpers to setup a test * Code review: round 2 * Move common helpers to `Steps` --------- Co-authored-by: Alexander Reinert <a.reinert@impuls-systems.de> Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
ThreeMammals · Feb 29, 2024 · 319e397 · 319e397
1 parent 8845d1b
commit 319e397
Show file tree

Hide file tree

Showing 8 changed files with 483 additions and 137 deletions.
diff --git a/src/Ocelot/Request/Mapper/RequestMapper.cs b/src/Ocelot/Request/Mapper/RequestMapper.cs
@@ -1,87 +1,91 @@
-using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Extensions;
-using Microsoft.Extensions.Primitives;
-using Ocelot.Configuration;
-
-namespace Ocelot.Request.Mapper;
-
-public class RequestMapper : IRequestMapper
-{
-    private static readonly HashSet<string> UnsupportedHeaders = new(StringComparer.OrdinalIgnoreCase) { "host" };
-    private static readonly string[] ContentHeaders = { "Content-Length", "Content-Language", "Content-Location", "Content-Range", "Content-MD5", "Content-Disposition", "Content-Encoding" };
-
-    public HttpRequestMessage Map(HttpRequest request, DownstreamRoute downstreamRoute)
-    {
-        var requestMessage = new HttpRequestMessage
-        {
-            Content = MapContent(request),
-            Method = MapMethod(request, downstreamRoute),
-            RequestUri = MapUri(request),
-            Version = downstreamRoute.DownstreamHttpVersion,
-        };
-
-        MapHeaders(request, requestMessage);
-
-        return requestMessage;
-    }
-
-    private static HttpContent MapContent(HttpRequest request)
-    {
-        // TODO We should check if we really need to call HttpRequest.Body.Length
-        // But we assume that if CanSeek is true, the length is calculated without an important overhead
-        if (request.Body is null or { CanSeek: true, Length: <= 0 })
-        {
-            return null;
-        }
-
-        var content = new StreamHttpContent(request.HttpContext);
-
-        AddContentHeaders(request, content);
-
-        return content;
-    }
-
-    private static void AddContentHeaders(HttpRequest request, HttpContent content)
-    {
-        if (!string.IsNullOrEmpty(request.ContentType))
-        {
-            content.Headers
-                .TryAddWithoutValidation("Content-Type", new[] { request.ContentType });
-        }
-
-        // The performance might be improved by retrieving the matching headers from the request
-        // instead of calling request.Headers.TryGetValue for each used content header
-        var matchingHeaders = ContentHeaders.Where(header => request.Headers.ContainsKey(header));
-
-        foreach (var key in matchingHeaders)
-        {
-            if (!request.Headers.TryGetValue(key, out var value))
-            {
-                continue;
-            }
-
-            content.Headers.TryAddWithoutValidation(key, value.ToArray());
-        }
-    }
-
-    private static HttpMethod MapMethod(HttpRequest request, DownstreamRoute downstreamRoute) => 
-        !string.IsNullOrEmpty(downstreamRoute?.DownstreamHttpMethod) ? 
-            new HttpMethod(downstreamRoute.DownstreamHttpMethod) : new HttpMethod(request.Method);
-
-    // TODO Review this method, request.GetEncodedUrl() could throw a NullReferenceException
-    private static Uri MapUri(HttpRequest request) => new(request.GetEncodedUrl());
-
-    private static void MapHeaders(HttpRequest request, HttpRequestMessage requestMessage)
-    {
-        foreach (var header in request.Headers)
-        {
-            if (IsSupportedHeader(header))
-            {
-                requestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value.ToArray());
-            }
-        }
-    }
-
-    private static bool IsSupportedHeader(KeyValuePair<string, StringValues> header) =>
-        !UnsupportedHeaders.Contains(header.Key);
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Extensions;
+using Microsoft.Extensions.Primitives;
+using Ocelot.Configuration;
+
+namespace Ocelot.Request.Mapper;
+
+public class RequestMapper : IRequestMapper
+{
+    private static readonly HashSet<string> UnsupportedHeaders = new(StringComparer.OrdinalIgnoreCase) { "host", "transfer-encoding" };
+    private static readonly string[] ContentHeaders = { "Content-Length", "Content-Language", "Content-Location", "Content-Range", "Content-MD5", "Content-Disposition", "Content-Encoding" };
+
+    public HttpRequestMessage Map(HttpRequest request, DownstreamRoute downstreamRoute)
+    {
+        var requestMessage = new HttpRequestMessage
+        {
+            Content = MapContent(request),
+            Method = MapMethod(request, downstreamRoute),
+            RequestUri = MapUri(request),
+            Version = downstreamRoute.DownstreamHttpVersion,
+        };
+
+        MapHeaders(request, requestMessage);
+
+        return requestMessage;
+    }
+
+    private static HttpContent MapContent(HttpRequest request)
+    {
+        HttpContent content;
+
+        // No content if we have no body or if the request has no content according to RFC 2616 section 4.3
+        if (request.Body == null
+            || (!request.ContentLength.HasValue && StringValues.IsNullOrEmpty(request.Headers.TransferEncoding)))
+        {
+            return null;
+        }
+
+        content = request.ContentLength is 0
+            ? new ByteArrayContent(Array.Empty<byte>()) 
+            : new StreamHttpContent(request.HttpContext);
+
+        AddContentHeaders(request, content);
+
+        return content;
+    }
+
+    private static void AddContentHeaders(HttpRequest request, HttpContent content)
+    {
+        if (!string.IsNullOrEmpty(request.ContentType))
+        {
+            content.Headers
+                .TryAddWithoutValidation("Content-Type", new[] { request.ContentType });
+        }
+
+        // The performance might be improved by retrieving the matching headers from the request
+        // instead of calling request.Headers.TryGetValue for each used content header
+        var matchingHeaders = ContentHeaders.Where(header => request.Headers.ContainsKey(header));
+
+        foreach (var key in matchingHeaders)
+        {
+            if (!request.Headers.TryGetValue(key, out var value))
+            {
+                continue;
+            }
+
+            content.Headers.TryAddWithoutValidation(key, value.ToArray());
+        }
+    }
+
+    private static HttpMethod MapMethod(HttpRequest request, DownstreamRoute downstreamRoute) => 
+        !string.IsNullOrEmpty(downstreamRoute?.DownstreamHttpMethod) ? 
+            new HttpMethod(downstreamRoute.DownstreamHttpMethod) : new HttpMethod(request.Method);
+
+    // TODO Review this method, request.GetEncodedUrl() could throw a NullReferenceException
+    private static Uri MapUri(HttpRequest request) => new(request.GetEncodedUrl());
+
+    private static void MapHeaders(HttpRequest request, HttpRequestMessage requestMessage)
+    {
+        foreach (var header in request.Headers)
+        {
+            if (IsSupportedHeader(header))
+            {
+                requestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value.ToArray());
+            }
+        }
+    }
+
+    private static bool IsSupportedHeader(KeyValuePair<string, StringValues> header) =>
+        !UnsupportedHeaders.Contains(header.Key);
 }
diff --git a/src/Ocelot/Request/Mapper/StreamHttpContent.cs b/src/Ocelot/Request/Mapper/StreamHttpContent.cs
@@ -8,25 +8,24 @@ public class StreamHttpContent : HttpContent
     private const int DefaultBufferSize = 65536;
     public const long UnknownLength = -1;
     private readonly HttpContext _context;
+    private readonly long _contentLength;
 
     public StreamHttpContent(HttpContext context)
     {
         _context = context ?? throw new ArgumentNullException(nameof(context));
+        _contentLength = context.Request.ContentLength ?? UnknownLength;
     }
 
-    protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context,
-        CancellationToken cancellationToken)
-        => await CopyAsync(_context.Request.Body, stream, Headers.ContentLength ?? UnknownLength, false,
-            cancellationToken);
+    protected override Task SerializeToStreamAsync(Stream stream, TransportContext context, CancellationToken cancellationToken)
+        => CopyAsync(_context.Request.Body, stream, _contentLength, false, cancellationToken);
 
-    protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context)
-        => await CopyAsync(_context.Request.Body, stream, Headers.ContentLength ?? UnknownLength, false,
-            CancellationToken.None);
+    protected override Task SerializeToStreamAsync(Stream stream, TransportContext context)
+        => CopyAsync(_context.Request.Body, stream, _contentLength, false, CancellationToken.None);
 
     protected override bool TryComputeLength(out long length)
     {
-        length = -1;
-        return false;
+        length = _contentLength;
+        return length >= 0;
     }
 
     // This is used internally by HttpContent.ReadAsStreamAsync(...)

diff --git a/test/Ocelot.AcceptanceTests/HttpTests.cs b/test/Ocelot.AcceptanceTests/HttpTests.cs
@@ -1,6 +1,8 @@
+using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Ocelot.Configuration.File;
+using System.Security.Authentication;
 
 namespace Ocelot.AcceptanceTests
 {
@@ -216,25 +218,45 @@ public void should_return_response_200_when_using_http_two_to_talk_to_server_run
         }
 
         private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int port, HttpProtocols protocols)
-        {
-            _serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, basePath, async context =>
+        {
+            void options(KestrelServerOptions serverOptions)
+            {
+                serverOptions.Listen(IPAddress.Loopback, port, listenOptions =>
+                {
+                    listenOptions.Protocols = protocols;
+                });
+            }
+
+            _serviceHandler.GivenThereIsAServiceRunningOnWithKestrelOptions(baseUrl, basePath, options, async context =>
             {
                 context.Response.StatusCode = 200;
                 var reader = new StreamReader(context.Request.Body);
                 var body = await reader.ReadToEndAsync();
                 await context.Response.WriteAsync(body);
-            }, port, protocols);
+            });
         }
 
         private void GivenThereIsAServiceUsingHttpsRunningOn(string baseUrl, string basePath, int port, HttpProtocols protocols)
         {
-            _serviceHandler.GivenThereIsAServiceRunningOnUsingHttps(baseUrl, basePath, async context =>
+            void options(KestrelServerOptions serverOptions)
+            {
+                serverOptions.Listen(IPAddress.Loopback, port, listenOptions =>
+                {
+                    listenOptions.UseHttps("mycert.pfx", "password", options =>
+                    {
+                        options.SslProtocols = SslProtocols.Tls12;
+                    });
+                    listenOptions.Protocols = protocols;
+                });
+            }
+
+            _serviceHandler.GivenThereIsAServiceRunningOnWithKestrelOptions(baseUrl, basePath, options, async context =>
             {
                 context.Response.StatusCode = 200;
                 var reader = new StreamReader(context.Request.Body);
                 var body = await reader.ReadToEndAsync();
                 await context.Response.WriteAsync(body);
-            }, port, protocols);
+            });
         }
 
         public void Dispose()