#2375 #2376 Map non-ASCII header HttpRequestException to 400 Bad Request

src/Ocelot/Errors/OcelotErrorCode.cs

@@ -44,4 +44,5 @@ public enum OcelotErrorCode
     CouldNotFindLoadBalancerCreator = 39,
     ErrorInvokingLoadBalancerCreator = 40,
     PayloadTooLargeError = 41,
+    BadRequestError = 42,
 }

src/Ocelot/Requester/BadRequestError.cs

@@ -0,0 +1,12 @@
+using Microsoft.AspNetCore.Http;
+using Ocelot.Errors;
+
+namespace Ocelot.Requester;
+
+public class BadRequestError : Error
+{
+    public BadRequestError(string message) 
+        : base(message, OcelotErrorCode.BadRequestError, StatusCodes.Status400BadRequest)
+    {
+    }
+}

src/Ocelot/Requester/HttpExceptionToErrorMapper.cs

@@ -47,6 +47,14 @@ public Error Map(Exception exception)
                 return new PayloadTooLargeError(exception);
             }
 
+            // Late Catch: Map the HttpRequestException to a 400 Bad Request.
+            // If the header format is invalid, HttpClient throws this exception locally.
+            // By catching it here, we ensure Ocelot returns a clean 4xx client error instead of a generic 502 Bad Gateway.
+            if (exception is HttpRequestException && exception.Message.Contains("only ASCII characters"))
+            {
+                return new BadRequestError(exception.Message);
+            }
+
             return new ConnectionToDownstreamServiceError(exception);
         }
 

src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs

@@ -61,6 +61,11 @@ public int Map(List<Error> errors)
             return 413;
         }
 
+        if (errors.Any(e => e.Code == OcelotErrorCode.BadRequestError))
+        {
+            return StatusCodes.Status400BadRequest;
+        }
+
         return 404;
     }
 }

test/Ocelot.AcceptanceTests/Requester/InvalidHeaderValueTests.cs

@@ -0,0 +1,86 @@
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
+using System.Net.Sockets;
+using System.Text;
+using Ocelot.Configuration.File;
+
+namespace Ocelot.AcceptanceTests.Requester;
+
+[Trait("Bug", "2376")]
+[Trait("PR", "2381")]
+public sealed class InvalidHeaderValueTests : Steps
+{
+    private const int BasePortSeed = 20000;
+    private const int PortStride = 2;
+    private const int RequestTimeoutSeconds = 10;
+    private const int ReadBufferSize = 4096;
+    private const string GatewayRequestPath = "/ocelot/posts/askdj";
+    private const string DownstreamRequestPath = "/todos/askdj";
+    private const string DownstreamResponseBody = "Hello from Laura";
+    private const string HostHeaderName = "Host";
+    private const string AcceptHeaderName = "Accept";
+    private const string ConnectionHeaderName = "Connection";
+    private const string TestHeaderName = "skull";
+    private const string ExpectedStatusLine = "HTTP/1.1 400 Bad Request";
+
+    [Theory]
+    [InlineData("💀")]
+    [InlineData("é")]
+    [InlineData("漢")]
+    public async Task Should_return_400_bad_request_when_request_contains_non_ascii_header_value(string headerValue)
+    {
+        var basePort = BasePortSeed + (Environment.ProcessId % 10000) * PortStride;
+        var downstreamPort = basePort;
+        var gatewayPort = basePort + 1;
+        var route = GivenRoute(downstreamPort, "/ocelot/posts/{id}", "/todos/{id}");
+        var configuration = GivenConfiguration(route);
+
+        GivenThereIsAConfiguration(configuration);
+        GivenThereIsAServiceRunningOn(downstreamPort, DownstreamRequestPath, context =>
+        {
+            context.Response.StatusCode = (int)HttpStatusCode.OK;
+            return context.Response.WriteAsync(DownstreamResponseBody);
+        });
+        await GivenOcelotHostIsRunning(null, null, null, builder => builder
+            .UseKestrel()
+            .ConfigureAppConfiguration(WithBasicConfiguration)
+            .ConfigureServices(WithAddOcelot)
+            .Configure(WithUseOcelot)
+            .UseUrls(DownstreamUrl(gatewayPort)), null, null, null);
+
+        var response = await SendRawRequestAsync(gatewayPort, headerValue);
+
+        response.FirstLine().ShouldBe(ExpectedStatusLine);
+    }
+
+    private static async Task<string> SendRawRequestAsync(int port, string headerValue)
+    {
+        using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(RequestTimeoutSeconds));
+        using var client = new TcpClient();
+        await client.ConnectAsync(System.Net.IPAddress.Loopback, port).WaitAsync(timeout.Token);
+
+        using var stream = client.GetStream();
+        var request = $"GET {GatewayRequestPath} HTTP/1.1\r\n{HostHeaderName}: localhost:{port}\r\n{AcceptHeaderName}: */*\r\n{TestHeaderName}: {headerValue}\r\n{ConnectionHeaderName}: close\r\n\r\n";
+        var requestBytes = Encoding.UTF8.GetBytes(request);
+        await stream.WriteAsync(requestBytes, timeout.Token);
+        await stream.FlushAsync(timeout.Token);
+
+        var buffer = new byte[ReadBufferSize];
+        var response = new StringBuilder();
+        int read;
+
+        while ((read = await stream.ReadAsync(buffer, timeout.Token)) > 0)
+        {
+            response.Append(Encoding.UTF8.GetString(buffer, 0, read));
+        }
+
+        return response.ToString();
+    }
+}
+
+internal static class InvalidHeaderValueTestsExtensions
+{
+    public static string FirstLine(this string response)
+    => response.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries)[0];
+}

