From cc4b593d94887cd64dbd99ce9792b815eb449e8d Mon Sep 17 00:00:00 2001
From: NachoEchevarria <53266532+NachoEchevarria@users.noreply.github.com>
Date: Thu, 16 Jan 2025 13:00:25 +0100
Subject: [PATCH] [ASM] Catch HttpException when reading the request body in
 .net461 (#6459)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary of changes

In .net461, we are getting an uncatched exception in two customers. The
exception is System.Web.HttpException --->
System.Runtime.InteropServices.COMException

The stacktrace is:
```
at System.Web.Hosting.IIS7WorkerRequest.RaiseCommunicationError(Int32 result, Boolean throwOnDisconnect)
at System.Web.Hosting.IIS7WorkerRequest.ReadEntityCoreSync(Byte[] buffer, Int32 offset, Int32 size)
at System.Web.HttpRequest.GetEntireRawContent()
at System.Web.HttpRequest.FillInFormCollection()
at System.Web.HttpRequest.EnsureForm()
at System.Web.HttpRequest.get_Form()
at Datadog.Trace.Util.RequestDataHelper.GetForm(HttpRequest request)
at Datadog.Trace.AppSec.Coordinator.SecurityCoordinator.GetBodyFromRequest()
at Datadog.Trace.AspNet.TracingHttpModule.OnBeginRequest(Object sender, EventArgs eventArgs)
```

If we take a look at the code of the system.Web assembly, we can see
that this error is originated in these methods:

```
private int ReadEntityCoreSync(byte[] buffer, int offset, int size) {
            bool fAsync = false;
            int bytesRead = 0;
            IntPtr ppAsyncReceiveBuffer;
            int result = 0;

            try {
                // Acquire blocking call
                IsInReadEntitySync = true;

                result = IIS.MgdReadEntityBody(_context, buffer, offset, size, fAsync, out bytesRead,out ppAsyncReceiveBuffer);
            }
            finally {
                // Release the blocking call
                IsInReadEntitySync = false;
            }

            // DevDiv Bugs 177323: mimic classic mode and don't throw if the client disconnected
            if (result < 0) {
                RaiseCommunicationError(result, false /*throwOnDisconnect*/);
            }

            if (bytesRead > 0) {
                PerfCounters.IncrementCounterEx(AppPerfCounter.REQUEST_BYTES_IN, bytesRead);
            }

            return bytesRead;
        }

        private void RaiseCommunicationError(int result, bool throwOnDisconnect) {
            if (IIS.MgdIsClientConnected(_context)) {
                throw new HttpException(SR.GetString(SR.Server_Support_Function_Error, result.ToString("X8", CultureInfo.InvariantCulture)), Marshal.GetExceptionForHR(result));
            }
            else {
                //give different error if connection was closed
                IncrementRequestsDisconnected();
                if (throwOnDisconnect) {
                    throw new HttpException(SR.GetString(SR.Server_Support_Function_Error_Disconnect, result.ToString("X8", CultureInfo.InvariantCulture)), result);
                }
            }
        }
```

Basically, the method IIS.MgdReadEntityBody returns an error that is
sent to the method RaiseCommunicationError.

Since the method RaiseCommunicationError does not launch exceptions when
the connection is closed because throwOnDisconnect is false, it is not
related to that.

The method belongs to the native library webengine4.dll

```
    [DllImport("webengine4.dll", CharSet = CharSet.Unicode)]
    internal static extern int MgdReadEntityBody(
      IntPtr pHandler,
      byte[] pBuffer,
      int dwOffset,
      int dwBytesToRead,
      bool fAsync,
      out int pBytesRead,
      out IntPtr ppAsyncReceiveBuffer);
```

Other people are experiencing it:

https://stackoverflow.com/questions/7825127/system-runtime-interopservices-comexception-incorrect-function-exception-from

https://stackoverflow.com/questions/30854972/response-isclientconnected-causes-exceptions

https://stackoverflow.com/questions/8299887/asp-net-4-0-web-app-throwing-incorrect-function-exception-from-hresult-0x800

Some people suggest that this error might be caused by network card
configuration problems. In any case, this is an error that might
occasionally happen when reading the body of a request and does not seem
to be related to the tracer's activity. In order to avoid this exception
to be thrown, we should change the scope of the caught exceptions in the
request helper method to include the System.Web.HttpExceptions and not
only the ValidationExceptions

## Reason for change

## Implementation details

## Test coverage

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
---
 tracer/src/Datadog.Trace/Util/RequestDataHelper.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tracer/src/Datadog.Trace/Util/RequestDataHelper.cs b/tracer/src/Datadog.Trace/Util/RequestDataHelper.cs
index d80bc34b3367..95c36499ef52 100644
--- a/tracer/src/Datadog.Trace/Util/RequestDataHelper.cs
+++ b/tracer/src/Datadog.Trace/Util/RequestDataHelper.cs
@@ -77,7 +77,7 @@ internal static QueryString GetQueryString(HttpRequest request)
         {
             return request.Form;
         }
-        catch (HttpRequestValidationException)
+        catch (HttpException)
         {
             Log.Debug("Error reading Form (body) from the request.");
             return null;