Fix null reference exception for Streaming null object

[Kahbazi]

Fixes #13191

[BrennanConroy]

Thanks for the PR @Kahbazi!

Unfortunately I think the fix is a little more complicated than this.

Now if someone passes null as an argument they'll fall through to the if statement that says // This should never happen, we instead would want the argument to be set on the arguments array.

Also, we should add a test to verify that the scenario is fixed and that we don't regress it in the future! The test would likely be similar to https://github.com/aspnet/AspNetCore/blob/7b0b2980dc6ffb860f2bcd71868d98d85c7d35b6/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs#L3524

[Kahbazi]

@BrennanConroy If it's possible I'd like to give it another try to fix this.
I have couple of question

• This was the original javascript which causes the exception

this.hubConnection.stream("LogIn", username, passwordHash, location.origin, null).subscribe(...)

and the workaround was to put an empty string instead of null

this.hubConnection.stream("LogIn", username, passwordHash, location.origin, "").subscribe(...)

Shouldn't the correct way to use javascript client to just omit the last parameter?

this.hubConnection.stream("LogIn", username, passwordHash, location.origin).subscribe(...)

• If I'm not wrong the current code has a bug whether the null is sent for cancellation token or any parameter so What should happen when there's a null argument for types like int, bool, DateTime? Should I put the default value on the arguments array or throw an exception?

• What should happen if the number of arguments coming from client doesn't match the number on the server?

[BrennanConroy]

If it's possible I'd like to give it another try to fix this.

Of course! I was just giving suggestions so that we can approve and merge this.

Shouldn't the correct way to use javascript client to just omit the last parameter?

No, in this case their method was public ChannelReader<ApiTypedData<LogInResponseType>> LogIn(string username, byte[] passwordHash, string baseUrl, string fastLoginPassword, CancellationToken cancellationToken) so they want to pass 4 arguments (userName, passwordHash, location.origin, and null).

What should happen when there's a null argument for types like int, bool, DateTime? Should I put the default value on the arguments array or throw an exception?

That should be handled by the underlying protocol.

What should happen if the number of arguments coming from client doesn't match the number on the server?

Also handled by the underlying protocol.

[Kahbazi]

No, in this case their method was public ChannelReader<ApiTypedData> LogIn(string username, byte[] passwordHash, string baseUrl, string fastLoginPassword, CancellationToken cancellationToken) so they want to pass 4 arguments (userName, passwordHash, location.origin, and null).

Oh my bad, I was miscounting and thought the last parameter (null) was for cancellation token. Everything makes much more sense now.

[BrennanConroy]

Thanks for the PR!

I want to give this a quick local test before merging and give @halter73 a chance to take a look if he'd like.

[BrennanConroy]

nit: remove this extra line