Repository navigation
Add throwing PNSE to System.Security.Cryptography on WASM - #38379
Conversation
|
@MaximLipnin Since the set of assemblies are going to be PNSE, you can use something like this: |
|
@steveisok I just noticed that using the PNSE generator would also generate it for RandomNumberGenerator which we've identified is used in a bunch of places. Either we fix those to not rely on it, or we need to do manual PNSE. |
|
Could |
|
ah, good point. @MaximLipnin I don't know. When I looked earlier it seemed all or nothing. |
b7fda98 to
244e9d2
Compare
… assembly and exclude random number generator which works on WASM
244e9d2 to
423be15
Compare
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetsBrowser)' == 'true'">SR.Cryptography_PlatformNotSupported_Browser</GeneratePlatformNotSupportedAssemblyMessage> |
There was a problem hiding this comment.
The two OS functions that this library uses are:
- Tell me what the friendly name of this OID is (Windows, Linux)
- Pretty print this identified item (Windows, Linux)
macOS has no OS fallbacks, it just uses managed implementations with limited string support; so Browser could just build with the "OSX" flavor.
That said, there's not a whole lot I'd expect anyone to do with this assembly without the rest of crypto, so maybe PNSE is best for now.
There was a problem hiding this comment.
I think PNSE is better for now.
| <AssemblyVersion Condition="'$(TargetFramework)' == 'netstandard2.0'">4.1.0.0</AssemblyVersion> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(TargetsUnix)' != 'true'"> | ||
| <PropertyGroup Condition="'$(TargetsUnix)' != 'true' or '$(TargetsBrowser)' == 'true'"> |
There was a problem hiding this comment.
Does the browser target claim to be Unix? If not, the or seems redundant.
There was a problem hiding this comment.
It does right now, but that will be changed with #37944. For now this is needed.
bartonjs
left a comment
There was a problem hiding this comment.
Assuming TargetsBrowser does not imply TargetsUnix, it'd be good to get the redundant "or" cleaned up, otherwise LGTM.
| <AssemblyVersion Condition="'$(TargetFramework)' == 'netstandard2.0'">4.1.0.0</AssemblyVersion> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(TargetsUnix)' != 'true'"> | ||
| <PropertyGroup Condition="'$(TargetsUnix)' != 'true' or '$(TargetsBrowser)' == 'true'"> |
There was a problem hiding this comment.
It does right now, but that will be changed with #37944. For now this is needed.
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetsBrowser)' == 'true'">SR.Cryptography_PlatformNotSupported_Browser</GeneratePlatformNotSupportedAssemblyMessage> |
There was a problem hiding this comment.
I think PNSE is better for now.
Since the entire set of System.Security.Cryptography.* tests has been disabled on WASM recently in #37723, we need to let everyone know that those areas are not quite supported on WASM at the moment. In that case System.Security.Cryptography classes could throw PlatformNotSupportedException until some solution comes (#37669)
cc @steveisok