Annotate System.Net.Security for nullable reference types #32541

eiriktsarpalis · 2020-02-19T14:18:36Z

Contributes to #2339

I think I might have spotted a few bugs, please see my comments in the diff /cc @dotnet/ncl

src/libraries/System.Net.Security/ref/System.Net.Security.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicy.cs

eiriktsarpalis · 2020-02-21T18:04:31Z

Rebased on top of newly added System.Net.Primitives annotations

src/libraries/System.Net.Security/ref/System.Net.Security.cs

eerhardt · 2020-02-26T16:03:26Z

    public InvalidCredentialException(string message, System.Exception innerException) { }

string? message, System.Exception? innerException. Here and for string message overload above.

Refers to: src/libraries/System.Net.Security/ref/System.Net.Security.cs:583 in cadecbf. [](commit_id = cadecbf680f43be03eb4fd7ca59df230ffe075ba, deletion_comment = False)

eerhardt · 2020-02-26T16:06:38Z

    public bool Contains(string searchServiceName) { throw null; }

From a quick look at the code, it appears that Contains(string searchServiceName) handles passing in null. Should this be string? searchServiceName?

Refers to: src/libraries/System.Net.Security/ref/System.Net.Security.cs:617 in cadecbf. [](commit_id = cadecbf680f43be03eb4fd7ca59df230ffe075ba, deletion_comment = False)

src/libraries/System.Net.Security/ref/System.Net.Security.cs

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Crypto.cs

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs

src/libraries/Common/src/Interop/Windows/SspiCli/NegotiationInfoClass.cs

src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs

src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs

src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs

src/libraries/Common/src/System/Net/NTAuthentication.Common.cs

src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslApplicationProtocol.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs

src/libraries/System.Net.Security/src/System/Net/Security/TlsCipherSuiteData.cs

eiriktsarpalis · 2020-02-27T09:36:06Z

Rebased on top of System.Net.Sockets annotations.

eerhardt

Assuming your questions to the library owners are resolved, this LGTM. Nice work.

eiriktsarpalis requested review from stephentoub, wfurt and buyaa-n February 19, 2020 14:18

eiriktsarpalis commented Feb 19, 2020

View reviewed changes

src/libraries/System.Net.Security/ref/System.Net.Security.cs Show resolved Hide resolved

eiriktsarpalis commented Feb 19, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs Show resolved Hide resolved

eiriktsarpalis commented Feb 19, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs Show resolved Hide resolved

eiriktsarpalis commented Feb 19, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs Show resolved Hide resolved

eiriktsarpalis commented Feb 19, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicy.cs Outdated Show resolved Hide resolved

eiriktsarpalis added the area-System.Net.Security label Feb 19, 2020

carlossanlop requested a review from safern February 19, 2020 17:38

eiriktsarpalis marked this pull request as ready for review February 19, 2020 20:42

eiriktsarpalis force-pushed the annotate-system-net-security branch 4 times, most recently from d496304 to 9c18638 Compare February 21, 2020 18:02