Skip to content

Commit

Permalink
Don't call into unsupported APIs on build targets which don't support…
Browse files Browse the repository at this point in the history
… quic (#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 #49201
Fixes #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]>
  • Loading branch information
marek-safar and ManickaP authored Apr 15, 2021
1 parent 7c5a92f commit d4818a3
Show file tree
Hide file tree
Showing 14 changed files with 192 additions and 86 deletions.
17 changes: 12 additions & 5 deletions eng/versioning.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
15 changes: 0 additions & 15 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -86,7 +87,7 @@ public HttpConnectionSettings CloneAndNormalize()
_cookieContainer = new CookieContainer();
}

return new HttpConnectionSettings()
var settings = new HttpConnectionSettings()
{
_allowAutoRedirect = _allowAutoRedirect,
_automaticDecompression = _automaticDecompression,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
@@ -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>
Loading

0 comments on commit d4818a3

Please sign in to comment.