test/Ocelot.UnitTests/Kubernetes/PollKubeTests.cs

@@ -132,4 +132,127 @@ public async Task GetAsync()
         Assert.Same(latestVersion, actual);
         _discoveryProvider.Verify(x => x.GetAsync(), Times.Never);
     }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task GetAsync_WhenDisposed_ReturnsEmpty()
+    {
+        // Arrange
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        _provider.Dispose();
+
+        // Act
+        var actual = await _provider.GetAsync();
+
+        // Assert
+        Assert.Same(PollKube.Empty, actual);
+    }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task PollAsync_WhenQueueCountGreaterThan3_ReturnsEmpty()
+    {
+        // Arrange
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = _provider.GetType().GetMethod("PollAsync", BindingFlags.Instance | BindingFlags.NonPublic);
+        var queueField = _provider.GetType().GetField("_queue", BindingFlags.Instance | BindingFlags.NonPublic);
+        var queue = (ConcurrentQueue<List<Service>>)queueField.GetValue(_provider);
+        for (int i = 0; i < 4; i++)
+        {
+            queue.Enqueue(new List<Service>());
+        }
+
+        // Act
+        var task = (Task<List<Service>>)method.Invoke(_provider, [CancellationToken.None]);
+        var actual = await task;
+
+        // Assert
+        Assert.Same(PollKube.Empty, actual);
+        _discoveryProvider.Verify(x => x.GetAsync(), Times.Never);
+    }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task PollAsync_WhenObjectDisposedExceptionThrown_ReturnsEmpty()
+    {
+        // Arrange
+        _discoveryProvider.Setup(x => x.GetAsync()).ThrowsAsync(new ObjectDisposedException("provider"));
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = _provider.GetType().GetMethod("PollAsync", BindingFlags.Instance | BindingFlags.NonPublic);
+
+        // Act
+        var task = (Task<List<Service>>)method.Invoke(_provider, [CancellationToken.None]);
+        var actual = await task;
+
+        // Assert
+        Assert.Same(PollKube.Empty, actual);
+    }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task PollAsync_WhenCancelled_ReturnsEmpty()
+    {
+        // Arrange
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = _provider.GetType().GetMethod("PollAsync", BindingFlags.Instance | BindingFlags.NonPublic);
+        var cts = new CancellationTokenSource();
+        cts.Cancel();
+
+        // Act
+        var task = (Task<List<Service>>)method.Invoke(_provider, [cts.Token]);
+        var actual = await task;
+
+        // Assert
+        Assert.Same(PollKube.Empty, actual);
+        _discoveryProvider.Verify(x => x.GetAsync(), Times.Never);
+    }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task PollAsync_WhenCancelledDuringWait_ReturnsEmpty()
+    {
+        // Arrange
+        var tcs = new TaskCompletionSource<List<Service>>();
+        _discoveryProvider.Setup(x => x.GetAsync()).Returns(tcs.Task);
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = _provider.GetType().GetMethod("PollAsync", BindingFlags.Instance | BindingFlags.NonPublic);
+        var innerCts = new CancellationTokenSource();
+
+        // Act
+        var task = (Task<List<Service>>)method.Invoke(_provider, [innerCts.Token]);
+        innerCts.Cancel();
+        tcs.SetResult(new List<Service>());
+        var actual = await task;
+
+        // Assert
+        Assert.Same(PollKube.Empty, actual);
+    }
+
+    [Fact]
+    public void Finalizer_DoesNotThrow()
+    {
+        var instance = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = instance.GetType().GetMethod("Dispose", BindingFlags.Instance | BindingFlags.NonPublic);
+        method.Invoke(instance, [false]);
+    }
+
+    [Fact]
+    [Trait("Bug", "2304")]
+    public async Task StartAsync_WhenProviderDisposed_CatchesOperationCanceledException()
+    {
+        // Arrange
+        _discoveryProvider.Setup(x => x.GetAsync()).ReturnsAsync(new List<Service>());
+        _provider = new PollKube(10_000, _factory.Object, _discoveryProvider.Object);
+        var method = _provider.GetType().GetMethod("StartAsync", BindingFlags.Instance | BindingFlags.NonPublic);
+        var ctsField = _provider.GetType().GetField("_cts", BindingFlags.Instance | BindingFlags.NonPublic);
+        var cts = (CancellationTokenSource)ctsField.GetValue(_provider);
+
+        // Act
+        var task = (Task)method.Invoke(_provider, null);
+        cts.Cancel(); // This will trigger OperationCanceledException in StartAsync loop
+        await task; // Should not throw
+
+        // Assert
+        Assert.True(task.IsCompletedSuccessfully); // Task ended without bubbling up the exception
+    }
 }

