Blazor developers get warnings when they call APIs that aren't available on WebAssembly

[jeffhandley]

For #41087, we annotated many APIs with [UnsupportedOSPlatform("browser")], which will allow the Platform Compatibility Analyzer to provide customers with warnings when calling APIs that are unsupported on browser from call sites intended to work on browser. This is working for customer scenarios, but the dotnet/runtime repo itself isn't currently producing warnings for the scenario.

Since support for browser is not implied, the analyzer requires an MSBuild item for <SupportedPlatform Include="browser" /> to be supplied in order for browser-related warnings to be produced. With #41760, we enabled the analyzer in the master branch, but a quick test during the review of that PR revealed that even with this MSBuild item specified, the errors were still not getting produced within the libraries projects.

We need to:

1. Identify which libraries projects should have <SupportedPlatform Include="browser" /> specified, and annotate them as such, indicating that the library is intended to be supported on browser
2. Ensure that each of those projects can produce warnings/errors from the analyzer during a normal build
3. Resolve any warnings/errors that currently exist with the annotations that have been made

/cc @mdh1418 @steveisok

[ghost]

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

[jeffhandley]

@safern @ViktorHofer -- I am going to assign this to someone on my team. Going to assign to myself for the moment.

[jeffhandley]

@mdh1418 @steveisok Question for you...

Identify which libraries projects should have <SupportedPlatform Include="browser" /> specified, and annotate them as such, indicating that the library is intended to be supported on browser

Is the correct answer here the following?

• Find assemblies that do not have [assembly: UnsupportedOSPlatform("browser")]
• Mark all of those assemblies with <SupportedPlatform Include="browser" />

That would mean for all assemblies that have at least partial browser support, that we want to see warnings if we call APIs unsupported on browser without also annotating those call sites.

[steveisok]

Makes sense to me. Seems like the easiest way.

[mdh1418]

That does seem like the way to go.
Is it not the case that assemblies by default are assumed to be supported on all platforms unless otherwise specified by either exclusive Supported or Unsupported? Otherwise, would marking <SupportedPlatform Include="<platform>" /> need to be done for all platforms that fully/partially support assemblies?

[marek-safar]

Why enable it for browser only? There are annotations for other configuration as well and all configurations would benefit from additional checks of not using unsupported APIs (e.g. Linux calling Windows only API or Android calling macOS API)

[buyaa-n]

For Unsupported APIs the analyzer would warn if the platform is included in the default MSBuild <SupportedPlatform>:

<Project>
  <ItemGroup>
    <!-- Platforms supported by this SDK for analyzer warnings. Spec: https://github.com/dotnet/designs/blob/master/accepted/2020/platform-exclusion/platform-exclusion.md  -->
    <SupportedPlatform Include="Android" />
    <SupportedPlatform Include="iOS" />
    <SupportedPlatform Include="Linux" />
    <SupportedPlatform Include="macOS" />
    <SupportedPlatform Include="Windows" />
  </ItemGroup>
</Project>

Or If the project is targeting the platform attributed as unsupported, or the platform is manually included within the MSBuild <SupportedPlatform> items group

[jeffhandley]

@mdh1418 @steveisok -- @buyaa-n and I were discussing this and we think we can actually apply this attribute even more liberally. For projects that have [assembly: UnsupportedOSPlatform("browser")] specified, it would even be OK to mark those with <SupportedPlatform Include="browser" /> as every call site in the application would be treated as unsupported on browser, so no diagnostics would be raised for calls into APIs unsupported on browser.

With that, @buyaa-n is intending to take an approach where <SupportedPlatform Include="browser" /> would be specified for any project while building against $(NetCoreAppCurrent)-Browser or $(NetCoreAppCurrent), regardless of whether or not [assembly: UnsupportedOSPlatform("browser")] is specified.

[buyaa-n]

Checked the Ensure that each of those projects can produce warnings/errors from the analyzer during a normal build task as the PR ensuring that is merged

For the 1st task, I have added the <SupportedPlatform Include="browser"/> for projects targeting browser and seems it is enough (i think no need to add it for all multitargeted projects if we do that we could get unwanted warnings for any unsupported browser API referenced for non-browser targets ).

  <ItemGroup Condition="('$(TargetsBrowser)' == 'true' or '$(TargetOS)' == '') and '$(IsTestProject)' != 'true'">
    <SupportedPlatform Include="browser"/>
  </ItemGroup>

And found 97 warnings found in System.Net.HttpListener, System.Security.Cryptography.Algorithms and System.Net.Http projects. With a quick review looks most of them referencing APIs having assembly-level UnsupportedOSPlatform("browser") but might be supported on the browser. The whole list of warnings attached
unsupported browser warnings.txt

The 3rd task says Resolve any warnings/errors that currently exist with the annotations that have been made, not sure if i should do that because i don't really know how they should be handled, if the warnings supposed to be Asserted or the referencing APIs should have SupportedOSPlatform("browser") attribute added or the call site should have UnsupportedOSPlatform("browser") attribute added or whatever other action needed
cc @jeffhandley @mdh1418 @steveisok

