Don't call into unsupported APIs on build targets which don't support…

… quic (dotnet#49261) * Don't call into unsupported APIs on build targets which don't support quic The code could be extracted to HttpConnectionPool.quick.cs but not sure it's worth the split as that would move the majority of the code. Fixes dotnet#49201 Fixes dotnet#49187 * if-def less version * Quic IsSupported working and not throwing for AnyOS. * Fix for running the newly added UnitTest. * Add missing unsupported targets * Reverted Quic changes. * Unsupported to Supported * Guarded unsupported calls with OperationgSystem.Is...() * Addressed comments. * Analyzer fixed, macOS fixed. * Trimming test. * Removed test. * Guard more unsupported platform calls * Some more analyzer attributes. * Fix for Android, which returns true for IsLinux(). * Skip failing trimming test on wasm * Removed additional UnsupportedOS attributes, referenced issue. Co-authored-by: Marie Píchová <[email protected]>
fnerdman · Apr 15, 2021 · d4818a3 · d4818a3
1 parent 7c5a92f
commit d4818a3
Show file tree

Hide file tree

Showing 14 changed files with 192 additions and 86 deletions.
diff --git a/eng/versioning.targets b/eng/versioning.targets
@@ -59,21 +59,28 @@
 
   <ItemGroup>
     <_unsupportedOSPlatforms Include="$(UnsupportedOSPlatforms)" />
+    <_supportedOSPlatforms Include="$(SupportedOSPlatforms)" />
   </ItemGroup>
 
-  <!-- Adds UnsupportedOSPlatform attribute to the assembly when:
-      * At least one <UnsupportedOSPlatforms /> has been specified
+  <!-- Adds UnsupportedOSPlatform and SupportedOSPlatform attributes to the assembly when:
+      * At least one <UnsupportedOSPlatforms /> or <SupportedOSPlatforms /> has been specified
       * This isn't a test project
       * This is a cross-platform target
       * The build isn't targeting .NET Framework
     -->
-  <Target Name="AddUnsupportedOSPlatformAttribute" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"
-    Condition="'@(_unsupportedOSPlatforms)' != '' and '$(TargetsAnyOS)' == 'true' and '$(IsTestProject)' != 'true' and '$(TargetFrameworkIdentifier)' != '.NETFramework'">
-    <ItemGroup>
+  <Target Name="AddOSPlatformAttributes" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"
+    Condition="'$(TargetsAnyOS)' == 'true' and '$(IsTestProject)' != 'true' and '$(TargetFrameworkIdentifier)' != '.NETFramework'">
+    <ItemGroup Condition="'@(_unsupportedOSPlatforms)' != ''">
       <AssemblyAttribute Include="System.Runtime.Versioning.UnsupportedOSPlatform">
         <_Parameter1>%(_unsupportedOSPlatforms.Identity)</_Parameter1>
       </AssemblyAttribute>
     </ItemGroup>
+
+    <ItemGroup Condition="'@(_supportedOSPlatforms)' != ''">
+      <AssemblyAttribute Include="System.Runtime.Versioning.SupportedOSPlatform">
+        <_Parameter1>%(_supportedOSPlatforms.Identity)</_Parameter1>
+      </AssemblyAttribute>
+    </ItemGroup>
   </Target>
 
   <PropertyGroup>

diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj
@@ -19,7 +19,6 @@
   <ItemGroup>
     <ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
   </ItemGroup>
-
   <ItemGroup>
     <Compile Include="System\Net\Http\ByteArrayContent.cs" />
     <Compile Include="System\Net\Http\ByteArrayHelpers.cs" />
@@ -662,20 +661,6 @@
   <ItemGroup Condition=" '$(TargetsBrowser)' == 'true'">
     <ProjectReference Include="$(LibrariesProjectRoot)System.Private.Runtime.InteropServices.JavaScript\src\System.Private.Runtime.InteropServices.JavaScript.csproj" />
   </ItemGroup>
-  <ItemGroup>
-    <Content Include="msquic.dll" Condition="Exists('msquic.dll')">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-      <CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
-    </Content>
-    <Content Include="libmsquic.so" Condition="Exists('libmsquic.so')">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-      <CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
-    </Content>
-    <Content Include="msquic.pdb" Condition="Exists('msquic.pdb')">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-      <CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
-    </Content>
-  </ItemGroup>
   <ItemGroup>
     <None Include="Resources\SR.resx" />
   </ItemGroup>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
@@ -7,6 +7,7 @@
 using System.Net.Quic.Implementations;
 using System.Net.Security;
 using System.Net.Sockets;
+using System.Runtime.Versioning;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 using System.Threading.Tasks;
@@ -100,6 +101,10 @@ private static async ValueTask<SslStream> EstablishSslConnectionAsyncCore(bool a
             return sslStream;
         }
 
+        // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+        [SupportedOSPlatform("windows")]
+        [SupportedOSPlatform("linux")]
+        [SupportedOSPlatform("macos")]
         public static async ValueTask<QuicConnection> ConnectQuicAsync(QuicImplementationProvider quicImplementationProvider, DnsEndPoint endPoint, SslClientAuthenticationOptions? clientAuthenticationOptions, CancellationToken cancellationToken)
         {
             QuicConnection con = new QuicConnection(quicImplementationProvider, endPoint, clientAuthenticationOptions);

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
@@ -4,6 +4,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using System.Runtime.CompilerServices;
+using System.Runtime.Versioning;
 using System.Net.Quic;
 using System.IO;
 using System.Collections.Generic;
@@ -13,6 +14,10 @@
 
 namespace System.Net.Http
 {
+    // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+    [SupportedOSPlatform("windows")]
+    [SupportedOSPlatform("linux")]
+    [SupportedOSPlatform("macos")]
     internal sealed class Http3Connection : HttpConnectionBase, IDisposable
     {
         // TODO: once HTTP/3 is standardized, create APIs for this.

diff --git a/...raries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs b/...raries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ConnectionException.cs
@@ -2,10 +2,15 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Runtime.Serialization;
+using System.Runtime.Versioning;
 
 namespace System.Net.Http
 {
     [Serializable]
+    // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+    [SupportedOSPlatform("windows")]
+    [SupportedOSPlatform("linux")]
+    [SupportedOSPlatform("macos")]
     internal sealed class Http3ConnectionException : Http3ProtocolException
     {
         public Http3ConnectionException(Http3ErrorCode errorCode)

diff --git a/...ibraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs b/...ibraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3ProtocolException.cs
@@ -2,10 +2,15 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Runtime.Serialization;
+using System.Runtime.Versioning;
 
 namespace System.Net.Http
 {
     [Serializable]
+    // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+    [SupportedOSPlatform("windows")]
+    [SupportedOSPlatform("linux")]
+    [SupportedOSPlatform("macos")]
     internal class Http3ProtocolException : Exception
     {
         public Http3ErrorCode ErrorCode { get; }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
@@ -10,11 +10,16 @@
 using System.Threading;
 using System.Threading.Tasks;
 using System.Runtime.CompilerServices;
+using System.Runtime.Versioning;
 using System.Net.Http.QPack;
 using System.Runtime.ExceptionServices;
 
 namespace System.Net.Http
 {
+    // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+    [SupportedOSPlatform("windows")]
+    [SupportedOSPlatform("linux")]
+    [SupportedOSPlatform("macos")]
     internal sealed class Http3RequestStream : IHttpHeadersHandler, IAsyncDisposable, IDisposable
     {
         private readonly HttpRequestMessage _request;

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
@@ -14,6 +14,7 @@
 using System.Net.Sockets;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Runtime.Versioning;
 using System.Security.Authentication;
 using System.Text;
 using System.Threading;
@@ -118,7 +119,11 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK
             }
 
             _http2Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version20;
-            _http3Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version30 && (_poolManager.Settings._quicImplementationProvider ?? QuicImplementationProviders.Default).IsSupported;
+            // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+            if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
+            {
+                _http3Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version30 && (_poolManager.Settings._quicImplementationProvider ?? QuicImplementationProviders.Default).IsSupported;
+            }
 
             switch (kind)
             {
@@ -244,10 +249,14 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK
                     _http3EncodedAuthorityHostHeader = QPackEncoder.EncodeLiteralHeaderFieldWithStaticNameReferenceToArray(H3StaticTable.Authority, hostHeader);
                 }
 
-                if (_http3Enabled)
+                // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+                if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
                 {
-                    _sslOptionsHttp3 = ConstructSslOptions(poolManager, sslHostName);
-                    _sslOptionsHttp3.ApplicationProtocols = s_http3ApplicationProtocols;
+                    if (_http3Enabled)
+                    {
+                        _sslOptionsHttp3 = ConstructSslOptions(poolManager, sslHostName);
+                        _sslOptionsHttp3.ApplicationProtocols = s_http3ApplicationProtocols;
+                    }
                 }
             }
 
@@ -260,10 +269,22 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK
             if (NetEventSource.Log.IsEnabled()) Trace($"{this}");
         }
 
-        private static readonly List<SslApplicationProtocol> s_http3ApplicationProtocols = new List<SslApplicationProtocol>() { Http3Connection.Http3ApplicationProtocol31, Http3Connection.Http3ApplicationProtocol30, Http3Connection.Http3ApplicationProtocol29 };
+        private static readonly List<SslApplicationProtocol> s_http3ApplicationProtocols = CreateHttp3ApplicationProtocols();
         private static readonly List<SslApplicationProtocol> s_http2ApplicationProtocols = new List<SslApplicationProtocol>() { SslApplicationProtocol.Http2, SslApplicationProtocol.Http11 };
         private static readonly List<SslApplicationProtocol> s_http2OnlyApplicationProtocols = new List<SslApplicationProtocol>() { SslApplicationProtocol.Http2 };
 
+        private static List<SslApplicationProtocol> CreateHttp3ApplicationProtocols()
+        {
+            // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+            if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
+            {
+                // TODO: Once the HTTP/3 versions are part of SslApplicationProtocol, see https://github.com/dotnet/runtime/issues/1293, move this back to field initialization.
+                return new List<SslApplicationProtocol>() { Http3Connection.Http3ApplicationProtocol31, Http3Connection.Http3ApplicationProtocol30, Http3Connection.Http3ApplicationProtocol29 };
+            }
+
+            return null!;
+        }
+
         private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName)
         {
             Debug.Assert(sslHostName != null);
@@ -355,24 +376,28 @@ public byte[] Http2AltSvcOriginUri
                 }
             }
 
-            // Either H3 explicitly requested or secured upgraded allowed.
-            if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure)))
+            // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+            if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
             {
-                HttpAuthority? authority = _http3Authority;
-                // H3 is explicitly requested, assume prenegotiated H3.
-                if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower)
+                // Either H3 explicitly requested or secured upgraded allowed.
+                if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure)))
                 {
-                    authority = authority ?? _originAuthority;
-                }
-                if (authority != null)
-                {
-                    if (IsAltSvcBlocked(authority))
+                    HttpAuthority? authority = _http3Authority;
+                    // H3 is explicitly requested, assume prenegotiated H3.
+                    if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower)
                     {
-                        return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>(
-                            new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3)));
+                        authority = authority ?? _originAuthority;
                     }
