Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[ASM] Catch HttpException when reading the request body in .net461 (#…
…6459) ## 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. -->
- Loading branch information