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

[Majdi-Zlitni]

Fixes #2376

• Matched requests that contain invalid header values (eg with emoji) return a 502 to the client #2376

Discussed in #2375

• Matched requests that contain invalid header values (eg with emoji) return a `502` to the client #2375

Changes

• Created a new BadRequestError mapped to HTTP 400 (Bad Request).
• Updated HttpExceptionToErrorMapper to specifically catch HttpRequestException instances complaining about "only ASCII characters".
• Mapped this specific exception to BadRequestError instead of falling back to the generic ConnectionToDownstreamServiceError (HTTP 502).

---

Why it was done

When a client sends a request containing invalid header characters (for example an emoji 💀), Ocelot forwards the request as-is.
While building the outbound request, the underlying .NET HttpClient throws the following exception:

Previously, Ocelot's error handler mapped this exception to HTTP 502 Bad Gateway, which incorrectly implies a failure in the downstream service.

By intercepting this specific exception (Late Catch), the gateway correctly identifies it as a malformed client request and returns the industry-standard HTTP 400 Bad Request instead.

---

Architectural note

The solution uses a Late Catch strategy in the exception mapper rather than introducing early validation middleware.

This avoids the CPU overhead of iterating through and validating every incoming request header, preserving Ocelot’s philosophy of acting as a transparent, high-performance proxy.

[raman-m]

@Majdi-Zlitni, please work on the suggestions.
This is the first round of pair programming.

[raman-m]

Unit tests have failed → build 543

Did you run the tests locally? Please pay attention to the tests, since submitting code with failed tests turns the pull request into draft status.

[Majdi-Zlitni]

I will run the tests locally and check

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.75%. Comparing base (5eeb9cc) to head (43f6aee).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2379      +/-   ##
===========================================
+ Coverage    91.55%   91.75%   +0.19%     
===========================================
  Files          296      297       +1     
  Lines         5876     5882       +6     
  Branches       799      801       +2     
===========================================
+ Hits          5380     5397      +17     
+ Misses         385      379       -6     
+ Partials       111      106       -5     