[safern]

For the 1st task, I have added the for projects targeting browser and seems it is enough (i think no need to add it for all multitargeted projects if we do that we could get unwanted warnings for any unsupported browser API referenced for non-browser targets ).

Will this be accurate? Some of this might have -Browser configuration to be able to generate a platform not supported assembly. I think it would be good to expand the condition in the ItemGroup to exclude not supported assemblies.

  <ItemGroup Condition="('$(TargetsBrowser)' == 'true' or '$(TargetOS)' == '' and '$(GeneratePlatformNotSupportedAssemblyMessage )' == '') and '$(IsTestProject)' != 'true'">
    <SupportedPlatform Include="browser"/>
  </ItemGroup>

[buyaa-n]

I think it would be good to expand the condition in the ItemGroup to exclude not supported assemblies.

Not sure if that is needed, for now i don't see any invalid warning caused by that.

And I see some project generating that conditionally

    <GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetsBrowser)' == 'true'">SR.SystemSecurityCryptographyAlgorithms_PlatformNotSupported</GeneratePlatformNotSupportedAssemblyMessage>
    <ApiExclusionListPath Condition="'$(TargetsBrowser)' == 'true'">ExcludeApiList.PNSE.Browser.txt</ApiExclusionListPath>

but still has some browser supported functionality TargetsBrowser

<ItemGroup Condition=" '$(TargetsBrowser)' == 'true'">
    <Compile Include="$(CommonPath)Internal\Cryptography\HashProvider.cs"
             Link="Internal\Cryptography\HashProvider.cs" />
    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetRandomBytes.cs"
             Link="Common\Interop\Unix\System.Native\Interop.GetRandomBytes.cs" />
    <Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs"
             Link="Common\Interop\Unix\Interop.Libraries.cs" />
    <Compile Include="Internal\Cryptography\HashAlgorithmNames.cs" />
    <Compile Include="Internal\Cryptography\HashProviderDispenser.Browser.cs" />
    <Compile Include="Internal\Cryptography\HMACCommon.cs" />
    <Compile Include="Internal\Cryptography\RandomNumberGeneratorImplementation.cs" />
    <Compile Include="Internal\Cryptography\RandomNumberGeneratorImplementation.Browser.cs" />
    <Compile Include="Internal\Cryptography\SHAHashProvider.Browser.cs" />
    <Compile Include="System\Security\Cryptography\RandomNumberGenerator.cs" />
    <Compile Include="System\Security\Cryptography\IncrementalHash.cs" />
    <Compile Include="System\Security\Cryptography\SHA1.cs" />
    <Compile Include="System\Security\Cryptography\SHA1Managed.cs" />
    <Compile Include="System\Security\Cryptography\SHA256.cs" />
    <Compile Include="System\Security\Cryptography\SHA256Managed.cs" />
    <Compile Include="System\Security\Cryptography\SHA384.cs" />
    <Compile Include="System\Security\Cryptography\SHA384Managed.cs" />
    <Compile Include="System\Security\Cryptography\SHA512.cs" />
    <Compile Include="System\Security\Cryptography\SHA512Managed.cs" />
  </ItemGroup>

[safern]

Ah that is because of the ApiExclusionList. We could check for that as well. If the exclusion list is set then we do have some supported APIs

[steveisok]

https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/ExcludeApiList.PNSE.Browser.txt

[buyaa-n]

Ah that is because of the ApiExclusionList. We could check for that as well. If the exclusion list is set then we do have some supported APIs

Yes but, i meant is checking GeneratePlatformNotSupportedAssemblyMessage is not really making any difference, i have tested with and without it, the result of the warning is the same

[jeffhandley]

Yes but, i meant is checking GeneratePlatformNotSupportedAssemblyMessage is not really making any difference, i have tested with and without it, the result of the warning is the same

Would that be covered because any place we have GeneratePlatformNotSupportedAssemblyMessage, we would also have <UnsupportedOSPlatforms>browser</UnsupportedOSPlatforms>?

If that's true, then I don't think there's any benefit to checking that property in the condition since all call sites in the assembly are already covered by [UnsupportedOSPlatform("browser")] but keeping <SupportedPlatform Include="browser" /> in place would keep validation in place that we don't regress that behavior inadvertently.

[buyaa-n]

Would that be covered because any place we have GeneratePlatformNotSupportedAssemblyMessage, we would also have browser?

I haven't checked all, but for the ones i checked it had <UnsupportedOSPlatforms>browser</UnsupportedOSPlatforms>, and i expect all other also has it

My point is even it has GeneratePlatformNotSupportedAssemblyMessage it could have some files compiled for browser only, or it might have no any files for browser, which is completely covered by $(TargetsBrowser)' == 'true' condition no need to add GeneratePlatformNotSupportedAssemblyMessage == '' check