System.Security.Cryptography fails on WASM #25285
Conversation
BrennanConroy
added
the
tell-mode
| _clientCertificates = new X509CertificateCollection(); | ||
| } | ||
| // System.Security.Cryptography isn't supported on WASM currently | ||
| catch { } |
There was a problem hiding this comment.
Catch PlatformNotSupportedException explicitly?
|
I approve the scenario for RC, pending other reviewers signing off, and CI passing. |
src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnectionOptions.cs
Show resolved
Hide resolved
| _clientCertificates = new X509CertificateCollection(); | ||
|
|
||
| // System.Security.Cryptography isn't supported on WASM currently | ||
| if (!Utils.IsRunningInBrowser()) |
There was a problem hiding this comment.
If you wanted to be extra nice, you could decorate the ClientCertificates property with [UnsupportedOSPlatform("browser")]
There was a problem hiding this comment.
I'm not familiar with that. Does it tell an analyzer that you shouldn't use that property? Will it complain at us using it internally for null checks?
There was a problem hiding this comment.
It'll warn if you use that property in a blazor application. Right now SignalR client doesn't claim to support browser so you wouldn't see the error internally. But it would print a warning in the null check once if you say the client targets browser.
There was a problem hiding this comment.
SignalR client doesn't claim to support browser
Is this something we should look into in 6.0?
There was a problem hiding this comment.
It doesn't look like we have [SupportedOSPlatform("browser")] anywhere yet in this repo or in the runtime repo. I see [UnsupportedOSPlatform("browser")] in a couple places in both repos though.
Why would SignalR need to explicitly claim to support the "browser" platform? If it's opt-in, why put [UnsupportedOSPlatform("browser")] all over?
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Ping reviewers |
Fixes #25259