+                    if (authority != null)
+                    {
+                        if (IsAltSvcBlocked(authority))
+                        {
+                            return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>(
+                                new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3)));
+                        }
 
-                    return GetHttp3ConnectionAsync(request, authority, cancellationToken);
+                        return GetHttp3ConnectionAsync(request, authority, cancellationToken);
+                    }
                 }
             }
 
@@ -722,6 +747,10 @@ private void AddHttp2Connection(Http2Connection newConnection)
             }
         }
 
+        // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+        [SupportedOSPlatform("windows")]
+        [SupportedOSPlatform("linux")]
+        [SupportedOSPlatform("macos")]
         private async ValueTask<(HttpConnectionBase connection, bool isNewConnection)>
             GetHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken)
         {
@@ -894,14 +923,18 @@ public async ValueTask<HttpResponseMessage> SendWithRetryAsync(HttpRequestMessag
                     HandleAltSvc(altSvcHeaderValues, response.Headers.Age);
                 }
 
-                // If an Alt-Svc authority returns 421, it means it can't actually handle the request.
-                // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug.
-                // In this case, we blocklist the authority and retry the request at the origin.
-                if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection is Http3Connection h3Connection && h3Connection.Authority != _originAuthority)
+                // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+                if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
                 {
-                    response.Dispose();
-                    BlocklistAuthority(h3Connection.Authority);
-                    continue;
+                    // If an Alt-Svc authority returns 421, it means it can't actually handle the request.
+                    // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug.
+                    // In this case, we blocklist the authority and retry the request at the origin.
+                    if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection is Http3Connection h3Connection && h3Connection.Authority != _originAuthority)
+                    {
+                        response.Dispose();
+                        BlocklistAuthority(h3Connection.Authority);
+                        continue;
+                    }
                 }
 
                 return response;

