Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate Windows version when using WinHttpHandler #2229

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Validate Windows version when using WinHttpHandler #2229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Grpc.Net.Client/GrpcChannel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr
{
Log.AddressPathUnused(Logger, Address.OriginalString);
}

// Validate the Windows version can support WinHttpHandler.
const int WinServer2022BuildVersion = 20348;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment needs to elaborate on why this particular build version was chosen.
Basically, as you said

  • If OS build version is greater or equal to WinServer 2022 then there is partial support.
  • If OS build version is greater or equal to Win 11 then there is full support.

The also need to be a note that this will allow creating GrpcChannel on WinServer 2022 with "partial support" (and explain what that is).

if (HttpHandlerType == HttpHandlerType.WinHttpHandler &&
OperatingSystem.IsWindows &&
OperatingSystem.OSVersion.Build < WinServer2022BuildVersion)
{
throw new InvalidOperationException("The channel configuration isn't valid on this operating system. " +
"The channel is configured to use WinHttpHandler and the current version of Windows " +
"doesn't support HTTP/2 features required by gRPC. Windows Server 2022 or Windows 11 or later is required. " +
"For more information, see https://aka.ms/aspnet/grpc/netframework.");
}
}

private void ResolveCredentials(GrpcChannelOptions channelOptions, out bool isSecure, out List<CallCredentials>? callCredentials)
Expand Down
8 changes: 7 additions & 1 deletion src/Grpc.Net.Client/Internal/OperatingSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand All @@ -24,6 +24,8 @@ internal interface IOperatingSystem
{
bool IsBrowser { get; }
bool IsAndroid { get; }
bool IsWindows { get; }
Version OSVersion { get; }
}

internal sealed class OperatingSystem : IOperatingSystem
Expand All @@ -32,6 +34,8 @@ internal sealed class OperatingSystem : IOperatingSystem

public bool IsBrowser { get; }
public bool IsAndroid { get; }
public bool IsWindows { get; }
public Version OSVersion { get; }

private OperatingSystem()
{
Expand All @@ -41,5 +45,7 @@ private OperatingSystem()
#else
IsAndroid = false;
#endif
IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
OSVersion = Environment.OSVersion.Version;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excerpt from offline discussion:

I experimented with Environment.OSVersion and one issue I found is that when called from .NET Framework, it seems to return garbage on newer windows versions (e.g. I tried on Microsoft Windows Server 2019 Datacenter, 10.0.17763 Build 17763 and it returned "Microsoft Windows NT 6.2.9200.0"). Looks like the value returned is the latest windows version the .NET Framework knows about (Which is way too old now and not useful to us).
So some better way of detecting OS Version will be needed.

Can you confirm that this has been taken into account and that we really rely on Environment.OSVersion.Version providing the right info when on .NET Framework (which is actually where we care about it the most)?
This would also deserve a test that actually targets net462 (not sure if we currently have one).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, if Environment.OSVersion.Version gives outdated info about build number, in practice this would mean that a high-enough build number would never be detected and creating the GrpcChannel would always throw on .NET Framework?

}
4 changes: 3 additions & 1 deletion test/Grpc.Net.Client.Tests/GetStatusTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand Down Expand Up @@ -214,6 +214,8 @@ private class TestOperatingSystem : IOperatingSystem
{
public bool IsBrowser { get; set; }
public bool IsAndroid { get; set; }
public bool IsWindows { get; set; }
public Version OSVersion { get; set; } = new Version(1, 2, 3, 4);
}

[Test]
Expand Down
60 changes: 60 additions & 0 deletions test/Grpc.Net.Client.Tests/GrpcChannelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,66 @@ private class TestOperatingSystem : IOperatingSystem
{
public bool IsBrowser { get; set; }
public bool IsAndroid { get; set; }
public bool IsWindows { get; set; }
public Version OSVersion { get; set; } = new Version(1, 2, 3, 4);
}

[Test]
public void WinHttpHandler_UnsupportedWindows_Throw()
{
// Arrange
var services = new ServiceCollection();
services.AddSingleton<IOperatingSystem>(new TestOperatingSystem
{
IsWindows = true,
OSVersion = new Version(1, 2, 3, 4)
});

#pragma warning disable CS0436 // Just need to have a type called WinHttpHandler to activate new behavior.
var winHttpHandler = new WinHttpHandler(new TestHttpMessageHandler());
#pragma warning restore CS0436

// Act
var ex = Assert.Throws<InvalidOperationException>(() =>
{
GrpcChannel.ForAddress("https://localhost", new GrpcChannelOptions
{
HttpHandler = winHttpHandler,
ServiceProvider = services.BuildServiceProvider()
});
});

// Assert
Assert.AreEqual(ex!.Message, "The channel configuration isn't valid on this operating system. " +
"The channel is configured to use WinHttpHandler and the current version of Windows " +
"doesn't support HTTP/2 features required by gRPC. Windows Server 2022 or Windows 11 or later is required. " +
"For more information, see https://aka.ms/aspnet/grpc/netframework.");
}

[Test]
public void WinHttpHandler_SupportedWindows_Success()
{
// Arrange
var services = new ServiceCollection();
services.AddSingleton<IOperatingSystem>(new TestOperatingSystem
{
IsWindows = true,
OSVersion = Version.Parse("10.0.20348.169")
});

#pragma warning disable CS0436 // Just need to have a type called WinHttpHandler to activate new behavior.
var winHttpHandler = new WinHttpHandler(new TestHttpMessageHandler());
#pragma warning restore CS0436

// Act
var channel = GrpcChannel.ForAddress("https://localhost", new GrpcChannelOptions
{
HttpHandler = winHttpHandler,
ServiceProvider = services.BuildServiceProvider()
});

// Assert
Assert.AreEqual(HttpHandlerType.WinHttpHandler, channel.HttpHandlerType);
}

#if SUPPORT_LOAD_BALANCING
Expand Down