Flag	Coverage Δ
unittests	91.75% <100.00%> (+0.19%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/Ocelot/Requester/BadRequestError.cs	100.00% <100.00%> (ø)
src/Ocelot/Requester/HttpExceptionToErrorMapper.cs	100.00% <100.00%> (ø)
...c/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs	85.18% <100.00%> (+1.18%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coveralls]

coverage: 92.618% (+0.09%) from 92.525% — Majdi-Zlitni:fix-invalid-header-502 into ThreeMammals:develop

[raman-m]

Majdi, thanks for being proactive. 💪

The next step is writing an acceptance test for the linked bug #2376. We must add at least one test to be able to close the bug officially.
I will re-check the previous code review and re-think the current bug fix, which looks simple, but the upcoming acceptance testing will uncover the next steps for this PR.

[Majdi-Zlitni]

Thank you @raman-m, I’ve added the acceptance test for #2376 to cover the bug end to end, so the fix is now backed by regression coverage.

[raman-m]

I've changed the priorities, and this PR has been included in the current release as a patch. Thus, the patch will be a part of .NET 10 release. I'm going to perform a final code review...

[Majdi-Zlitni]

Thanks @raman-m for the update and for including this in the release. I really appreciate the opportunity to contribute and learn. Looking forward to your final review.

[raman-m]

This is the second round, and my suggestions are below.
I will push a commit tomorrow with my review comments, if not fixed by you.

My current concern is that the acceptance test requires refactoring to use helpers from the Ocelot.Testing namespace. The test should be shorter and easier to read.
After that, we will be able to verify the bug fix, continue debugging, and find an optimal solution.

P.S. AI helps, but it sometimes produces redundant and unusual code that doesn’t match the current style and libraries.

[raman-m]

It’s hard to judge this solution until a valid acceptance test has been written.

I believe we can improve this area, since the Message.Contains method handles a specific user scenario. But what about codes, internal exceptions, and so on? We should inspect the generated exception and reuse its state rather than relying only on messages.

We might also consider covering more scenarios that return a BadRequestError response.
Given the purpose of BadRequestError, it should cover all exceptions that result in a 400 Bad Request status. So, I'd expect more if-else block(s).
You (we) might be inspired by this #2375 (reply in thread).

[Majdi-Zlitni]

Appreciate the guidance

[Majdi-Zlitni]

Ocelot is really cool 😎 but found it a little hard for me, I really enjoy learning from it and hope to keep on learning, also I need to improve my AI gen code review to contribute more effectively.
Thanks @raman-m for your patience🙏

[raman-m]

@Majdi-Zlitni Please stop pushing commits! You've done a lot from your side. Thank you!
I will do the rest of the work.

[raman-m]

Yeah, that is funny.
This life hack — which catches the HttpClient exception early by adjusting the response to return a 400 status — actually triggers the exception too early. Here is the call stack it produces:

   at System.Net.Http.HttpConnection.<WriteString>g__ThrowForInvalidCharEncoding|55_0()
   at System.Net.Http.HttpConnection.WriteString(String s, Encoding encoding)
   at System.Net.Http.HttpConnection.WriteHeaderCollection(HttpHeaders headers, String cookiesFromContainer)
   at System.Net.Http.HttpConnection.WriteHeaders(HttpRequestMessage request)
   at System.Net.Http.HttpConnection.<SendAsync>d__56.MoveNext()
   at System.Net.Http.HttpConnection.<SendAsync>d__56.MoveNext()
   at System.Net.Http.HttpConnectionPool.<SendWithVersionDetectionAndRetryAsync>d__52.MoveNext()
   at System.Net.Http.DiagnosticsHandler.<SendAsyncCore>d__10.MoveNext()
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult()
   at System.Net.Http.RedirectHandler.<SendAsync>d__4.MoveNext()
   at System.Net.Http.SocketsHttpHandler.<<SendAsync>g__CreateHandlerAndSendAsync|115_0>d.MoveNext()
   at System.Net.Http.HttpClient.<<SendAsync>g__Core|83_0>d.MoveNext()
   at Ocelot.Testing.AcceptanceSteps.<WhenIGetUrlOnTheApiGateway>d__86.MoveNext() in D:\github\Ocelot\Majdi-Zlitni\Ocelot\testing\AcceptanceSteps.cs:line 368
   at Ocelot.AcceptanceTests.Requester.InvalidHeaderValueTests.<WhenIGetUrlOnTheApiGatewaySafely>d__1.MoveNext() in D:\github\Ocelot\Majdi-Zlitni\Ocelot\test\Ocelot.AcceptanceTests\Requester\InvalidHeaderValueTests.cs:line 34

However, the original bug #2374 has a different call stack:

         at System.Net.Http.HttpConnection.<WriteString>g__ThrowForInvalidCharEncoding|55_0()
         at System.Net.Http.HttpConnection.WriteString(String s, Encoding encoding)
         at System.Net.Http.HttpConnection.WriteHeaderCollection(HttpHeaders headers, String cookiesFromContainer)
         at System.Net.Http.HttpConnection.WriteHeaders(HttpRequestMessage request)
         at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
         at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
         at Ocelot.Requester.TimeoutDelegatingHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /home/laura/scratch_pad/Ocelot/src/Ocelot/Requester/TimeoutDelegatingHandler.cs:line 30
         at Ocelot.Requester.MessageInvokerHttpRequester.GetResponse(HttpContext httpContext) in /home/laura/scratch_pad/Ocelot/src/Ocelot/Requester/MessageInvokerHttpRequester.cs:line 29

As a result, we are reproducing the same bug on the HttpClient side, and the actual request never reaches the Ocelot side. This happened because the acceptance test was changed when I asked to use our testing helpers.

I was wrong — the original test version actually used the correct approach to emulate a curl request, based on TcpClient streams.

Tomorrow I will rewrite the test based on the original version. The AI coding agent was right when it suggested how to properly emulate curl requests in .NET apps.
In the end, we must use plain TcpClient streams only if we want to reliably reproduce the bug.
This is my fault 🙃