Skip to content

Commit c883fb2

Browse files
author
John Luo
committed
Feedback
1 parent b60c656 commit c883fb2

File tree

3 files changed

+57
-27
lines changed

3 files changed

+57
-27
lines changed

src/Servers/Kestrel/Core/src/CoreStrings.resx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
563563
<data name="RequestTrailersNotAvailable" xml:space="preserve">
564564
<value>The request trailers are not available yet. They may not be available until the full request body is read.</value>
565565
</data>
566-
<data name="HTTP2NoTlsOsx" xml:space="preserve">
566+
<data name="Http2NoTlsOsx" xml:space="preserve">
567567
<value>HTTP/2 over TLS is not supported on macOS due to missing ALPN support.</value>
568568
</data>
569569
<data name="Http2StreamResetByApplication" xml:space="preserve">
@@ -602,10 +602,10 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
602602
<data name="HttpsConnectionEstablished" xml:space="preserve">
603603
<value>Connection "{connectionId}" established using the following protocol: {protocol}</value>
604604
</data>
605-
<data name="HTTP2DefaultCiphersInsufficient" xml:space="preserve">
605+
<data name="Http2DefaultCiphersInsufficient" xml:space="preserve">
606606
<value>HTTP/2 over TLS is not supported on Windows versions older than Windows 10 and Windows Server 2016 due to incompatible ciphers or missing ALPN support. Falling back to HTTP/1.1 instead.</value>
607607
</data>
608-
<data name="HTTP2NoTlsWin81" xml:space="preserve">
608+
<data name="Http2NoTlsWin81" xml:space="preserve">
609609
<value>HTTP/2 over TLS is not supported on Windows versions earlier than Windows 10 and Windows Server 2016 due to incompatible ciphers or missing ALPN support.</value>
610610
</data>
611611
<data name="Http2ErrorKeepAliveTimeout" xml:space="preserve">

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal
2525
{
2626
internal class HttpsConnectionMiddleware
2727
{
28+
private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2";
2829
private readonly ConnectionDelegate _next;
2930
private readonly HttpsConnectionAdapterOptions _options;
3031
private readonly ILogger _logger;
3132
private readonly X509Certificate2 _serverCertificate;
3233
private readonly Func<ConnectionContext, string, X509Certificate2> _serverCertificateSelector;
33-
private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2";
3434

3535
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options)
3636
: this(next, options, loggerFactory: NullLoggerFactory.Instance)
@@ -52,31 +52,17 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter
5252
{
5353
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
5454
{
55-
throw new NotSupportedException(CoreStrings.HTTP2NoTlsOsx);
55+
throw new NotSupportedException(CoreStrings.Http2NoTlsOsx);
5656
}
57-
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
57+
else if (IsWindowsVersionIncompatible())
5858
{
59-
var enableHttp2OnWindows81 = AppContext.TryGetSwitch(EnableWindows81Http2, out var enabled) && enabled;
60-
if (Environment.OSVersion.Version < new Version(6, 3)
61-
|| (Environment.OSVersion.Version < new Version(10, 0) && !enableHttp2OnWindows81))
62-
{
63-
throw new NotSupportedException(CoreStrings.HTTP2NoTlsWin81);
64-
}
59+
throw new NotSupportedException(CoreStrings.Http2NoTlsWin81);
6560
}
6661
}
67-
68-
if (options.HttpProtocols == HttpProtocols.Http1AndHttp2)
62+
else if (options.HttpProtocols == HttpProtocols.Http1AndHttp2 && IsWindowsVersionIncompatible())
6963
{
70-
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
71-
{
72-
var enableHttp2OnWindows81 = AppContext.TryGetSwitch(EnableWindows81Http2, out var enabled) && enabled;
73-
if (Environment.OSVersion.Version < new Version(6, 3)
74-
|| (Environment.OSVersion.Version < new Version(10, 0) && !enableHttp2OnWindows81))
75-
{
76-
_logger.HTTP2DefaultCiphersInsufficient();
77-
options.HttpProtocols = HttpProtocols.Http1;
78-
}
79-
}
64+
_logger.Http2DefaultCiphersInsufficient();
65+
options.HttpProtocols = HttpProtocols.Http1;
8066
}
8167

8268
_next = next;
@@ -318,6 +304,21 @@ private static X509Certificate2 ConvertToX509Certificate2(X509Certificate certif
318304

319305
return new X509Certificate2(certificate);
320306
}
307+
308+
private static bool IsWindowsVersionIncompatible()
309+
{
310+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
311+
{
312+
var enableHttp2OnWindows81 = AppContext.TryGetSwitch(EnableWindows81Http2, out var enabled) && enabled;
313+
if (Environment.OSVersion.Version < new Version(6, 3)
314+
|| (Environment.OSVersion.Version < new Version(10, 0) && !enableHttp2OnWindows81))
315+
{
316+
return true;
317+
}
318+
}
319+
320+
return false;
321+
}
321322
}
322323