diff --git a/...ibraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/...ibraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs
@@ -6,6 +6,7 @@
 using System.IO;
 using System.Net.Quic;
 using System.Net.Quic.Implementations;
+using System.Runtime.Versioning;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -86,7 +87,7 @@ public HttpConnectionSettings CloneAndNormalize()
                 _cookieContainer = new CookieContainer();
             }
 
-            return new HttpConnectionSettings()
+            var settings = new HttpConnectionSettings()
             {
                 _allowAutoRedirect = _allowAutoRedirect,
                 _automaticDecompression = _automaticDecompression,
@@ -118,9 +119,17 @@ public HttpConnectionSettings CloneAndNormalize()
                 _responseHeaderEncodingSelector = _responseHeaderEncodingSelector,
                 _enableMultipleHttp2Connections = _enableMultipleHttp2Connections,
                 _connectCallback = _connectCallback,
-                _plaintextStreamFilter = _plaintextStreamFilter,
-                _quicImplementationProvider = _quicImplementationProvider
+                _plaintextStreamFilter = _plaintextStreamFilter
             };
+
+            // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+            // TODO: Remove if/when QuicImplementationProvider is removed from System.Net.Quic.
+            if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
+            {
+                settings._quicImplementationProvider = _quicImplementationProvider;
+            }
+
+            return settings;
         }
 
         private static bool AllowHttp2
