#827 #1679 Improve performance of Request Mapper

docs/introduction/gotchas.rst

@@ -1,12 +1,15 @@
 Gotchas
 =============
-
+
+Many errors and incidents (gotchas) are related to web server hosting scenarios.
+Please review deployment and web hosting common user scenarios below depending on your web server.
+
 IIS
------
+---
 
     Microsoft Learn: `Host ASP.NET Core on Windows with IIS <https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/iis/>`_
 
-We do not recommend to deploy Ocelot app to IIS environments, but if you do, keep in mind the gotchas below.
+We **do not** recommend to deploy Ocelot app to IIS environments, but if you do, keep in mind the gotchas below.
 
 * When using ASP.NET Core 2.2+ and you want to use In-Process hosting, replace ``UseIISIntegration()`` with ``UseIIS()``, otherwise you will get startup errors.
 
@@ -23,3 +26,36 @@ Probably you will find a ready solution by Ocelot community members.
 Finally, we have special label |IIS| for all IIS related objects. Feel free to put this label onto issues, PRs, discussions, etc.
 
 .. |IIS| image:: https://img.shields.io/badge/-IIS-c5def5.svg
+
+Kestrel
+-------
+
+    Microsoft Learn: `Kestrel web server in ASP.NET Core <https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel>`_
+
+We **do** recommend to deploy Ocelot app to self-hosting environments, aka Kestrel vs Docker.
+We try to optimize Ocelot web app for Kestrel & Docker hosting scenarios, but keep in mind the following gotchas.
+
+* **Upload and download large files**, proxying the content through the gateway. It is strange when you pump large (static) files using the gateway.
+  We believe that your client apps should have direct integration to (static) files persistent storages and services: remote & destributed file systems, CDNs, static files & blob storages, etc.
+  We **do not** recommend to pump large files (100Mb+ or even larger 1GB+) using gateway because of performance reasons: consuming memory and CPU, long delay times, producing network errors for downstream streaming, impact on other routes.
+
+  | The community constanly reports issues related to `large files <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+%22large+file%22&type=issues>`_ (``application/octet-stream`` content type, :ref:`chunked-encoding`, etc.), see issues `749 <https://github.com/ThreeMammals/Ocelot/issues/749>`_, `1472 <https://github.com/ThreeMammals/Ocelot/issues/1472>`_.
+  If you still want to pump large files through an Ocelot gateway instance, we believe our PRs (`1724 <https://github.com/ThreeMammals/Ocelot/pull/1724>`_, `1769 <https://github.com/ThreeMammals/Ocelot/pull/1769>`_) will help resolve the issues and stabilize large content proxying.
+  In case of some errors, see the next point.
+
+* **Maximum request body size**. ASP.NET ``HttpRequest`` behaves erroneously for application instances that do not have their Kestrel `MaxRequestBodySize <https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserverlimits.maxrequestbodysize>`_ option configured correctly and having pumped large files of unpredictable size which exceeds the limit.
+
+  | Please review these docs: `Maximum request body size | Configure options for the ASP.NET Core Kestrel web server <https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/options#maximum-request-body-size>`_.
+
+  | As a quick fix, use this configuration recipe:
+
+  .. code-block:: csharp
+
+      builder.WebHost.ConfigureKestrel((context, serverOptions) =>
+      {
+          int myVideoFileMaxSize = 1_073_741_824; // assume your file storage has max file size as 1 GB (1_073_741_824)
+          int totalSize = myVideoFileMaxSize + 26_258_176; // and add some extra size
+          serverOptions.Limits.MaxRequestBodySize = totalSize; // 1_100_000_000 thus 1 GB file should not exceed the limit
+      });
+
+  Hope it helps.

docs/introduction/notsupported.rst

@@ -2,7 +2,9 @@ Not Supported
 =============
 
 Ocelot does not support...
-
+
+.. _chunked-encoding:
+
 Chunked Encoding
 ----------------
 

src/Ocelot/Request/Mapper/IRequestMapper.cs

@@ -1,11 +1,9 @@
 using Microsoft.AspNetCore.Http;
 using Ocelot.Configuration;
