Merge branch 'main' into fix/#3545

Author: bricefriha

.github/workflows/build.yml

@@ -212,7 +212,7 @@ jobs:
           java-version: '17'
 
       - name: Setup Android SDK
-        uses: android-actions/setup-android@00854ea68c109d98c75d956347303bf7c45b0277 # v3.2.1
+        uses: android-actions/setup-android@9fc6c4e9069bf8d3d10b2204b1fb8f6ef7065407 # v3.2.2
 
       - run: dotnet workload install android maui-android
 

CHANGELOG.md

@@ -5,12 +5,13 @@
 ### Fixes
 
 - prevent event duplication and missing attachements when `JavaBackgroundThread` crashes occur ([#3756](https://github.com/getsentry/sentry-dotnet/pull/3756))
+- 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))
-    - [changelog](https://github.com/getsentry/sentry-cocoa/blob/main/CHANGELOG.md#8390)
-    - [diff](https://github.com/getsentry/sentry-cocoa/compare/8.36.0...8.39.0)
+  - [changelog](https://github.com/getsentry/sentry-cocoa/blob/main/CHANGELOG.md#8390)
+  - [diff](https://github.com/getsentry/sentry-cocoa/compare/8.36.0...8.39.0)
 - Bump Native SDK from v0.7.11 to v0.7.12 ([#3731](https://github.com/getsentry/sentry-dotnet/pull/3731))
   - [changelog](https://github.com/getsentry/sentry-native/blob/master/CHANGELOG.md#0712)
   - [diff](https://github.com/getsentry/sentry-native/compare/0.7.11...0.7.12)

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.Bindings.Cocoa/Sentry.Bindings.Cocoa.csproj

@@ -91,6 +91,7 @@
   <!-- Generate bindings -->
   <Target Name="_GenerateSentryCocoaBindings" AfterTargets="SetupCocoaSDK"
           Condition="$([MSBuild]::IsOSPlatform('OSX')) and Exists('$(SentryCocoaFrameworkHeaders)')"
+          Inputs="../../modules/sentry-cocoa.properties"
           Outputs="ApiDefinitions.cs;StructsAndEnums.cs">
     <MSBuild Projects="$(MSBuildProjectFile)" Targets="_InnerGenerateSentryCocoaBindings" Properties="TargetFramework=once" />
   </Target>

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();