Skip to content

Commit

Permalink
Address System.Net.Http.WinHttpHandler's nullable warnings targeting …
Browse files Browse the repository at this point in the history 
….NETCoreApp (#54995)

* Update nullability for WinHttpHandler

* Update project file

* Use ! for PtrToStringUni

* Move assertion of _sessionHandle

* Remove false assertion about RequestHandle.
  • Loading branch information
@huoyaoyuan
huoyaoyuan authored Jul 14, 2021
1 parent 3e8ef85 commit 69a4f83
Show file tree
Hide file tree
Showing 15 changed files with 163 additions and 94 deletions.
10 changes: 5 additions & 5 deletions src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ internal static partial class WinHttp
public static extern SafeWinHttpHandle WinHttpOpen(
IntPtr userAgent,
uint accessType,
string proxyName,
string proxyBypass, int flags);
string? proxyName,
string? proxyBypass, int flags);

[DllImport(Interop.Libraries.WinHttp, CharSet = CharSet.Unicode, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
Expand All @@ -33,7 +33,7 @@ public static extern SafeWinHttpHandle WinHttpOpenRequest(
SafeWinHttpHandle connectHandle,
string verb,
string objectName,
string version,
string? version,
string referrer,
string acceptTypes,
uint flags);
Expand Down Expand Up @@ -161,8 +161,8 @@ public static extern bool WinHttpSetCredentials(
SafeWinHttpHandle requestHandle,
uint authTargets,
uint authScheme,
string userName,
string password,
string? userName,
string? password,
IntPtr reserved);

[DllImport(Interop.Libraries.WinHttp, CharSet = CharSet.Unicode, SetLastError = true)]
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
<PropertyGroup>
<GeneratePlatformNotSupportedAssemblyMessage Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) != '.NETFramework' and
'$(TargetsWindows)' != 'true'">SR.PlatformNotSupported_WinHttpHandler</GeneratePlatformNotSupportedAssemblyMessage>
<!-- S.N.Http.WinHttpHandler has a lot nullable warnings that need to be addressed: https://github.com/dotnet/runtime/issues/54598. -->
<Nullable Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETCoreApp'">annotations</Nullable>
</PropertyGroup>

<ItemGroup Condition="'$(TargetsWindows)' == 'true'" >
Expand Down
49 changes: 33 additions & 16 deletions src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;

using System.Diagnostics.CodeAnalysis;
using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle;

namespace System.Net.Http
Expand All @@ -14,7 +14,7 @@ internal sealed class WinHttpAuthHelper
// WINHTTP_AUTH_SCHEME_NTLM = 0x00000002;
// WINHTTP_AUTH_SCHEME_DIGEST = 0x00000008;
// WINHTTP_AUTH_SCHEME_NEGOTIATE = 0x00000010;
private static readonly string[] s_authSchemeStringMapping =
private static readonly string?[] s_authSchemeStringMapping =
{
null,
"Basic",
Expand Down Expand Up @@ -54,6 +54,10 @@ public void CheckResponseForAuthentication(
uint supportedSchemes = 0;
uint firstSchemeIgnored = 0;
uint authTarget = 0;

Debug.Assert(state.RequestMessage != null);
Debug.Assert(state.RequestMessage.RequestUri != null);
Debug.Assert(state.RequestHandle != null);
Uri uri = state.RequestMessage.RequestUri;

state.RetryRequest = false;
Expand Down Expand Up @@ -121,7 +125,7 @@ public void CheckResponseForAuthentication(
state.LastStatusCode = statusCode;

// If we don't have any proxy credentials to try, then we end up with 407.
ICredentials proxyCreds = state.Proxy == null ?
ICredentials? proxyCreds = state.Proxy == null ?
state.DefaultProxyCredentials :
state.Proxy.Credentials;
if (proxyCreds == null)
Expand Down Expand Up @@ -161,6 +165,7 @@ public void CheckResponseForAuthentication(
default:
if (state.PreAuthenticate && serverAuthScheme != 0)
{
Debug.Assert(state.ServerCredentials != null);
SaveServerCredentialsToCache(uri, serverAuthScheme, state.ServerCredentials);
}
break;
Expand All @@ -169,6 +174,10 @@ public void CheckResponseForAuthentication(

public void PreAuthenticateRequest(WinHttpRequestState state, uint proxyAuthScheme)
{
Debug.Assert(state.RequestHandle != null);
Debug.Assert(state.RequestMessage != null);
Debug.Assert(state.RequestMessage.RequestUri != null);

// Set proxy credentials if we have them.
// If a proxy authentication challenge was responded to, reset
// those credentials before each SendRequest, because the proxy
Expand All @@ -177,8 +186,9 @@ public void PreAuthenticateRequest(WinHttpRequestState state, uint proxyAuthSche
// 407-401-407-401- loop.
if (proxyAuthScheme != 0)
{
ICredentials proxyCredentials;
Uri proxyUri;
ICredentials? proxyCredentials;
Uri? proxyUri;

if (state.Proxy != null)
{
proxyCredentials = state.Proxy.Credentials;
Expand All @@ -190,6 +200,9 @@ public void PreAuthenticateRequest(WinHttpRequestState state, uint proxyAuthSche
proxyUri = state.RequestMessage.RequestUri;
}

Debug.Assert(proxyCredentials != null);
Debug.Assert(proxyUri != null);

SetWinHttpCredential(
state.RequestHandle,
proxyCredentials,
Expand All @@ -202,7 +215,7 @@ public void PreAuthenticateRequest(WinHttpRequestState state, uint proxyAuthSche
if (state.PreAuthenticate)
{
uint authScheme;
NetworkCredential serverCredentials;
NetworkCredential? serverCredentials;
if (GetServerCredentialsFromCache(
state.RequestMessage.RequestUri,
out authScheme,
Expand All @@ -226,18 +239,18 @@ public void PreAuthenticateRequest(WinHttpRequestState state, uint proxyAuthSche
public bool GetServerCredentialsFromCache(
Uri uri,
out uint serverAuthScheme,
out NetworkCredential serverCredentials)
[NotNullWhen(true)] out NetworkCredential? serverCredentials)
{
serverAuthScheme = 0;
serverCredentials = null;

NetworkCredential cred = null;
NetworkCredential? cred = null;

lock (_credentialCacheLock)
{
foreach (uint authScheme in s_authSchemePriorityOrder)
{
cred = _credentialCache.GetCredential(uri, s_authSchemeStringMapping[authScheme]);
cred = _credentialCache.GetCredential(uri, s_authSchemeStringMapping[authScheme]!);
if (cred != null)
{
serverAuthScheme = authScheme;
Expand All @@ -253,10 +266,10 @@ public bool GetServerCredentialsFromCache(

public void SaveServerCredentialsToCache(Uri uri, uint authScheme, ICredentials serverCredentials)
{
string authType = s_authSchemeStringMapping[authScheme];
string? authType = s_authSchemeStringMapping[authScheme];
Debug.Assert(!string.IsNullOrEmpty(authType));

NetworkCredential cred = serverCredentials.GetCredential(uri, authType);
NetworkCredential? cred = serverCredentials.GetCredential(uri, authType);
if (cred != null)
{
lock (_credentialCacheLock)
Expand Down Expand Up @@ -303,15 +316,17 @@ private bool SetWinHttpCredential(
uint authScheme,
uint authTarget)
{
string userName;
string password;
string? userName;
string? password;

Debug.Assert(credentials != null);
Debug.Assert(authScheme != 0);
Debug.Assert(authTarget == Interop.WinHttp.WINHTTP_AUTH_TARGET_PROXY ||
authTarget == Interop.WinHttp.WINHTTP_AUTH_TARGET_SERVER);

NetworkCredential networkCredential = credentials.GetCredential(uri, s_authSchemeStringMapping[authScheme]);
string? authType = s_authSchemeStringMapping[authScheme];
Debug.Assert(!string.IsNullOrEmpty(authType));
NetworkCredential? networkCredential = credentials.GetCredential(uri, authType);

if (networkCredential == null)
{
Expand Down Expand Up @@ -367,7 +382,7 @@ private bool SetWinHttpCredential(
return true;
}

private static uint ChooseAuthScheme(uint supportedSchemes, Uri uri, ICredentials credentials)
private static uint ChooseAuthScheme(uint supportedSchemes, Uri? uri, ICredentials? credentials)
{
if (credentials == null)
{
Expand All @@ -383,9 +398,11 @@ private static uint ChooseAuthScheme(uint supportedSchemes, Uri uri, ICredential
return 0;
}

Debug.Assert(uri != null);

foreach (uint authScheme in s_authSchemePriorityOrder)
{
if ((supportedSchemes & authScheme) != 0 && credentials.GetCredential(uri, s_authSchemeStringMapping[authScheme]) != null)
if ((supportedSchemes & authScheme) != 0 && credentials.GetCredential(uri, s_authSchemeStringMapping[authScheme]!) != null)
{
return authScheme;
}
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCertificateHelper.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Net.Security;
using System.Runtime.InteropServices;
using System.Security.Cryptography;
Expand All @@ -20,7 +21,6 @@ public static void BuildChain(
out X509Chain chain,
out SslPolicyErrors sslPolicyErrors)
{
chain = null;
sslPolicyErrors = SslPolicyErrors.None;

// Build the chain.
Expand Down Expand Up @@ -69,6 +69,8 @@ public static void BuildChain(
Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_ALL &
~Interop.Crypt32.CertChainPolicyIgnoreFlags.CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG;

Debug.Assert(chain.SafeHandle != null);

Interop.Crypt32.CERT_CHAIN_POLICY_STATUS status = default;
status.cbSize = (uint)sizeof(Interop.Crypt32.CERT_CHAIN_POLICY_STATUS);
if (Interop.Crypt32.CertVerifyCertificateChainPolicy(
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpChannelBinding.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace System.Net.Http
internal sealed class WinHttpChannelBinding : ChannelBinding
{
private int _size;
private string _cachedToString;
private string? _cachedToString;

internal WinHttpChannelBinding(SafeWinHttpHandle requestHandle)
{
Expand Down Expand Up @@ -56,7 +56,7 @@ public override int Size
}
}

public override string ToString()
public override string? ToString()
{
if (_cachedToString == null && !IsInvalid)
{
Expand Down
25 changes: 16 additions & 9 deletions ...aries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@ internal static class WinHttpCookieContainerAdapter

public static void AddResponseCookiesToContainer(WinHttpRequestState state)
{
HttpRequestMessage request = state.RequestMessage;
SafeWinHttpHandle requestHandle = state.RequestHandle;
CookieContainer cookieContainer = state.Handler.CookieContainer;
HttpRequestMessage? request = state.RequestMessage;
SafeWinHttpHandle? requestHandle = state.RequestHandle;
Debug.Assert(state.Handler != null);
CookieContainer? cookieContainer = state.Handler.CookieContainer;

Debug.Assert(state.Handler.CookieUsePolicy == CookieUsePolicy.UseSpecifiedCookieContainer);
Debug.Assert(request != null);
Debug.Assert(requestHandle != null);
Debug.Assert(cookieContainer != null);
Debug.Assert(request.RequestUri != null);

// Get 'Set-Cookie' headers from response.
char[] buffer = null;
char[]? buffer = null;
uint index = 0;
string cookieHeader;
string? cookieHeader;
while (WinHttpResponseParser.GetResponseHeader(
requestHandle, Interop.WinHttp.WINHTTP_QUERY_SET_COOKIE, ref buffer, ref index, out cookieHeader))
{
Expand All @@ -44,9 +48,12 @@ public static void AddResponseCookiesToContainer(WinHttpRequestState state)

public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redirectUri)
{
SafeWinHttpHandle requestHandle = state.RequestHandle;
SafeWinHttpHandle? requestHandle = state.RequestHandle;

Debug.Assert(state.Handler != null);
Debug.Assert(state.Handler.CookieUsePolicy == CookieUsePolicy.UseSpecifiedCookieContainer);
Debug.Assert(state.Handler.CookieContainer != null);
Debug.Assert(requestHandle != null);

// Clear cookies.
if (!Interop.WinHttp.WinHttpAddRequestHeaders(
Expand All @@ -64,7 +71,7 @@ public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redi

// Re-add cookies. The GetCookieHeader() method will return the correct set of
// cookies based on the redirectUri.
string cookieHeader = GetCookieHeader(redirectUri, state.Handler.CookieContainer);
string? cookieHeader = GetCookieHeader(redirectUri, state.Handler.CookieContainer);
if (!string.IsNullOrEmpty(cookieHeader))
{
if (!Interop.WinHttp.WinHttpAddRequestHeaders(
Expand All @@ -78,9 +85,9 @@ public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redi
}
}

public static string GetCookieHeader(Uri uri, CookieContainer cookies)
public static string? GetCookieHeader(Uri uri, CookieContainer cookies)
{
string cookieHeader = null;
string? cookieHeader = null;

Debug.Assert(cookies != null);

Expand Down
Loading

0 comments on commit 69a4f83

Please sign in to comment.