-using Ocelot.Responses;
-
-namespace Ocelot.Request.Mapper
-{
-    public interface IRequestMapper
-    {
-        Task<Response<HttpRequestMessage>> Map(HttpRequest request, DownstreamRoute downstreamRoute);
-    }
+
+namespace Ocelot.Request.Mapper;
+
+public interface IRequestMapper
+{
+    HttpRequestMessage Map(HttpRequest request, DownstreamRoute downstreamRoute);
 }

src/Ocelot/Request/Mapper/RequestMapper.cs

@@ -2,109 +2,86 @@
 using Microsoft.AspNetCore.Http.Extensions;
 using Microsoft.Extensions.Primitives;
 using Ocelot.Configuration;
-using Ocelot.Responses;
-
-namespace Ocelot.Request.Mapper
-{
-    public class RequestMapper : IRequestMapper
-    {
-        private readonly string[] _unsupportedHeaders = { "host" };
-
-        public async Task<Response<HttpRequestMessage>> Map(HttpRequest request, DownstreamRoute downstreamRoute)
-        {
-            try
-            {
-                var requestMessage = new HttpRequestMessage
-                {
-                    Content = await MapContent(request),
-                    Method = MapMethod(request, downstreamRoute),
-                    RequestUri = MapUri(request),
-                    Version = downstreamRoute.DownstreamHttpVersion,
-                };
-
-                MapHeaders(request, requestMessage);
-
-                return new OkResponse<HttpRequestMessage>(requestMessage);
-            }
-            catch (Exception ex)
-            {
-                return new ErrorResponse<HttpRequestMessage>(new UnmappableRequestError(ex));
-            }
-        }
-
-        private static async Task<HttpContent> MapContent(HttpRequest request)
-        {
-            if (request.Body == null || (request.Body.CanSeek && request.Body.Length <= 0))
-            {
-                return null;
-            }
-
-            // Never change this to StreamContent again, I forgot it doesnt work in #464.
-            var content = new ByteArrayContent(await ToByteArray(request.Body));
-
-            if (!string.IsNullOrEmpty(request.ContentType))
-            {
-                content.Headers
-                    .TryAddWithoutValidation("Content-Type", new[] { request.ContentType });
-            }
-
-            AddHeaderIfExistsOnRequest("Content-Language", content, request);
-            AddHeaderIfExistsOnRequest("Content-Location", content, request);
-            AddHeaderIfExistsOnRequest("Content-Range", content, request);
-            AddHeaderIfExistsOnRequest("Content-MD5", content, request);
-            AddHeaderIfExistsOnRequest("Content-Disposition", content, request);
-            AddHeaderIfExistsOnRequest("Content-Encoding", content, request);
-
-            return content;
-        }
-
-        private static void AddHeaderIfExistsOnRequest(string key, HttpContent content, HttpRequest request)
-        {
-            if (request.Headers.ContainsKey(key))
-            {
-                content.Headers
-                    .TryAddWithoutValidation(key, request.Headers[key].ToArray());
-            }
-        }
-
-        private static HttpMethod MapMethod(HttpRequest request, DownstreamRoute downstreamRoute)
-        {
-            if (!string.IsNullOrEmpty(downstreamRoute?.DownstreamHttpMethod))
-            {
-                return new HttpMethod(downstreamRoute.DownstreamHttpMethod);
-            }
-
-            return new HttpMethod(request.Method);
-        }
-
-        private static Uri MapUri(HttpRequest request) => new(request.GetEncodedUrl());
-
-        private void MapHeaders(HttpRequest request, HttpRequestMessage requestMessage)
-        {
-            foreach (var header in request.Headers)
-            {
-                if (IsSupportedHeader(header))
-                {
-                    requestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value.ToArray());
-                }
-            }
-        }
-
-        private bool IsSupportedHeader(KeyValuePair<string, StringValues> header)
-        {
-            return !_unsupportedHeaders.Contains(header.Key.ToLower());
-        }
-
-        private static async Task<byte[]> ToByteArray(Stream stream)
-        {
-            await using (stream)
-            {
-                using (var memStream = new MemoryStream())
-                {
-                    await stream.CopyToAsync(memStream);
-                    return memStream.ToArray();
-                }
-            }
-        }
-    }
+
+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);
 }