@@ -178,6 +187,11 @@ private static bool AllowDraftHttp3
         public bool EnableMultipleHttp2Connections => _enableMultipleHttp2Connections;
 
         private byte[]? _http3SettingsFrame;
+
+        // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305
+        [SupportedOSPlatform("windows")]
+        [SupportedOSPlatform("linux")]
+        [SupportedOSPlatform("macos")]
         internal byte[] Http3SettingsFrame => _http3SettingsFrame ??= Http3Connection.BuildSettingsFrame(this);
     }
 }
diff --git a/src/libraries/System.Net.Http/tests/TrimmingTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/TrimmingTests/HttpClientTest.cs
@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.IO;
+using System.Net.Http;
+using System.Threading.Tasks;
+
+class Program
+{
+    static async Task<int> Main(string[] args)
+    {
+        using var client = new HttpClient();
+        using var response = await client.GetAsync("https://www.microsoft.com");            
+        var result = await response.Content.ReadAsStringAsync();
+        Console.WriteLine(result);
+
+        const string quicDll = "System.Net.Quic.dll";
+        var quicDllExists = File.Exists(Path.Combine(AppContext.BaseDirectory, quicDll));
+
+        // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished
+        if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS())
+        {
+            Console.WriteLine($"Expected {quicDll} is {(quicDllExists ? "present - OK" : "missing - BAD")}.");
+            return quicDllExists ? 100 : -1;
+        }
+        else
+        {
+            Console.WriteLine($"Unexpected {quicDll} is {(quicDllExists ? "present - BAD" : "missing - OK")}.");
+            return quicDllExists ? -1 : 100;
+        }
+    }
+}
diff --git a/src/libraries/System.Net.Http/tests/TrimmingTests/System.Net.Http.TrimmingTests.proj b/src/libraries/System.Net.Http/tests/TrimmingTests/System.Net.Http.TrimmingTests.proj
@@ -0,0 +1,11 @@
+<Project DefaultTargets="Build">
+  <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.props))" />
+
+  <ItemGroup>
+    <TestConsoleAppSourceFiles Include="HttpClientTest.cs">
+      <SkipOnTestRuntimes>browser-wasm</SkipOnTestRuntimes>
+    </TestConsoleAppSourceFiles>
+  </ItemGroup>
+
+  <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.targets))" />
+</Project>