fix(request-info): avoid reading handled post data

niemyjski · niemyjski · commit c1193d9276c7 · 2026-03-24T23:14:17.000-05:00
diff --git a/samples/Exceptionless.SampleAspNetCore/Controllers/ValuesController.cs b/samples/Exceptionless.SampleAspNetCore/Controllers/ValuesController.cs
@@ -26,14 +26,16 @@ public Dictionary<string, string> Get() {
 
             try {
                 throw new Exception($"Handled Exception: {Guid.NewGuid()}");
-            } catch (Exception handledException) {
+            }
+            catch (Exception handledException) {
                 // Use the ToExceptionless extension method to submit this handled exception to Exceptionless using the client instance from DI.
                 handledException.ToExceptionless(_exceptionlessClient).Submit();
             }
 
             try {
                 throw new Exception($"Handled Exception (Default Client): {Guid.NewGuid()}");
-            } catch (Exception handledException) {
+            }
+            catch (Exception handledException) {
                 // Use the ToExceptionless extension method to submit this handled exception to Exceptionless using the default client instance (ExceptionlessClient.Default).
                 // This works and is convenient, but its generally not recommended to use static singleton instances because it makes testing and
                 // other things harder.
@@ -44,4 +46,4 @@ public Dictionary<string, string> Get() {
             throw new Exception($"Unhandled Exception: {Guid.NewGuid()}");
         }
     }
-}
+}
diff --git a/src/Platforms/Exceptionless.AspNetCore/ExceptionlessAspNetCorePlugin.cs b/src/Platforms/Exceptionless.AspNetCore/ExceptionlessAspNetCorePlugin.cs
@@ -32,7 +32,7 @@ public void Run(EventPluginContext context) {
                 return;
 
             try {
-                ri = httpContext.GetRequestInfo(context.Client.Configuration);
+                ri = httpContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);
             } catch (Exception ex) {
                 context.Log.Error(typeof(ExceptionlessAspNetCorePlugin), ex, "Error adding request info.");
             }
