The `NegotiationResponse` object is incorrectly deserialized with the `NegotiateProtocol.ParseResponse` API

[IEvangelist]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

The NegotiationResponse object is incorrectly deserialized with the NegotiateProtocol.ParseResponse API. The default behavior for the NegotiationResponse type assumes its serialized in a specific way, where properties are ignored if they're null, this is not the default behavior, however; of the System.Text.Json serializer for both JsonSerializerDefaults.Web or JsonSeriaizerDefaults.Default. For example, when a response is serialized as follows:

var response = new NegotiationResponse
{
    Url = "https://example.com/signalr",
    AccessToken = "fake-access-token"
};

var json = JsonSerializer.Serialize(
    response,
    new JsonSerializerOptions(JsonSerializerDefaults.Web));

The resulting JSON is as follows:

{"url":"https://example.com/signalr","accessToken":"fake-access-token","connectionId":null,"availableTransports":null}

When calling NegotiateProtocol.ParseResponse from this JSON, theere's an exception thrown that is both misleading and inaccurate. It results in the following exception:

System.IO.InvalidDataException : Invalid negotiation response received.
    ---- System.IO.InvalidDataException : Expected 'connectionId' to be of type String.

While the connectionId is in fact null, there is a url—which should be the only thing required when deserializing the response, per:

aspnetcore/src/SignalR/common/Http.Connections.Common/src/NegotiateProtocol.cs

Line 233 in f355564

if (url == null && error == null)

Additionally, I've confirmed that this bug doesn't exist in the SignalR JavaScript client. See https://github.com/dotnet/aspire/blob/main/playground/signalr/SignalRServerlessWeb/Pages/Index.cshtml.

Expected Behavior

I would expect that the NegotiateProtocol.ParseResponse would either successfully deserialize the NegotiationResponse object, even if there are null properties.

Ideally, client code shouldn't have to know to explicit ignore null properties when deserializing this type. This seems related to Azure/azure-functions-dotnet-worker#2213.

Steps To Reproduce

Here's a sample xUnit project the demonstrates two tests, one that fails and one that passes, I'd expect both of these tests to pass:

Project.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net9.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="coverlet.collector" Version="6.0.2" />
    <PackageReference Include="Microsoft.AspNetCore.Http.Connections" Version="1.2.0" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
    <PackageReference Include="xunit" Version="2.9.2" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
  </ItemGroup>

  <ItemGroup>
    <Using Include="Xunit" />
  </ItemGroup>

</Project>

UnitTest1.cs_:

using Microsoft.AspNetCore.Http.Connections;
using System.Text.Json;
using System.Text.Json.Serialization;
using Xunit.Abstractions;

namespace NegotiateResponse.Tests;

public class UnitTest1(ITestOutputHelper output)
{
    private static readonly JsonSerializerOptions s_webJsonOptions = new(JsonSerializerDefaults.Web);
    private static readonly JsonSerializerOptions s_webJsonOptionsWithIgnoreNull = new(JsonSerializerDefaults.Web)
    {
        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
    };

    [Fact]
    public void RoundTripFailsWithMisleadingAndInaccurateError()
    {
        var newResponse = new NegotiationResponse
        {
            Url = "https://example.com/signalr",
            AccessToken = "fake-access-token"
        };

        var json = JsonSerializer.Serialize(newResponse, s_webJsonOptions);

        output.WriteLine($"Response JSON: {json}");

        using var memoryStream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(json));

        /* This is a bug, the NegotiateProtocol.ParseResponse method should
           not throw an exception when given valid JSON.
        
         {"url":"https://example.com/signalr","accessToken":"fake-access-token","connectionId":null,"availableTransports":null}

           The NegotiateProtocol.ParseResponse method should be able to
           parse the JSON response correctly, but it throws an exception ->

           System.IO.InvalidDataException : Invalid negotiation response received.
               ---- System.IO.InvalidDataException : Expected 'connectionId' to be of type String.

           The connectionId is only actually needed when the `url` is `null`, but the `url` isn't `null`.
           Instead, the ParseResponse method incorrectly parses the JSON and throws an exception.
         */
        NegotiationResponse parsedResponse = NegotiateProtocol.ParseResponse(memoryStream);

        Assert.NotNull(parsedResponse);
        Assert.Equal(newResponse.Url, parsedResponse.Url);
        Assert.Equal(newResponse.AccessToken, parsedResponse.AccessToken);
    }

    [Fact]
    public void RoundTripSucceedsWithExplicitAndUnknownSerializationRules()
    {
        var newResponse = new NegotiationResponse
        {
            Url = "https://example.com/signalr",
            AccessToken = "fake-access-token"
        };

        var json = JsonSerializer.Serialize(newResponse, s_webJsonOptionsWithIgnoreNull);

        output.WriteLine($"Response JSON: {json}");

        using var memoryStream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(json));

        var parsedResponse = NegotiateProtocol.ParseResponse(memoryStream);

        Assert.NotNull(parsedResponse);
        Assert.Equal(newResponse.Url, parsedResponse.Url);
        Assert.Equal(newResponse.AccessToken, parsedResponse.AccessToken);
    }
}

Exceptions (if any)

System.IO.InvalidDataException : Invalid negotiation response received.
    ---- System.IO.InvalidDataException : Expected 'connectionId' to be of type String.

.NET Version

9.0.200

Anything else?

.NET SDK:
 Version:           9.0.200
 Commit:            90e8b202f2
 Workload version:  9.0.200-manifests.34e45f8a
 MSBuild version:   17.13.8+cbc39bea8

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.26100
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\9.0.200\

.NET workloads installed:
 [aspire]
   Installation Source: VS 17.13.35806.99, VS 17.14.35806.103
   Manifest Version:    8.2.2/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.2.2\WorkloadManifest.json
   Install Type:              Msi

Configured to use loose manifests when installing new manifests.

Host:
  Version:      9.0.3
  Architecture: x64
  Commit:       831d23e561

.NET SDKs installed:
  8.0.310 [C:\Program Files\dotnet\sdk]
  9.0.104 [C:\Program Files\dotnet\sdk]
  9.0.200 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 8.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 9.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 9.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download