Fix: ArgumentNullException in FormRequestPayloadExtractor when handling invalid form data on ASP.NET

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- Fixed ArgumentNullException in FormRequestPayloadExtractor when handling invalid form data on ASP.NET ([#3734](https://github.com/getsentry/sentry-dotnet/pull/3734))
+
 ### Dependencies
 
 - Bump Cocoa SDK from v8.36.0 to v8.39.0 ([#3727](https://github.com/getsentry/sentry-dotnet/pull/3727))

src/Sentry.AspNet/Internal/SystemWebHttpRequest.cs

@@ -1,3 +1,4 @@
+using System.Collections.Specialized;
 using Sentry.Extensibility;
 
 namespace Sentry.AspNet.Internal;
@@ -12,8 +13,19 @@ internal class SystemWebHttpRequest : IHttpRequest
 
     public Stream? Body => _request?.InputStream;
 
-    public IEnumerable<KeyValuePair<string, IEnumerable<string>>>? Form
-        => _request.Form.AllKeys.Select(kv => new KeyValuePair<string, IEnumerable<string>>(kv, _request.Form.GetValues(kv)));
+    public IEnumerable<KeyValuePair<string, IEnumerable<string>>>? Form => GetFormData(_request.Form);
 
     public SystemWebHttpRequest(HttpRequest request) => _request = request;
+
+    internal static IEnumerable<KeyValuePair<string, IEnumerable<string>>> GetFormData(NameValueCollection formdata)
+    {
+        return StripNulls(formdata.AllKeys).Select(key => new KeyValuePair<string, IEnumerable<string>>(
+            key, StripNulls(formdata.GetValues(key)
+            )));
+
+        // Poorly constructed form submissions can result in null keys/values on .NET Framework.
+        // See: https://github.com/getsentry/sentry-dotnet/issues/3701
+        IEnumerable<string> StripNulls(IEnumerable<string>? values) => values?.Where(x => x is not null) ?? [];
+    }
+
 }

src/Sentry/Extensibility/FormRequestPayloadExtractor.cs

@@ -10,9 +10,9 @@ public class FormRequestPayloadExtractor : BaseRequestPayloadExtractor
     /// <summary>
     /// Supports <see cref="IHttpRequest"/> with content type application/x-www-form-urlencoded.
     /// </summary>
-    protected override bool IsSupported(IHttpRequest request)
-        => SupportedContentType
-            .Equals(request.ContentType, StringComparison.InvariantCulture);
+    protected override bool IsSupported(IHttpRequest request) =>
+        SupportedContentType.Equals(request.ContentType, StringComparison.InvariantCulture)
+        || (request.ContentType?.StartsWith($"{SupportedContentType};", StringComparison.InvariantCulture) == true);
 
     /// <summary>
     /// Extracts the request form data as a dictionary.

test/Sentry.AspNet.Tests/Internal/SystemWebHttpRequestTests.cs

@@ -0,0 +1,48 @@
+using System.Collections.Specialized;
+using Sentry.AspNet.Internal;
+
+namespace Sentry.AspNet.Tests.Internal;
+
+public class SystemWebHttpRequestTests
+{
+    [Fact]
+    public void GetFormData_GoodData_ReturnsCorrectValues()
+    {
+        // Arrange
+        var formCollection = new NameValueCollection
+        {
+            { "key1", "value1" },
+            { "key2", "value2" }
+        };
+
+        // Act
+        var form = SystemWebHttpRequest.GetFormData(formCollection).ToDict();
+
+        // Assert
+        form.Should().NotBeNull();
+        form.Should().Contain(kvp => kvp.Key == "key1" && kvp.Value.Contains("value1"));
+        form.Should().Contain(kvp => kvp.Key == "key2" && kvp.Value.Contains("value2"));
+    }
+
+    [Fact]
+    public void GetFormData_BadData_ReturnsCorrectValues()
+    {
+        // Arrange
+        var formCollection = new NameValueCollection
+        {
+            { "key1", "value1" },
+            { "key2", "value2" },
+            { null, "badkey" },
+            { "badvalue", null },
+            { null, null }
+        };
+
+        // Act
+        var form = SystemWebHttpRequest.GetFormData(formCollection).ToDict();
+
+        // Assert
+        form.Should().NotBeNull();
+        form.Should().Contain(kvp => kvp.Key == "key1" && kvp.Value.Contains("value1"));
+        form.Should().Contain(kvp => kvp.Key == "key2" && kvp.Value.Contains("value2"));
+    }
+}

test/Sentry.AspNetCore.Tests/BaseRequestPayloadExtractorTests.cs

@@ -32,6 +32,7 @@ public void ExtractPayload_OriginalStreamPosition_Reset()
     {
         const int originalPosition = 100;
         _ = TestFixture.Stream.Position.Returns(originalPosition);
+        TestFixture.HttpRequestCore.ContentType.Returns(SupportedContentType);
 
         var sut = TestFixture.GetSut();
 
@@ -40,6 +41,8 @@ public void ExtractPayload_OriginalStreamPosition_Reset()
         TestFixture.Stream.Received().Position = originalPosition;
     }
 
+    protected abstract string SupportedContentType { get; }
+
     [Fact]
     public void ExtractPayload_OriginalStream_NotClosed()
     {

test/Sentry.AspNetCore.Tests/DefaultRequestPayloadExtractorTests.cs

@@ -2,6 +2,8 @@ namespace Sentry.AspNetCore.Tests;
 
 public class DefaultRequestPayloadExtractorTests : BaseRequestPayloadExtractorTests<DefaultRequestPayloadExtractor>
 {
+    protected override string SupportedContentType => string.Empty;
+
     [Fact]
     public void ExtractPayload_StringData_ReadCorrectly()
     {

test/Sentry.AspNetCore.Tests/FormRequestPayloadExtractorTests.cs

@@ -5,15 +5,20 @@ namespace Sentry.AspNetCore.Tests;
 
 public class FormRequestPayloadExtractorTests : BaseRequestPayloadExtractorTests<FormRequestPayloadExtractor>
 {
+    protected override string SupportedContentType => "application/x-www-form-urlencoded";
+
     public FormRequestPayloadExtractorTests()
     {
         TestFixture = new Fixture();
-        _ = TestFixture.HttpRequest.ContentType.Returns("application/x-www-form-urlencoded");
     }
 
-    [Fact]
-    public void ExtractPayload_SupportedContentType_ReadForm()
+    [Theory]
+    [InlineData("application/x-www-form-urlencoded")]
+    [InlineData("application/x-www-form-urlencoded; charset=utf-8")]
+    public void ExtractPayload_SupportedContentType_ReadForm(string contentType)
     {
+        TestFixture.HttpRequest.ContentType.Returns(contentType);
+
         var expected = new Dictionary<string, StringValues> { { "key", new StringValues("val") } };
         var f = new FormCollection(expected);
         _ = TestFixture.HttpRequestCore.Form.Returns(f);
@@ -32,7 +37,7 @@ public void ExtractPayload_SupportedContentType_ReadForm()
     [Fact]
     public void ExtractPayload_UnsupportedContentType_DoesNotReadStream()
     {
-        _ = TestFixture.HttpRequest.ContentType.Returns("application/json");
+        TestFixture.HttpRequest.ContentType.Returns("application/json");
 
         var sut = TestFixture.GetSut();