dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.99k stars 4.66k forks source link

`NetworkCredential` method parameters nullability do not match `ICredentials` and `ICredentialsByHost` #108134

Open nil4 opened 2 hours ago

nil4 commented 2 hours ago

Description

Some method parameters in the NetworkCredentials class do not match the nullability of its implemented interfaces ICredentials and ICredentialsByHost.

The interfaces ref API surface:

https://github.com/dotnet/runtime/blob/b5833d2cd22f474485f74696609f6a6c3617cd15/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs#L218-L225

And their declarations in src:

https://github.com/dotnet/runtime/blob/b5833d2cd22f474485f74696609f6a6c3617cd15/src/libraries/System.Net.Primitives/src/System/Net/ICredentials.cs#L22

https://github.com/dotnet/runtime/blob/b5833d2cd22f474485f74696609f6a6c3617cd15/src/libraries/System.Net.Primitives/src/System/Net/ICredentialsByHost.cs#L22

Have all parameters declared as non-nullable.

However, the same methods in NetworkCredentials declare a few nullable parameters instead (e.g. uri, host and authenticationType), in the ref API surface:

https://github.com/dotnet/runtime/blob/b5833d2cd22f474485f74696609f6a6c3617cd15/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs#L349-L350

And the implementation:

https://github.com/dotnet/runtime/blob/b5833d2cd22f474485f74696609f6a6c3617cd15/src/libraries/System.Net.Primitives/src/System/Net/NetworkCredential.cs#L144-L152

Tnis is a problem for tools which try to expose .NET-based APIs to other languages with stricter null/non-null or optional/non-optional type systems (for example, Kotlin.)

Reproduction Steps

https://github.com/royalapplications/beyondnet/commit/a208fa7a5f3c2d9c2d3292e9b516be68e5369b8c

Expected behavior

The nullability of NetworkCredential method parameters should match the implemented interfaces.

Actual behavior

Method parameters in the NetworkCredential class do not match the implemented interfaces w.r.t nullability.

Regression?

No response

Known Workarounds

Nullability mismatches can't be easily worked-around, and require excluding such classes/interfaces from projections to other languages.

Configuration

.NET SDK 9.0-RC1 (9.0.100-rc.1.24452.12)

Other information

ref. https://github.com/royalapplications/beyondnet/issues/81

https://github.com/royalapplications/beyondnet/commit/a208fa7a5f3c2d9c2d3292e9b516be68e5369b8c#diff-5aa8d856f0109ff101b733a9bbceea8752c1f10fa2b69fd0d16fc9095cac0d23R20-R30

dotnet-policy-service[bot] commented 2 hours ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

huoyaoyuan commented 2 hours ago

It's correct and expected. Interface implementations/overrides can have more relaxed nullability than the base definition, that is removing ? on return value and add ?s on parameters. See https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/api-guidelines/nullability.md#virtualabstract-methods-and-interfaces .

An example is object.ToString(). While the base definition returns nullable string to mitigate implementations that returns null in the wild, most overrides are returning non-nullable string.

Nullability mismatches can't be easily worked-around, and require excluding such classes/interfaces from projections to other languages.

The rules may not be exactly the same between different ecosystems. In C# there are also many attributes like [NotNullWhen(true)] that alters the rule of nullability. That's something that projections must deal with.