diff --git a/src/Platforms/Exceptionless.AspNetCore/ExceptionlessExtensions.cs b/src/Platforms/Exceptionless.AspNetCore/ExceptionlessExtensions.cs
@@ -68,8 +68,9 @@ public static IApplicationBuilder UseExceptionless(this IApplicationBuilder app,
         /// </summary>
         /// <param name="context">The http context to gather information from.</param>
         /// <param name="config">The config.</param>
-        public static RequestInfo GetRequestInfo(this HttpContext context, ExceptionlessConfiguration config) {
-            return RequestInfoCollector.Collect(context, config);
+        /// <param name="isUnhandledError">Whether this is an unhandled error. POST data is only collected for unhandled errors to avoid consuming the request stream.</param>
+        public static RequestInfo GetRequestInfo(this HttpContext context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
+            return RequestInfoCollector.Collect(context, config, isUnhandledError);
         }
 
         /// <summary>
diff --git a/src/Platforms/Exceptionless.AspNetCore/RequestInfoCollector.cs b/src/Platforms/Exceptionless.AspNetCore/RequestInfoCollector.cs
@@ -16,7 +16,7 @@ public static class RequestInfoCollector {
         private const int MAX_BODY_SIZE = 50 * 1024;
         private const int MAX_DATA_ITEM_LENGTH = 1000;
 
-        public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguration config) {
+        public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
             if (context == null)
                 return null;
 
@@ -50,7 +50,9 @@ public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguratio
             if (config.IncludeQueryString)
                 info.QueryString = context.Request.Query.ToDictionary(exclusionList);
 
-            if (config.IncludePostData && !String.Equals(context.Request.Method, "GET", StringComparison.OrdinalIgnoreCase))
+            // Only collect POST data for unhandled errors to avoid consuming the request stream
+            // and breaking model binding for handled errors where the app continues processing.
+            if (config.IncludePostData && isUnhandledError && !String.Equals(context.Request.Method, "GET", StringComparison.OrdinalIgnoreCase))
                 info.PostData = GetPostData(context, config, exclusionList);
 
             return info;
@@ -59,12 +61,7 @@ public static RequestInfo Collect(HttpContext context, ExceptionlessConfiguratio
         private static object GetPostData(HttpContext context, ExceptionlessConfiguration config, string[] exclusionList) {
             var log = config.Resolver.GetLog();
 
-            if (context.Request.HasFormContentType && context.Request.Form.Count > 0) {
-                log.Debug("Reading POST data from Request.Form");
-                return context.Request.Form.ToDictionary(exclusionList);
-            }
-
-            var contentLength = context.Request.ContentLength.GetValueOrDefault();
+            long contentLength = context.Request.ContentLength.GetValueOrDefault();
             if(contentLength == 0) {
                 string message = "Content-length was zero, empty post.";
                 log.Debug(message);
@@ -98,6 +95,11 @@ private static object GetPostData(HttpContext context, ExceptionlessConfiguratio
                     return message;
                 }
 
+                if (context.Request.HasFormContentType && context.Request.Form.Count > 0) {
+                    log.Debug("Reading POST data from Request.Form");
+                    return context.Request.Form.ToDictionary(exclusionList);
+                }
+
                 // pass default values, except for leaveOpen: true. This prevents us from disposing the underlying stream
                 using (var inputStream = new StreamReader(context.Request.Body, Encoding.UTF8, true, 1024, true)) {
                     var sb = new StringBuilder();
diff --git a/src/Platforms/Exceptionless.Web/ExceptionlessWebExtensions.cs b/src/Platforms/Exceptionless.Web/ExceptionlessWebExtensions.cs
@@ -11,23 +11,25 @@ public static class ExceptionlessWebExtensions {
         /// </summary>
         /// <param name="context">The http context to gather information from.</param>
         /// <param name="config">The config.</param>
-        public static RequestInfo GetRequestInfo(this HttpContext context, ExceptionlessConfiguration config) {
+        /// <param name="isUnhandledError">Whether this is an unhandled error. POST data is only collected for unhandled errors to avoid consuming the request stream.</param>
+        public static RequestInfo GetRequestInfo(this HttpContext context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
             if (context == null)
                 return null;
 
-            return GetRequestInfo(context.ToWrapped(), config);
+            return GetRequestInfo(context.ToWrapped(), config, isUnhandledError);
         }
 
         /// <summary>
         /// Adds the current request info.
         /// </summary>
         /// <param name="context">The http context to gather information from.</param>
         /// <param name="config">The config.</param>
-        public static RequestInfo GetRequestInfo(this HttpContextBase context, ExceptionlessConfiguration config) {
+        /// <param name="isUnhandledError">Whether this is an unhandled error. POST data is only collected for unhandled errors to avoid consuming the request stream.</param>
+        public static RequestInfo GetRequestInfo(this HttpContextBase context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
             if (context == null && HttpContext.Current != null)
                 context = HttpContext.Current.ToWrapped();
 
-            return RequestInfoCollector.Collect(context, config);
+            return RequestInfoCollector.Collect(context, config, isUnhandledError);
         }
 
         /// <summary>
diff --git a/src/Platforms/Exceptionless.Web/ExceptionlessWebPlugin.cs b/src/Platforms/Exceptionless.Web/ExceptionlessWebPlugin.cs
@@ -35,7 +35,7 @@ public void Run(EventPluginContext context) {
                 return;
 
             try {
-                ri = httpContext.GetRequestInfo(context.Client.Configuration);
+                ri = httpContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);
             } catch (Exception ex) {
                 context.Log.Error(typeof(ExceptionlessWebPlugin), ex, "Error adding request info.");
             }
diff --git a/src/Platforms/Exceptionless.Web/RequestInfoCollector.cs b/src/Platforms/Exceptionless.Web/RequestInfoCollector.cs
@@ -15,7 +15,7 @@ internal static class RequestInfoCollector {
         private const int MAX_BODY_SIZE = 50 * 1024;
         private const int MAX_DATA_ITEM_LENGTH = 1000;
 
-        public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfiguration config) {
+        public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
             if (context == null)
                 return null;
 
@@ -63,7 +63,9 @@ public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfigur
                 }
             }
 
-            if (config.IncludePostData && !String.Equals(context.Request.HttpMethod, "GET", StringComparison.OrdinalIgnoreCase))
+            // Only collect POST data for unhandled errors to avoid consuming the request stream
+            // and breaking model binding for handled errors where the app continues processing.
+            if (config.IncludePostData && isUnhandledError && !String.Equals(context.Request.HttpMethod, "GET", StringComparison.OrdinalIgnoreCase))
                 info.PostData = GetPostData(context, config, exclusionList);
 
             return info;
@@ -72,13 +74,7 @@ public static RequestInfo Collect(HttpContextBase context, ExceptionlessConfigur
         private static object GetPostData(HttpContextBase context, ExceptionlessConfiguration config, string[] exclusionList) {
             var log = config.Resolver.GetLog();
 
-            if (context.Request.Form.Count > 0) {
-                log.Debug("Reading POST data from Request.Form");
-
-                return context.Request.Form.ToDictionary(exclusionList);
-            }
-
-            var contentLength = context.Request.ContentLength;
+            int contentLength = context.Request.ContentLength;
             if (contentLength == 0) {
                 string message = "Content-length was zero, empty post.";
                 log.Debug(message);
@@ -112,7 +108,13 @@ private static object GetPostData(HttpContextBase context, ExceptionlessConfigur
                     return message;
                 }
 
-                var maxDataToRead = contentLength == 0 ? MAX_BODY_SIZE : contentLength;
+                if (context.Request.Form.Count > 0) {
+                    log.Debug("Reading POST data from Request.Form");
+
+                    return context.Request.Form.ToDictionary(exclusionList);
+                }
+
+                int maxDataToRead = contentLength == 0 ? MAX_BODY_SIZE : contentLength;
 
                 // pass default values, except for leaveOpen: true. This prevents us from disposing the underlying stream
                 using (var inputStream = new StreamReader(context.Request.InputStream, Encoding.UTF8, true, 1024, true)) {
@@ -233,4 +235,4 @@ private static string GetUserIpAddress(HttpContextBase context) {
             return clientIp;
         }
     }
-}
+}
diff --git a/src/Platforms/Exceptionless.WebApi/ExceptionlessWebApiExtensions.cs b/src/Platforms/Exceptionless.WebApi/ExceptionlessWebApiExtensions.cs
@@ -65,8 +65,9 @@ private static void ReplaceHttpErrorHandler(HttpConfiguration config, Exceptionl
         /// </summary>
         /// <param name="context">The http action context to gather information from.</param>
         /// <param name="config">The config.</param>
-        public static RequestInfo GetRequestInfo(this HttpActionContext context, ExceptionlessConfiguration config) {
-            return RequestInfoCollector.Collect(context, config);
+        /// <param name="isUnhandledError">Whether this is an unhandled error. POST data collection is not implemented for WebApi.</param>
+        public static RequestInfo GetRequestInfo(this HttpActionContext context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
+            return RequestInfoCollector.Collect(context, config, isUnhandledError);
         }
 
         /// <summary>
diff --git a/src/Platforms/Exceptionless.WebApi/ExceptionlessWebApiPlugin.cs b/src/Platforms/Exceptionless.WebApi/ExceptionlessWebApiPlugin.cs
@@ -29,7 +29,7 @@ public void Run(EventPluginContext context) {
                 return;
 
             try {
-                ri = actionContext.GetRequestInfo(context.Client.Configuration);
+                ri = actionContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);
             } catch (Exception ex) {
                 context.Log.Error(typeof(ExceptionlessWebApiPlugin), ex, "Error adding request info.");
             }
diff --git a/src/Platforms/Exceptionless.WebApi/RequestInfoCollector.cs b/src/Platforms/Exceptionless.WebApi/RequestInfoCollector.cs
@@ -14,7 +14,7 @@ namespace Exceptionless.ExtendedData {
     internal static class RequestInfoCollector {
         private const int MAX_DATA_ITEM_LENGTH = 1000;
 
-        public static RequestInfo Collect(HttpActionContext context, ExceptionlessConfiguration config) {
+        public static RequestInfo Collect(HttpActionContext context, ExceptionlessConfiguration config, bool isUnhandledError = false) {
             if (context == null)
                 return null;
 
@@ -48,8 +48,11 @@ public static RequestInfo Collect(HttpActionContext context, ExceptionlessConfig
             if (config.IncludeQueryString)
                 info.QueryString = context.Request.RequestUri.ParseQueryString().ToDictionary(exclusionList);
 
+            // POST data collection is not implemented for WebApi due to async complexities and
+            // the difficulty of reading the request body without interfering with model binding.
+            // Other platforms (AspNetCore, Web) now only collect POST data for unhandled errors.
             // TODO: support getting post data asyncly.
-            //if (config.IncludePostData && context.Request.Method != HttpMethod.Get)
+            //if (config.IncludePostData && isUnhandledError && context.Request.Method != HttpMethod.Get)
             //    info.PostData = GetPostData(context, config, exclusionList);
 
             return info;
diff --git a/test/Exceptionless.Tests/Exceptionless.Tests.csproj b/test/Exceptionless.Tests/Exceptionless.Tests.csproj
@@ -29,10 +29,12 @@
 
   <ItemGroup>
     <ProjectReference Include="..\..\src\Exceptionless\Exceptionless.csproj" />
+    <ProjectReference Include="..\..\src\Platforms\Exceptionless.AspNetCore\Exceptionless.AspNetCore.csproj" />
     <ProjectReference Include="..\Exceptionless.TestHarness\Exceptionless.TestHarness.csproj" />
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.2.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" />
     <PackageReference Include="BenchmarkDotNet" Version="0.13.12" />
     <PackageReference Include="Moq" Version="4.20.72" />
@@ -54,4 +56,4 @@
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
   </ItemGroup>
-</Project>
+</Project>
diff --git a/test/Exceptionless.Tests/Platforms/AspNetCoreRequestInfoTests.cs b/test/Exceptionless.Tests/Platforms/AspNetCoreRequestInfoTests.cs
@@ -0,0 +1,47 @@
+using System.IO;
+using System.Text;
+using Exceptionless;
+using Exceptionless.Dependency;
+using Microsoft.AspNetCore.Http;
+using Xunit;
+
+namespace Exceptionless.Tests.Platforms {
+    public class AspNetCoreRequestInfoTests {
+        [Fact]
+        public void GetRequestInfo_DoesNotReadPostData_ForHandledErrors() {
+            var context = CreateHttpContext("hello=world");
+            var config = new ExceptionlessConfiguration(DependencyResolver.CreateDefault());
+
+            var requestInfo = context.GetRequestInfo(config);
+
+            Assert.NotNull(requestInfo);
+            Assert.Null(requestInfo.PostData);
+            Assert.Equal(0L, context.Request.Body.Position);
+        }
+
+        [Fact]
+        public void GetRequestInfo_ReadsAndRestoresPostData_ForUnhandledErrors() {
+            const string body = "{\"hello\":\"world\"}";
+            var context = CreateHttpContext(body);
+            var config = new ExceptionlessConfiguration(DependencyResolver.CreateDefault());
+
+            context.Request.Body.Position = 5;
+
+            var requestInfo = context.GetRequestInfo(config, isUnhandledError: true);
+
+            Assert.NotNull(requestInfo);
+            Assert.Equal(body, Assert.IsType<string>(requestInfo.PostData));
+            Assert.Equal(5L, context.Request.Body.Position);
+        }
+
+        private static DefaultHttpContext CreateHttpContext(string body) {
+            var bodyBytes = Encoding.UTF8.GetBytes(body);
+            var context = new DefaultHttpContext();
+            context.Request.Method = HttpMethods.Post;
+            context.Request.ContentType = "application/json";
+            context.Request.Body = new MemoryStream(bodyBytes);
+            context.Request.ContentLength = bodyBytes.Length;
+            return context;
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public void Run(EventPluginContext context) {`
`32`	`32`	`return;`
`33`	`33`
`34`	`34`	`try {`
`35`		`- ri = httpContext.GetRequestInfo(context.Client.Configuration);`
	`35`	`+ ri = httpContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);`
`36`	`36`	`} catch (Exception ex) {`
`37`	`37`	`context.Log.Error(typeof(ExceptionlessAspNetCorePlugin), ex, "Error adding request info.");`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public void Run(EventPluginContext context) {`
`35`	`35`	`return;`
`36`	`36`
`37`	`37`	`try {`
`38`		`- ri = httpContext.GetRequestInfo(context.Client.Configuration);`
	`38`	`+ ri = httpContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);`
`39`	`39`	`} catch (Exception ex) {`
`40`	`40`	`context.Log.Error(typeof(ExceptionlessWebPlugin), ex, "Error adding request info.");`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public void Run(EventPluginContext context) {`
`29`	`29`	`return;`
`30`	`30`
`31`	`31`	`try {`
`32`		`- ri = actionContext.GetRequestInfo(context.Client.Configuration);`
	`32`	`+ ri = actionContext.GetRequestInfo(context.Client.Configuration, context.ContextData.IsUnhandledError);`
`33`	`33`	`} catch (Exception ex) {`
`34`	`34`	`context.Log.Error(typeof(ExceptionlessWebApiPlugin), ex, "Error adding request info.");`
`35`	`35`	`}`