Merge pull request #174 from getsentry/feature/max-request-body-size

feat: MaxRequest for ASP.NET and Core

Author: bruno-garcia

Directory.Build.props

@@ -23,4 +23,8 @@
     <FrameworkPathOverride Condition="'$(BaseFrameworkPathOverrideForMono)' != '' AND '$(TargetFramework)' == 'net461'">$(BaseFrameworkPathOverrideForMono)/4.6.1-api</FrameworkPathOverride>
     <EnableFrameworkPathOverride Condition="'$(BaseFrameworkPathOverrideForMono)' != ''">true</EnableFrameworkPathOverride>
   </PropertyGroup>
+
+  <PropertyGroup Condition="$(TargetFramework.StartsWith('net4'))">
+    <DefineConstants>SYSTEM_WEB;$(AdditionalConstants)</DefineConstants>
+  </PropertyGroup>
 </Project>

src/Sentry.AspNetCore/HttpRequestAdapter.cs

@@ -0,0 +1,23 @@
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using Microsoft.AspNetCore.Http;
+using Sentry.Extensibility;
+
+namespace Sentry.AspNetCore
+{
+    internal class HttpRequestAdapter : IHttpRequest
+    {
+        private readonly HttpRequest _request;
+
+        public HttpRequestAdapter(HttpRequest request) => _request = request;
+
+        public long? ContentLength => _request?.ContentLength;
+        public string ContentType => _request?.ContentType;
+        public Stream Body => _request?.Body;
+
+        public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Form =>
+            _request?.Form.Select(k => new KeyValuePair<string, IEnumerable<string>>(k.Key, k.Value))
+            ?? Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
+    }
+}

src/Sentry.AspNetCore/IRequestPayloadExtractor.cs

@@ -5,6 +5,8 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Net.Http.Headers;
+using Sentry.Extensibility;
+using Sentry.Protocol;
 
 namespace Sentry.AspNetCore
 {
@@ -90,31 +92,24 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt
             }
         }
 
-        private static void SetBody(Scope scope, HttpContext context, SentryAspNetCoreOptions options)
+        private static void SetBody(BaseScope scope, HttpContext context, SentryAspNetCoreOptions options)
         {
-            if (options?.IncludeRequestPayload == true)
+            if (context == null || scope == null || options == null)
             {
-                // GetServices<IRequestPayloadExtractor> throws! Shouldn't it return Enumerable.Empty<T>?
-                var extractors = context.RequestServices.GetService<IEnumerable<IRequestPayloadExtractor>>();
-                if (extractors == null)
-                {
-                    return;
-                }
-
-                foreach (var extractor in extractors)
-                {
-                    var data = extractor.ExtractPayload(context.Request);
+                return;
+            }
 
-                    if (data == null
-                        || data is string dataString
-                        && string.IsNullOrEmpty(dataString))
-                    {
-                        continue;
-                    }
+            var extractors = context.RequestServices.GetService<IEnumerable<IRequestPayloadExtractor>>();
+            if (extractors == null)
+            {
+                return;
+            }
+            var dispatcher = new RequestBodyExtractionDispatcher(extractors, options, () => options.MaxRequestBodySize);
 
-                    scope.Request.Data = data;
-                    break;
-                }
+            var body = dispatcher.ExtractPayload(new HttpRequestAdapter(context.Request));
+            if (body != null)
+            {
+                scope.Request.Data = body;
             }
         }
 

src/Sentry.AspNetCore/ScopeExtensions.cs

@@ -1,3 +1,5 @@
+using System;
+using Sentry.Extensibility;
 using Sentry.Extensions.Logging;
 
 namespace Sentry.AspNetCore
@@ -14,7 +16,13 @@ public class SentryAspNetCoreOptions : SentryLoggingOptions
         /// <value>
         ///   <c>true</c> if [the request payload shall be included in events]; otherwise, <c>false</c>.
         /// </value>
-        public bool IncludeRequestPayload { get; set; }
+        [Obsolete("Use MaxRequestBodySize instead.")]
+        public bool IncludeRequestPayload
+        {
+            get => MaxRequestBodySize != RequestSize.None;
+            // As originally there was no truncation, setting to Large.
+            set => MaxRequestBodySize = value ? RequestSize.Always : RequestSize.None;
+        }
 
         /// <summary>
         /// Gets or sets a value indicating whether [include System.Diagnostic.Activity data] to events.
@@ -25,5 +33,14 @@ public class SentryAspNetCoreOptions : SentryLoggingOptions
         /// <see cref="System.Diagnostics.Activity"/>
         /// <seealso href="https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/ActivityUserGuide.md"/>
         public bool IncludeActivityData { get; set; }
+
+        /// <summary>
+        /// Controls the size of the request body to extract if any.
+        /// </summary>
+        /// <remarks>
+        /// No truncation is done by the SDK.
+        /// If the request body is larger than the accepted size, nothing is sent.
+        /// </remarks>
+        public RequestSize MaxRequestBodySize { get; set; }
     }
 }