test/Ocelot.UnitTests/Requester/BadRequestErrorTests.cs

@@ -0,0 +1,23 @@
+using Microsoft.AspNetCore.Http;
+using Ocelot.Errors;
+using Ocelot.Requester;
+
+namespace Ocelot.UnitTests.Requester;
+
+public class BadRequestErrorTests
+{
+    [Fact]
+    public void Should_create_bad_request_error()
+    {
+        // Arrange
+        var message = "This is a bad request message.";
+
+        // Act
+        var error = new BadRequestError(message);
+
+        // Assert
+        error.Message.ShouldBe(message);
+        error.Code.ShouldBe(OcelotErrorCode.BadRequestError);
+        error.HttpStatusCode.ShouldBe(StatusCodes.Status400BadRequest);
+    }
+}

test/Ocelot.UnitTests/Requester/HttpExceptionToErrorMapperTests.cs

@@ -52,6 +52,21 @@ public void Should_return_ConnectionToDownstreamServiceError()
         error.ShouldBeOfType<ConnectionToDownstreamServiceError>();
     }
 
+    [Fact]
+    public void Map_HttpRequestException_With_NonASCII_To_BadRequestError()
+    {
+        // Arrange
+        var ex = new HttpRequestException("Some header contains only ASCII characters but it doesn't.");
+
+        // Act
+        var error = _mapper.Map(ex);
+
+        // Assert
+        Assert.IsType<BadRequestError>(error);
+        Assert.Equal(42, (int)error.Code);
+        Assert.Equal(400, error.HttpStatusCode);
+    }
+
     private class SomeException : OperationCanceledException { }
 
     [Fact]

test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs

@@ -47,6 +47,13 @@ public void Should_return_bad_gateway_error(OcelotErrorCode errorCode)
         ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.BadGateway);
     }
 
+    [Theory]
+    [InlineData(OcelotErrorCode.BadRequestError)]
+    public void Should_return_bad_request(OcelotErrorCode errorCode)
+    {
+        ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.BadRequest);
+    }
+
     [Theory]
     [InlineData(OcelotErrorCode.CannotAddDataError)]
     [InlineData(OcelotErrorCode.CannotFindDataError)]
@@ -73,15 +80,15 @@ public void Should_return_bad_gateway_error(OcelotErrorCode errorCode)
     public void Should_return_not_found(OcelotErrorCode errorCode)
     {
         ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.NotFound);
-    }
+    }
 
     [Fact]
     [Trait("Bug", "749")] // https://github.com/ThreeMammals/Ocelot/issues/749
-    [Trait("PR", "1769")] // https://github.com/ThreeMammals/Ocelot/pull/1769
-    public void Should_return_request_entity_too_large()
-    {
-        ShouldMapErrorsToStatusCode(new() { OcelotErrorCode.PayloadTooLargeError }, HttpStatusCode.RequestEntityTooLarge);
-    }
+    [Trait("PR", "1769")] // https://github.com/ThreeMammals/Ocelot/pull/1769
+    public void Should_return_request_entity_too_large()
+    {
+        ShouldMapErrorsToStatusCode(new() { OcelotErrorCode.PayloadTooLargeError }, HttpStatusCode.RequestEntityTooLarge);
+    }
 
     [Fact]
     public void AuthenticationErrorsHaveHighestPriority()
@@ -128,7 +135,7 @@ public void Check_we_have_considered_all_errors_in_these_tests()
         // If this test fails then it's because the number of error codes has changed.
         // You should make the appropriate changes to the test cases here to ensure
         // they cover all the error codes, and then modify this assertion.
-        Enum.GetNames<OcelotErrorCode>().Length.ShouldBe(42, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
+        Enum.GetNames<OcelotErrorCode>().Length.ShouldBe(43, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
     }
 
     private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)