323324
internal static class HttpsConnectionMiddlewareLoggerExtensions
@@ -344,15 +345,15 @@ internal static class HttpsConnectionMiddlewareLoggerExtensions
344345
private static readonly Action<ILogger, Exception> _http2DefaultCiphersInsufficient =
345346
LoggerMessage.Define(
346347
logLevel: LogLevel.Information,
347-
eventId: new EventId(4, "HTTP2DefaultCiphersInsufficient"),
348-
formatString: CoreStrings.HTTP2DefaultCiphersInsufficient);
348+
eventId: new EventId(4, "Http2DefaultCiphersInsufficient"),
349+
formatString: CoreStrings.Http2DefaultCiphersInsufficient);
349350

350351
public static void AuthenticationFailed(this ILogger logger, Exception exception) => _authenticationFailed(logger, exception);
351352

352353
public static void AuthenticationTimedOut(this ILogger logger) => _authenticationTimedOut(logger, null);
353354

354355
public static void HttpsConnectionEstablished(this ILogger logger, string connectionId, SslProtocols sslProtocol) => _httpsConnectionEstablished(logger, connectionId, sslProtocol, null);
355356

356-
public static void HTTP2DefaultCiphersInsufficient(this ILogger logger) => _http2DefaultCiphersInsufficient(logger, null);
357+
public static void Http2DefaultCiphersInsufficient(this ILogger logger) => _http2DefaultCiphersInsufficient(logger, null);
357358
}
358359
}

src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,35 @@ public void Http1AndHttp2DoesNotDowngradeOnCompatibleWindowsVersions()
653653
Assert.Equal(HttpProtocols.Http1AndHttp2, httpConnectionAdapterOptions.HttpProtocols);
654654
}
655655

656+
[ConditionalFact]
657+
[OSSkipCondition(OperatingSystems.MacOSX | OperatingSystems.Linux, SkipReason = "Error logic only applies on Windows")]
658+
[MaximumOSVersion(OperatingSystems.Windows, WindowsVersions.Win81)]
659+
public void Http2ThrowsOnIncompatibleWindowsVersions()
660+
{
661+
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
662+
{
663+
ServerCertificate = _x509Certificate2,
664+
HttpProtocols = HttpProtocols.Http2
665+
};
666+
667+
Assert.Throws<NotSupportedException>(() => new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions));
668+
}
669+
670+
[ConditionalFact]
671+
[OSSkipCondition(OperatingSystems.MacOSX | OperatingSystems.Linux, SkipReason = "Error logic only applies on Windows")]
672+
[MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win10)]
673+
public void Http2DoesNotThrowOnCompatibleWindowsVersions()
674+
{
675+
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
676+
{
677+
ServerCertificate = _x509Certificate2,
678+
HttpProtocols = HttpProtocols.Http2
679+
};
680+
681+
// Does not throw
682+
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
683+
}
684+
656685
private static async Task App(HttpContext httpContext)
657686
{
658687
var request = httpContext.Request;

0 commit comments

Comments
 (0)