src/Sentry.AspNetCore/SentryAspNetCoreOptions.cs

@@ -8,6 +8,7 @@
 using Microsoft.AspNetCore.Http.Internal;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Sentry.Extensibility;
 using Sentry.Reflection;
 
 namespace Sentry.AspNetCore
@@ -79,7 +80,7 @@ public async Task InvokeAsync(HttpContext context)
 
             using (hub.PushAndLockScope())
             {
-                if (_options?.IncludeRequestPayload == true)
+                if (_options != null && _options.MaxRequestBodySize != RequestSize.None)
                 {
                     context.Request.EnableRewind();
                 }

src/Sentry.AspNetCore/SentryMiddleware.cs

@@ -1,10 +1,8 @@
-using Microsoft.AspNetCore.Http;
-
-namespace Sentry.AspNetCore
+namespace Sentry.Extensibility
 {
     public abstract class BaseRequestPayloadExtractor : IRequestPayloadExtractor
     {
-        public object ExtractPayload(HttpRequest request)
+        public object ExtractPayload(IHttpRequest request)
         {
             if (!request.Body.CanSeek
                 || !request.Body.CanRead
@@ -26,8 +24,8 @@ public object ExtractPayload(HttpRequest request)
             }
         }
 
-        protected abstract bool IsSupported(HttpRequest request);
+        protected abstract bool IsSupported(IHttpRequest request);
 
-        protected abstract object DoExtractPayLoad(HttpRequest request);
+        protected abstract object DoExtractPayLoad(IHttpRequest request);
     }
 }

src/Sentry.AspNetCore/BaseRequestPayloadExtractor.cs renamed to src/Sentry/Extensibility/BaseRequestPayloadExtractor.cs

@@ -1,14 +1,13 @@
 using System.IO;
 using System.Text;
-using Microsoft.AspNetCore.Http;
 
-namespace Sentry.AspNetCore
+namespace Sentry.Extensibility
 {
     public class DefaultRequestPayloadExtractor : BaseRequestPayloadExtractor
     {
-        protected override bool IsSupported(HttpRequest request) => true;
+        protected override bool IsSupported(IHttpRequest request) => true;
 
-        protected override object DoExtractPayLoad(HttpRequest request)
+        protected override object DoExtractPayLoad(IHttpRequest request)
         {
             // https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/IO/StreamReader.cs#L186
             // Default parameters other than 'leaveOpen'

src/Sentry.AspNetCore/DefaultRequestPayloadExtractor.cs renamed to src/Sentry/Extensibility/DefaultRequestPayloadExtractor.cs

@@ -17,7 +17,7 @@ public static void LogDebug<TArg, TArg2>(
             string message,
             TArg arg,
             TArg2 arg2)
-        => logger.LogIfEnabled(SentryLevel.Debug, message, arg, arg2);
+            => logger.LogIfEnabled(SentryLevel.Debug, message, arg, arg2);
 
         public static void LogDebug(
             this IDiagnosticLogger logger,
@@ -40,7 +40,7 @@ public static void LogInfo<TArg, TArg2>(
             string message,
             TArg arg,
             TArg2 arg2)
-        => logger.LogIfEnabled(SentryLevel.Info, message, arg, arg2);
+            => logger.LogIfEnabled(SentryLevel.Info, message, arg, arg2);
 
         public static void LogWarning(
             this IDiagnosticLogger logger,
@@ -53,6 +53,13 @@ public static void LogWarning<TArg>(
             TArg arg)
             => logger.LogIfEnabled(SentryLevel.Warning, message, arg);
 
+        public static void LogWarning<TArg, TArg2>(
+            this IDiagnosticLogger logger,
+            string message,
+            TArg arg,
+            TArg2 arg2)
+            => logger.LogIfEnabled(SentryLevel.Warning, message, arg, arg2);
+
         public static void LogError(
             this IDiagnosticLogger logger,
             string message,

src/Sentry/Extensibility/DiagnosticLoggerExtensions.cs

@@ -1,18 +1,17 @@
 using System;
 using System.Linq;
-using Microsoft.AspNetCore.Http;
 
-namespace Sentry.AspNetCore
+namespace Sentry.Extensibility
 {
     public class FormRequestPayloadExtractor : BaseRequestPayloadExtractor
     {
         private const string SupportedContentType = "application/x-www-form-urlencoded";
 
-        protected override bool IsSupported(HttpRequest request)
+        protected override bool IsSupported(IHttpRequest request)
             => SupportedContentType
                 .Equals(request.ContentType, StringComparison.InvariantCulture);
 
-        protected override object DoExtractPayLoad(HttpRequest request)
+        protected override object DoExtractPayLoad(IHttpRequest request)
             => request.Form?.ToDictionary(k => k.Key, v => v.Value);
     }
 }