src/Ocelot/Request/Mapper/StreamHttpContent.cs

@@ -0,0 +1,113 @@
+using Microsoft.AspNetCore.Http;
+using System.Buffers;
+
+namespace Ocelot.Request.Mapper;
+
+public class StreamHttpContent : HttpContent
+{
+    private const int DefaultBufferSize = 65536;
+    public const long UnknownLength = -1;
+    private readonly HttpContext _context;
+
+    public StreamHttpContent(HttpContext context)
+    {
+        _context = context ?? throw new ArgumentNullException(nameof(context));
+    }
+
+    protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context,
+        CancellationToken cancellationToken)
+        => await CopyAsync(_context.Request.Body, stream, Headers.ContentLength ?? UnknownLength, 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 bool TryComputeLength(out long length)
+    {
+        length = -1;
+        return false;
+    }
+
+    // This is used internally by HttpContent.ReadAsStreamAsync(...)
+    protected override Task<Stream> CreateContentReadStreamAsync()
+    {
+        // Nobody should be calling this...
+        throw new NotImplementedException();
+    }
+
+    private static async Task CopyAsync(Stream input, Stream output, long announcedContentLength,
+        bool autoFlush, CancellationToken cancellation)
+    {
+        // For smaller payloads, avoid allocating a buffer that is larger than the announced content length
+        var minBufferSize = announcedContentLength != UnknownLength && announcedContentLength < DefaultBufferSize
+            ? (int)announcedContentLength
+            : DefaultBufferSize;
+
+        var buffer = ArrayPool<byte>.Shared.Rent(minBufferSize);
+        long contentLength = 0;
+        try
+        {
+            while (true)
+            {
+                // Issue a zero-byte read to the input stream to defer buffer allocation until data is available.
+                // Note that if the underlying stream does not supporting blocking on zero byte reads, then this will
+                // complete immediately and won't save any memory, but will still function correctly.
+                var zeroByteReadTask = input.ReadAsync(Memory<byte>.Empty, cancellation);
+                if (zeroByteReadTask.IsCompletedSuccessfully)
+                {
+                    // Consume the ValueTask's result in case it is backed by an IValueTaskSource
+                    _ = zeroByteReadTask.Result;
+                }
+                else
+                {
+                    // Take care not to return the same buffer to the pool twice in case zeroByteReadTask throws
+                    var bufferToReturn = buffer;
+                    buffer = null;
+                    ArrayPool<byte>.Shared.Return(bufferToReturn);
+
+                    await zeroByteReadTask;
+
+                    buffer = ArrayPool<byte>.Shared.Rent(minBufferSize);
+                }
+
+                var read = await input.ReadAsync(buffer.AsMemory(), cancellation);
+                contentLength += read;
+
+                // Normally this is enforced by the server, but it could get out of sync if something in the proxy modified the body.
+                if (announcedContentLength != UnknownLength && contentLength > announcedContentLength)
+                {
+                    throw new InvalidOperationException($"More data ({contentLength} bytes) received than the specified Content-Length of {announcedContentLength} bytes.");
+                }
+
+                // End of the source stream.
+                if (read == 0)
+                {
+                    if (announcedContentLength == UnknownLength || contentLength == announcedContentLength)
+                    {
+                        return;
+                    }
+                    else
+                    {
+                        throw new InvalidOperationException($"Sent {contentLength} request content bytes, but Content-Length promised {announcedContentLength}.");
+                    }
+                }
+
+                await output.WriteAsync(buffer.AsMemory(0, read), cancellation);
+                if (autoFlush)
+                {
+                    // HttpClient doesn't always flush outgoing data unless the buffer is full or the caller asks.
+                    // This is a problem for streaming protocols like WebSockets and gRPC.
+                    await output.FlushAsync(cancellation);
+                }
+            }
+        }
+        finally
+        {
+            if (buffer != null)
+            {
+                ArrayPool<byte>.Shared.Return(buffer);
+            }
+        }
+    }
+}