From 211f5b324a3272138e1acccbe26d2e63a760ee6f Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 6 May 2021 13:29:28 -0700 Subject: [PATCH 01/14] Initial WebSocket compression implementation --- .../WebSockets/src/WebSocketMiddleware.cs | 179 +++++++++++++++++- .../UnitTests/WebSocketMiddlewareTests.cs | 35 ++++ 2 files changed, 213 insertions(+), 1 deletion(-) diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index 72fbc077b33f..5024d8e5d126 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -3,9 +3,13 @@ using System; using System.Collections.Generic; +using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; +using System.Net.Http.Headers; using System.Net.WebSockets; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -151,9 +155,46 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) HandshakeHelpers.GenerateResponseHeaders(key, subProtocol, _context.Response.Headers); + // TODO: get from options + WebSocketDeflateOptions? deflateOptions = null; + var ext = _context.Request.Headers["Sec-WebSocket-Extensions"]; + if (ext.Count != 0) + { + var decline = false; + foreach (var extension in ext) + { + if (extension.TrimStart().StartsWith(ClientWebSocketDeflateConstants.Extension, StringComparison.Ordinal)) + { + deflateOptions = new(); + if (ParseDeflateOptions(extension, deflateOptions, out var hasClientMaxWindowBits)) + { + Resp(_context.Response.Headers, deflateOptions, hasClientMaxWindowBits); + decline = false; + break; + } + else + { + decline = true; + } + } + } + if (decline) + { + throw new InvalidOperationException("'permessage-deflate' extension not accepted."); + } + } + Stream opaqueTransport = await _upgradeFeature.UpgradeAsync(); // Sets status code to 101 - return WebSocket.CreateFromStream(opaqueTransport, isServer: true, subProtocol: subProtocol, keepAliveInterval: keepAliveInterval); + var options = new WebSocketCreationOptions() + { + IsServer = true, + KeepAliveInterval = keepAliveInterval, + SubProtocol = subProtocol, + DangerousDeflateOptions = deflateOptions, + }; + + return WebSocket.CreateFromStream(opaqueTransport, options); } public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictionary requestHeaders) @@ -226,6 +267,142 @@ public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictiona return HandshakeHelpers.IsRequestKeyValid(requestHeaders.SecWebSocketKey.ToString()); } + + internal static class ClientWebSocketDeflateConstants + { + /// + /// The maximum length that this extension can have, assuming that we're not abusing white space. + /// + /// "permessage-deflate; client_max_window_bits=15; client_no_context_takeover; server_max_window_bits=15; server_no_context_takeover" + /// + public const int MaxExtensionLength = 128; + + public const string Extension = "permessage-deflate"; + + public const string ClientMaxWindowBits = "client_max_window_bits"; + public const string ClientNoContextTakeover = "client_no_context_takeover"; + + public const string ServerMaxWindowBits = "server_max_window_bits"; + public const string ServerNoContextTakeover = "server_no_context_takeover"; + } + + private static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions options, out bool hasClientMaxWindowBits) + { + hasClientMaxWindowBits = false; + while (true) + { + int end = extension.IndexOf(';'); + ReadOnlySpan value = (end >= 0 ? extension[..end] : extension).Trim(); + + if (value.Length > 0) + { + if (value.SequenceEqual(ClientWebSocketDeflateConstants.ClientNoContextTakeover)) + { + options.ClientContextTakeover = false; + } + else if (value.SequenceEqual(ClientWebSocketDeflateConstants.ServerNoContextTakeover)) + { + options.ServerContextTakeover = false; + } + else if (value.StartsWith(ClientWebSocketDeflateConstants.ClientMaxWindowBits)) + { + hasClientMaxWindowBits = true; + var clientMaxWindowBits = ParseWindowBits(value); + if (clientMaxWindowBits > options.ClientMaxWindowBits) + { + return false; + } + options.ClientMaxWindowBits = clientMaxWindowBits; + } + else if (value.StartsWith(ClientWebSocketDeflateConstants.ServerMaxWindowBits)) + { + var serverMaxWindowBits = ParseWindowBits(value); + if (serverMaxWindowBits > options.ServerMaxWindowBits) + { + return false; + } + options.ServerMaxWindowBits = serverMaxWindowBits; + } + + static int ParseWindowBits(ReadOnlySpan value) + { + // parameters can be sent without a value by the client + var startIndex = value.IndexOf('='); + + if (startIndex < 0 || + !int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || + windowBits < 9 || + windowBits > 15) + { + throw new WebSocketException(WebSocketError.HeaderError, ""); + } + + return windowBits; + } + } + + if (end < 0) + { + break; + } + extension = extension[(end + 1)..]; + } + + return true; + } + + private static void Resp(IHeaderDictionary headers, WebSocketDeflateOptions options, bool hasClientMaxWindowBits) + { + headers.Add("Sec-WebSocket-Extensions", GetDeflateOptions(options, hasClientMaxWindowBits)); + + static string GetDeflateOptions(WebSocketDeflateOptions options, bool hasClientMaxWindowBits) + { + var builder = new StringBuilder(ClientWebSocketDeflateConstants.MaxExtensionLength); + builder.Append(ClientWebSocketDeflateConstants.Extension); + + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. + // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 + if (hasClientMaxWindowBits) + { + if (options.ClientMaxWindowBits != 15) + { + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits).Append('=') + .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + else + { + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits); + } + } + + if (!options.ClientContextTakeover) + { + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientNoContextTakeover); + } + + if (options.ServerMaxWindowBits != 15) + { + builder.Append("; ") + .Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits).Append('=') + .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + else + { + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits); + } + + if (!options.ServerContextTakeover) + { + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ServerNoContextTakeover); + } + + Debug.Assert(builder.Length <= ClientWebSocketDeflateConstants.MaxExtensionLength); + return builder.ToString(); + } + } } } } diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs index 1fb5686a909b..76ab48dd2b6d 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Linq; using System.Net; using System.Net.Http; using System.Net.WebSockets; @@ -632,5 +633,39 @@ public async Task MultipleValueHeadersNotOverridden() } } } + + [Fact] + public async Task InitialCompression() + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + request.Headers.Connection.Clear(); + request.Headers.Connection.Add("Upgrade"); + request.Headers.Connection.Add("keep-alive"); + request.Headers.Upgrade.Add(new System.Net.Http.Headers.ProductHeaderValue("websocket")); + request.Headers.Add(HeaderNames.SecWebSocketVersion, "13"); + // SecWebSocketKey required to be 16 bytes + request.Headers.Add(HeaderNames.SecWebSocketKey, Convert.ToBase64String(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }, Base64FormattingOptions.None)); + request.Headers.Add("Sec-WebSocket-Extensions", "permessage-deflate"); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.Equal("permessage-deflate; server_max_window_bits", response.Headers.GetValues("Sec-WebSocket-Extensions").Aggregate((l, r) => $"{l}; {r}")); + } + } + } + } } } From 77f7252ad8285053e0ae5baea2331c41ee5872ea Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Mon, 10 May 2021 14:23:01 -0700 Subject: [PATCH 02/14] fix and some tests --- .../WebSockets/src/WebSocketMiddleware.cs | 54 +++++++++---------- .../UnitTests/WebSocketMiddlewareTests.cs | 16 ++++-- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index 5024d8e5d126..fa34a3a2036a 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -161,14 +161,15 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) if (ext.Count != 0) { var decline = false; - foreach (var extension in ext) + // loop over each extension offer, extensions can have multiple offers we can accept any + foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues("Sec-WebSocket-Extensions")) { if (extension.TrimStart().StartsWith(ClientWebSocketDeflateConstants.Extension, StringComparison.Ordinal)) { deflateOptions = new(); if (ParseDeflateOptions(extension, deflateOptions, out var hasClientMaxWindowBits)) { - Resp(_context.Response.Headers, deflateOptions, hasClientMaxWindowBits); + WriteDeflatNegotiateResponseHeader(_context.Response.Headers, deflateOptions, hasClientMaxWindowBits); decline = false; break; } @@ -180,7 +181,8 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) } if (decline) { - throw new InvalidOperationException("'permessage-deflate' extension not accepted."); + // TODO: Do we care? + throw new WebSocketException(WebSocketError.HeaderError, "'permessage-deflate' extension not accepted."); } } @@ -312,7 +314,8 @@ private static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketD { return false; } - options.ClientMaxWindowBits = clientMaxWindowBits; + // if client didn't send a value for ClientMaxWindowBits use the value the server set + options.ClientMaxWindowBits = clientMaxWindowBits ?? options.ClientMaxWindowBits; } else if (value.StartsWith(ClientWebSocketDeflateConstants.ServerMaxWindowBits)) { @@ -321,19 +324,25 @@ private static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketD { return false; } - options.ServerMaxWindowBits = serverMaxWindowBits; + // if client didn't send a value for ServerMaxWindowBits use the value the server set + options.ServerMaxWindowBits = serverMaxWindowBits ?? options.ServerMaxWindowBits; } - static int ParseWindowBits(ReadOnlySpan value) + static int? ParseWindowBits(ReadOnlySpan value) { - // parameters can be sent without a value by the client var startIndex = value.IndexOf('='); - if (startIndex < 0 || - !int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || + // parameters can be sent without a value by the client + if (startIndex < 0) + { + return null; + } + + if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || windowBits < 9 || windowBits > 15) { + // TODO throw new WebSocketException(WebSocketError.HeaderError, ""); } @@ -351,7 +360,7 @@ static int ParseWindowBits(ReadOnlySpan value) return true; } - private static void Resp(IHeaderDictionary headers, WebSocketDeflateOptions options, bool hasClientMaxWindowBits) + private static void WriteDeflatNegotiateResponseHeader(IHeaderDictionary headers, WebSocketDeflateOptions options, bool hasClientMaxWindowBits) { headers.Add("Sec-WebSocket-Extensions", GetDeflateOptions(options, hasClientMaxWindowBits)); @@ -367,15 +376,8 @@ static string GetDeflateOptions(WebSocketDeflateOptions options, bool hasClientM // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 if (hasClientMaxWindowBits) { - if (options.ClientMaxWindowBits != 15) - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits).Append('=') - .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); - } - else - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits); - } + builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits).Append('=') + .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); } if (!options.ClientContextTakeover) @@ -383,16 +385,10 @@ static string GetDeflateOptions(WebSocketDeflateOptions options, bool hasClientM builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientNoContextTakeover); } - if (options.ServerMaxWindowBits != 15) - { - builder.Append("; ") - .Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits).Append('=') - .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); - } - else - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits); - } + // TODO: we could avoid sending this in some cases + builder.Append("; ") + .Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits).Append('=') + .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); if (!options.ServerContextTakeover) { diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs index 76ab48dd2b6d..7bab6130096b 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs @@ -634,8 +634,16 @@ public async Task MultipleValueHeadersNotOverridden() } } - [Fact] - public async Task InitialCompression() + [Theory] + [InlineData("permessage-deflate", "permessage-deflate; server_max_window_bits=15")] + [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_max_window_bits=15; server_no_context_takeover")] + [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover; server_max_window_bits=15")] + [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9; server_max_window_bits=15")] + [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15; server_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] + public async Task CompressionNegotiationProducesCorrectHeader(string clientHeader, string expectedResponse) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { @@ -658,11 +666,11 @@ public async Task InitialCompression() request.Headers.Add(HeaderNames.SecWebSocketVersion, "13"); // SecWebSocketKey required to be 16 bytes request.Headers.Add(HeaderNames.SecWebSocketKey, Convert.ToBase64String(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }, Base64FormattingOptions.None)); - request.Headers.Add("Sec-WebSocket-Extensions", "permessage-deflate"); + request.Headers.Add("Sec-WebSocket-Extensions", clientHeader); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.Equal("permessage-deflate; server_max_window_bits", response.Headers.GetValues("Sec-WebSocket-Extensions").Aggregate((l, r) => $"{l}; {r}")); + Assert.Equal(expectedResponse, response.Headers.GetValues("Sec-WebSocket-Extensions").Aggregate((l, r) => $"{l}; {r}")); } } } From 57437c860afaaa51d7427f9255f11691622e34aa Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 11 May 2021 16:04:54 -0700 Subject: [PATCH 03/14] More compression --- src/Http/Headers/src/HeaderNames.cs | 3 + src/Http/Headers/src/PublicAPI.Unshipped.txt | 1 + .../src/PublicAPI.Unshipped.txt | 1 + .../Http.Abstractions/src/WebSocketManager.cs | 10 +- .../src/IHeaderDictionary.Keyed.cs | 3 + .../Http.Features/src/PublicAPI.Unshipped.txt | 2 + .../src/Internal/DefaultWebSocketManager.cs | 7 +- .../src/ExtendedWebSocketAcceptContext.cs | 6 + .../WebSockets/src/HandshakeHelpers.cs | 87 +++++ .../WebSockets/src/PublicAPI.Unshipped.txt | 2 + .../src/WebSocketDeflateConstants.cs | 23 ++ .../WebSockets/src/WebSocketMiddleware.cs | 236 +++++------- .../WebSocketCompressionMiddlewareTests.cs | 342 ++++++++++++++++++ .../UnitTests/WebSocketMiddlewareTests.cs | 81 +---- 14 files changed, 586 insertions(+), 218 deletions(-) create mode 100644 src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs create mode 100644 src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs diff --git a/src/Http/Headers/src/HeaderNames.cs b/src/Http/Headers/src/HeaderNames.cs index 8b1b8af991b3..2528c481cd6f 100644 --- a/src/Http/Headers/src/HeaderNames.cs +++ b/src/Http/Headers/src/HeaderNames.cs @@ -225,6 +225,9 @@ public static class HeaderNames /// Gets the Sec-WebSocket-Version HTTP header name. public static readonly string SecWebSocketVersion = "Sec-WebSocket-Version"; + /// Gets the Sec-WebSocket-Extensions HTTP header name. + public static readonly string SecWebSocketExtensions = "Sec-WebSocket-Extensions"; + /// Gets the Server HTTP header name. public static readonly string Server = "Server"; diff --git a/src/Http/Headers/src/PublicAPI.Unshipped.txt b/src/Http/Headers/src/PublicAPI.Unshipped.txt index 9035747ce6c4..b5ecbe22ebbb 100644 --- a/src/Http/Headers/src/PublicAPI.Unshipped.txt +++ b/src/Http/Headers/src/PublicAPI.Unshipped.txt @@ -5,6 +5,7 @@ Microsoft.Net.Http.Headers.RangeConditionHeaderValue.RangeConditionHeaderValue(M static readonly Microsoft.Net.Http.Headers.HeaderNames.Baggage -> string! static readonly Microsoft.Net.Http.Headers.HeaderNames.Link -> string! static readonly Microsoft.Net.Http.Headers.HeaderNames.ProxyConnection -> string! +static readonly Microsoft.Net.Http.Headers.HeaderNames.SecWebSocketExtensions -> string! static readonly Microsoft.Net.Http.Headers.HeaderNames.XContentTypeOptions -> string! static readonly Microsoft.Net.Http.Headers.HeaderNames.XPoweredBy -> string! static readonly Microsoft.Net.Http.Headers.HeaderNames.XUACompatible -> string! diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt index 687f1f1e034b..0d6ad2ae7320 100644 --- a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt @@ -23,3 +23,4 @@ abstract Microsoft.AspNetCore.Http.HttpRequest.ContentType.get -> string? static Microsoft.AspNetCore.Builder.UseExtensions.Use(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, System.Func! middleware) -> Microsoft.AspNetCore.Builder.IApplicationBuilder! static Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.UseMiddleware(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, System.Type! middleware, params object?[]! args) -> Microsoft.AspNetCore.Builder.IApplicationBuilder! static Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.UseMiddleware(this Microsoft.AspNetCore.Builder.IApplicationBuilder! app, params object?[]! args) -> Microsoft.AspNetCore.Builder.IApplicationBuilder! +virtual Microsoft.AspNetCore.Http.WebSocketManager.AcceptWebSocketAsync(Microsoft.AspNetCore.Http.WebSocketAcceptContext! acceptContext) -> System.Threading.Tasks.Task! diff --git a/src/Http/Http.Abstractions/src/WebSocketManager.cs b/src/Http/Http.Abstractions/src/WebSocketManager.cs index 1fe47d59587d..79fd84bc184c 100644 --- a/src/Http/Http.Abstractions/src/WebSocketManager.cs +++ b/src/Http/Http.Abstractions/src/WebSocketManager.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Net.WebSockets; using System.Threading.Tasks; @@ -8,7 +9,7 @@ namespace Microsoft.AspNetCore.Http { /// - /// Manages the establishment of WebSocket connections for a specific HTTP request. + /// Manages the establishment of WebSocket connections for a specific HTTP request. /// public abstract class WebSocketManager { @@ -37,5 +38,12 @@ public virtual Task AcceptWebSocketAsync() /// The sub-protocol to use. /// A task representing the completion of the transition. public abstract Task AcceptWebSocketAsync(string? subProtocol); + + /// + /// + /// + /// + /// + public virtual Task AcceptWebSocketAsync(WebSocketAcceptContext acceptContext) => throw new NotImplementedException(); } } diff --git a/src/Http/Http.Features/src/IHeaderDictionary.Keyed.cs b/src/Http/Http.Features/src/IHeaderDictionary.Keyed.cs index 7f7739599ad9..fd98c8ae4d1d 100644 --- a/src/Http/Http.Features/src/IHeaderDictionary.Keyed.cs +++ b/src/Http/Http.Features/src/IHeaderDictionary.Keyed.cs @@ -202,6 +202,9 @@ public partial interface IHeaderDictionary /// Gets or sets the Sec-WebSocket-Version HTTP header. StringValues SecWebSocketVersion { get => this[HeaderNames.SecWebSocketVersion]; set => this[HeaderNames.SecWebSocketVersion] = value; } + /// Gets or sets the Sec-WebSocket-Extensions HTTP header. + StringValues SecWebSocketExtensions { get => this[HeaderNames.SecWebSocketExtensions]; set => this[HeaderNames.SecWebSocketExtensions] = value; } + /// Gets or sets the Server HTTP header. StringValues Server { get => this[HeaderNames.Server]; set => this[HeaderNames.Server] = value; } diff --git a/src/Http/Http.Features/src/PublicAPI.Unshipped.txt b/src/Http/Http.Features/src/PublicAPI.Unshipped.txt index 6911dbe5b627..7fe313271429 100644 --- a/src/Http/Http.Features/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Features/src/PublicAPI.Unshipped.txt @@ -161,6 +161,8 @@ Microsoft.AspNetCore.Http.IHeaderDictionary.RetryAfter.get -> Microsoft.Extensio Microsoft.AspNetCore.Http.IHeaderDictionary.RetryAfter.set -> void Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketAccept.get -> Microsoft.Extensions.Primitives.StringValues Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketAccept.set -> void +Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketExtensions.get -> Microsoft.Extensions.Primitives.StringValues +Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketExtensions.set -> void Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketKey.get -> Microsoft.Extensions.Primitives.StringValues Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketKey.set -> void Microsoft.AspNetCore.Http.IHeaderDictionary.SecWebSocketProtocol.get -> Microsoft.Extensions.Primitives.StringValues diff --git a/src/Http/Http/src/Internal/DefaultWebSocketManager.cs b/src/Http/Http/src/Internal/DefaultWebSocketManager.cs index 43ad4c0907ea..1645351159fd 100644 --- a/src/Http/Http/src/Internal/DefaultWebSocketManager.cs +++ b/src/Http/Http/src/Internal/DefaultWebSocketManager.cs @@ -61,12 +61,17 @@ public override IList WebSocketRequestedProtocols } public override Task AcceptWebSocketAsync(string? subProtocol) + { + return AcceptWebSocketAsync(new WebSocketAcceptContext() { SubProtocol = subProtocol }); + } + + public override Task AcceptWebSocketAsync(WebSocketAcceptContext acceptContext) { if (WebSocketFeature == null) { throw new NotSupportedException("WebSockets are not supported"); } - return WebSocketFeature.AcceptAsync(new WebSocketAcceptContext() { SubProtocol = subProtocol }); + return WebSocketFeature.AcceptAsync(acceptContext); } struct FeatureInterfaces diff --git a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs index 7bfd7de4963a..8cfced631c7b 100644 --- a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs +++ b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Net.WebSockets; using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.WebSockets @@ -24,5 +25,10 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext /// The interval to send pong frames. This is a heart-beat that keeps the connection alive. /// public TimeSpan? KeepAliveInterval { get; set; } + + /// + /// + /// + public WebSocketCreationOptions? WebSocketOptions { get; set; } } } diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index 05c5ac5363a3..fe817f5967e3 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -2,6 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Net.WebSockets; using System.Security.Cryptography; using System.Text; using Microsoft.AspNetCore.Http; @@ -72,5 +75,89 @@ public static string CreateResponseKey(string requestKey) return Convert.ToBase64String(hashedBytes); } + + public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions options, [NotNullWhen(true)] out string? response) + { + response = null; + var builder = new StringBuilder(WebSocketDeflateConstants.MaxExtensionLength); + builder.Append(WebSocketDeflateConstants.Extension); + + while (true) + { + int end = extension.IndexOf(';'); + ReadOnlySpan value = (end >= 0 ? extension[..end] : extension).Trim(); + + if (value.Length > 0) + { + if (value.SequenceEqual(WebSocketDeflateConstants.ClientNoContextTakeover)) + { + // REVIEW: If someone specifies true for server options, do we allow client to override this? + options.ClientContextTakeover = false; + builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover); + } + else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) + { + options.ServerContextTakeover = false; + builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover); + } + else if (value.StartsWith(WebSocketDeflateConstants.ClientMaxWindowBits)) + { + var clientMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ClientMaxWindowBits); + if (clientMaxWindowBits > options.ClientMaxWindowBits) + { + return false; + } + // if client didn't send a value for ClientMaxWindowBits use the value the server set + options.ClientMaxWindowBits = clientMaxWindowBits ?? options.ClientMaxWindowBits; + builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=') + .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) + { + var serverMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ServerMaxWindowBits); + if (serverMaxWindowBits > options.ServerMaxWindowBits) + { + return false; + } + // if client didn't send a value for ServerMaxWindowBits use the value the server set + options.ServerMaxWindowBits = serverMaxWindowBits ?? options.ServerMaxWindowBits; + + builder.Append("; ") + .Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=') + .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + + static int? ParseWindowBits(ReadOnlySpan value, string propertyName) + { + var startIndex = value.IndexOf('='); + + // parameters can be sent without a value by the client, we'll use the values set by the app developer or the default of 15 + if (startIndex < 0) + { + return null; + } + + if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || + windowBits < 9 || + windowBits > 15) + { + throw new WebSocketException(WebSocketError.HeaderError, $"invalid {propertyName} used: {value[(startIndex + 1)..].ToString()}"); + } + + return windowBits; + } + } + + if (end < 0) + { + break; + } + extension = extension[(end + 1)..]; + } + + response = builder.ToString(); + + return true; + } } } diff --git a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt index cdb19672009b..d901bbf0bbe0 100644 --- a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.AspNetCore.Builder.WebSocketOptions.AllowedOrigins.get -> System.Collections.Generic.IList! +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.get -> System.Net.WebSockets.WebSocketCreationOptions? +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.set -> void Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task! ~Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.WebSocketMiddleware(Microsoft.AspNetCore.Http.RequestDelegate! next, Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.get -> string? diff --git a/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs new file mode 100644 index 000000000000..aff0a93dccf4 --- /dev/null +++ b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs @@ -0,0 +1,23 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.WebSockets +{ + internal static class WebSocketDeflateConstants + { + /// + /// The maximum length that this extension can have, assuming that we're not using extra white space. + /// + /// "permessage-deflate; client_max_window_bits=15; client_no_context_takeover; server_max_window_bits=15; server_no_context_takeover" + /// + public const int MaxExtensionLength = 128; + + public const string Extension = "permessage-deflate"; + + public const string ClientMaxWindowBits = "client_max_window_bits"; + public const string ClientNoContextTakeover = "client_no_context_takeover"; + + public const string ServerMaxWindowBits = "server_max_window_bits"; + public const string ServerNoContextTakeover = "server_no_context_takeover"; + } +} diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index fa34a3a2036a..32c13db992f8 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; using System.Linq; @@ -143,62 +144,119 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) } TimeSpan keepAliveInterval = _options.KeepAliveInterval; + WebSocketCreationOptions? creationOptions = null; if (acceptContext is ExtendedWebSocketAcceptContext advancedAcceptContext) { if (advancedAcceptContext.KeepAliveInterval.HasValue) { keepAliveInterval = advancedAcceptContext.KeepAliveInterval.Value; } + if (advancedAcceptContext.WebSocketOptions is not null) + { + creationOptions = advancedAcceptContext.WebSocketOptions; + } } string key = _context.Request.Headers.SecWebSocketKey; HandshakeHelpers.GenerateResponseHeaders(key, subProtocol, _context.Response.Headers); - // TODO: get from options WebSocketDeflateOptions? deflateOptions = null; - var ext = _context.Request.Headers["Sec-WebSocket-Extensions"]; + var ext = _context.Request.Headers.SecWebSocketExtensions; if (ext.Count != 0) { - var decline = false; // loop over each extension offer, extensions can have multiple offers we can accept any - foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues("Sec-WebSocket-Extensions")) + foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions)) { - if (extension.TrimStart().StartsWith(ClientWebSocketDeflateConstants.Extension, StringComparison.Ordinal)) + if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal) + && creationOptions?.DangerousDeflateOptions is not null) { - deflateOptions = new(); - if (ParseDeflateOptions(extension, deflateOptions, out var hasClientMaxWindowBits)) + // We do not want to modify the users options + deflateOptions = CloneWebSocketDeflateOptions(creationOptions.DangerousDeflateOptions); + if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions!, out var response)) { - WriteDeflatNegotiateResponseHeader(_context.Response.Headers, deflateOptions, hasClientMaxWindowBits); - decline = false; + if (CompareDeflateOptions(deflateOptions, creationOptions.DangerousDeflateOptions)) + { + // avoids allocating a new WebSocketCreationOptions below when checking if we have new deflate options to apply + deflateOptions = null; + } + // If more extension types are added, this would need to be a header append + // and we wouldn't want to break out of the loop + _context.Response.Headers.SecWebSocketExtensions = response; break; } - else - { - decline = true; - } } } - if (decline) - { - // TODO: Do we care? - throw new WebSocketException(WebSocketError.HeaderError, "'permessage-deflate' extension not accepted."); - } } Stream opaqueTransport = await _upgradeFeature.UpgradeAsync(); // Sets status code to 101 - var options = new WebSocketCreationOptions() + WebSocketCreationOptions? options = creationOptions; + if (options is null) { - IsServer = true, - KeepAliveInterval = keepAliveInterval, - SubProtocol = subProtocol, - DangerousDeflateOptions = deflateOptions, - }; + options = new WebSocketCreationOptions() + { + IsServer = true, + KeepAliveInterval = keepAliveInterval, + SubProtocol = subProtocol, + DangerousDeflateOptions = deflateOptions, + }; + } + else if (deflateOptions is not null) + { + // use a new options instance so we don't modify the users options + options = new WebSocketCreationOptions() + { + IsServer = true, + KeepAliveInterval = creationOptions!.KeepAliveInterval, + SubProtocol = creationOptions!.SubProtocol, + DangerousDeflateOptions = deflateOptions, + }; + } + else if (options.IsServer == false) + { + // use a new options instance so we don't modify the users options + options = new WebSocketCreationOptions() + { + IsServer = true, + KeepAliveInterval = creationOptions!.KeepAliveInterval, + SubProtocol = creationOptions!.SubProtocol, + DangerousDeflateOptions = deflateOptions, + }; + } return WebSocket.CreateFromStream(opaqueTransport, options); } + private static WebSocketDeflateOptions? CloneWebSocketDeflateOptions([NotNullIfNotNull("options")] WebSocketDeflateOptions? options) + { + if (options is null) + { + return null; + } + + return new WebSocketDeflateOptions() + { + ClientContextTakeover = options.ClientContextTakeover, + ClientMaxWindowBits = options.ClientMaxWindowBits, + ServerContextTakeover = options.ServerContextTakeover, + ServerMaxWindowBits = options.ServerMaxWindowBits + }; + } + + private static bool CompareDeflateOptions(WebSocketDeflateOptions? lhs, WebSocketDeflateOptions? rhs) + { + if (lhs is null || rhs is null) + { + return lhs == rhs; + } + + return lhs.ClientContextTakeover == rhs.ClientContextTakeover && + lhs.ClientMaxWindowBits == rhs.ClientMaxWindowBits && + lhs.ServerContextTakeover == rhs.ServerContextTakeover && + lhs.ServerMaxWindowBits == rhs.ServerMaxWindowBits; + } + public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictionary requestHeaders) { if (!HttpMethods.IsGet(method)) @@ -269,136 +327,6 @@ public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictiona return HandshakeHelpers.IsRequestKeyValid(requestHeaders.SecWebSocketKey.ToString()); } - - internal static class ClientWebSocketDeflateConstants - { - /// - /// The maximum length that this extension can have, assuming that we're not abusing white space. - /// - /// "permessage-deflate; client_max_window_bits=15; client_no_context_takeover; server_max_window_bits=15; server_no_context_takeover" - /// - public const int MaxExtensionLength = 128; - - public const string Extension = "permessage-deflate"; - - public const string ClientMaxWindowBits = "client_max_window_bits"; - public const string ClientNoContextTakeover = "client_no_context_takeover"; - - public const string ServerMaxWindowBits = "server_max_window_bits"; - public const string ServerNoContextTakeover = "server_no_context_takeover"; - } - - private static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions options, out bool hasClientMaxWindowBits) - { - hasClientMaxWindowBits = false; - while (true) - { - int end = extension.IndexOf(';'); - ReadOnlySpan value = (end >= 0 ? extension[..end] : extension).Trim(); - - if (value.Length > 0) - { - if (value.SequenceEqual(ClientWebSocketDeflateConstants.ClientNoContextTakeover)) - { - options.ClientContextTakeover = false; - } - else if (value.SequenceEqual(ClientWebSocketDeflateConstants.ServerNoContextTakeover)) - { - options.ServerContextTakeover = false; - } - else if (value.StartsWith(ClientWebSocketDeflateConstants.ClientMaxWindowBits)) - { - hasClientMaxWindowBits = true; - var clientMaxWindowBits = ParseWindowBits(value); - if (clientMaxWindowBits > options.ClientMaxWindowBits) - { - return false; - } - // if client didn't send a value for ClientMaxWindowBits use the value the server set - options.ClientMaxWindowBits = clientMaxWindowBits ?? options.ClientMaxWindowBits; - } - else if (value.StartsWith(ClientWebSocketDeflateConstants.ServerMaxWindowBits)) - { - var serverMaxWindowBits = ParseWindowBits(value); - if (serverMaxWindowBits > options.ServerMaxWindowBits) - { - return false; - } - // if client didn't send a value for ServerMaxWindowBits use the value the server set - options.ServerMaxWindowBits = serverMaxWindowBits ?? options.ServerMaxWindowBits; - } - - static int? ParseWindowBits(ReadOnlySpan value) - { - var startIndex = value.IndexOf('='); - - // parameters can be sent without a value by the client - if (startIndex < 0) - { - return null; - } - - if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || - windowBits < 9 || - windowBits > 15) - { - // TODO - throw new WebSocketException(WebSocketError.HeaderError, ""); - } - - return windowBits; - } - } - - if (end < 0) - { - break; - } - extension = extension[(end + 1)..]; - } - - return true; - } - - private static void WriteDeflatNegotiateResponseHeader(IHeaderDictionary headers, WebSocketDeflateOptions options, bool hasClientMaxWindowBits) - { - headers.Add("Sec-WebSocket-Extensions", GetDeflateOptions(options, hasClientMaxWindowBits)); - - static string GetDeflateOptions(WebSocketDeflateOptions options, bool hasClientMaxWindowBits) - { - var builder = new StringBuilder(ClientWebSocketDeflateConstants.MaxExtensionLength); - builder.Append(ClientWebSocketDeflateConstants.Extension); - - // If a received extension negotiation offer doesn't have the - // "client_max_window_bits" extension parameter, the corresponding - // extension negotiation response to the offer MUST NOT include the - // "client_max_window_bits" extension parameter. - // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 - if (hasClientMaxWindowBits) - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientMaxWindowBits).Append('=') - .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); - } - - if (!options.ClientContextTakeover) - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ClientNoContextTakeover); - } - - // TODO: we could avoid sending this in some cases - builder.Append("; ") - .Append(ClientWebSocketDeflateConstants.ServerMaxWindowBits).Append('=') - .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); - - if (!options.ServerContextTakeover) - { - builder.Append("; ").Append(ClientWebSocketDeflateConstants.ServerNoContextTakeover); - } - - Debug.Assert(builder.Length <= ClientWebSocketDeflateConstants.MaxExtensionLength); - return builder.ToString(); - } - } } } } diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs new file mode 100644 index 000000000000..3f50a73e088b --- /dev/null +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs @@ -0,0 +1,342 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.WebSockets; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Testing; +using Microsoft.Net.Http.Headers; +using Xunit; + +namespace Microsoft.AspNetCore.WebSockets.Test +{ + public class WebSocketCompressionMiddlewareTests : LoggedTest + { + [Theory] + [InlineData("permessage-deflate", "permessage-deflate")] + [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover")] + [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover")] + [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9")] + [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12")] + public async Task CompressionNegotiationProducesCorrectHeader(string clientHeader, string expectedResponse) + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = new WebSocketCreationOptions() { DangerousDeflateOptions = new WebSocketDeflateOptions() } + }); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); + } + } + } + } + + [Fact] + public async Task CompressionNegotiationIgnoredIfNotEnabledOnServer() + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, "permessage-deflate"); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.False(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); + } + } + } + } + + [Theory] + [InlineData("permessage-deflate; server_max_window_bits=12")] + [InlineData("permessage-deflate; client_max_window_bits=12")] + public async Task CompressionNegotiateNotAccepted(string clientHeader) + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = new WebSocketCreationOptions() + { + DangerousDeflateOptions = new WebSocketDeflateOptions() + { + ClientMaxWindowBits = 11, + ServerMaxWindowBits = 11, + } + } + }); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.False(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); + } + } + } + } + + [Theory] + [InlineData("permessage-deflate; server_max_window_bits=16", "invalid server_max_window_bits used: 16")] + [InlineData("permessage-deflate; server_max_window_bits=8", "invalid server_max_window_bits used: 8")] + [InlineData("permessage-deflate; client_max_window_bits=16", "invalid client_max_window_bits used: 16")] + [InlineData("permessage-deflate; client_max_window_bits=8", "invalid client_max_window_bits used: 8")] + public async Task InvalidCompressionNegotiateThrows(string clientHeader, string errorMessage) + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var ex = await Assert.ThrowsAsync(() => context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = new WebSocketCreationOptions() + { + DangerousDeflateOptions = new WebSocketDeflateOptions(), + } + })); + Assert.Equal(errorMessage, ex.Message); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + + var response = await client.SendAsync(request); + } + } + } + } + + [Theory] + [InlineData("permessage-deflate; server_max_window_bits=15, permessage-deflate; server_max_window_bits=12", "permessage-deflate; server_max_window_bits=12")] + [InlineData("permessage-deflate; server_max_window_bits=11, permessage-deflate; server_max_window_bits=12", "permessage-deflate; server_max_window_bits=11")] + [InlineData("permessage-deflate; client_max_window_bits=15, permessage-deflate; client_max_window_bits=12", "permessage-deflate; client_max_window_bits=12")] + [InlineData("permessage-deflate; client_max_window_bits=11, permessage-deflate; client_max_window_bits=14", "permessage-deflate; client_max_window_bits=11")] + public async Task CompressionNegotiationCanChooseExtension(string clientHeader, string expectedResponse) + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = new WebSocketCreationOptions() + { + DangerousDeflateOptions = new WebSocketDeflateOptions() + { + ServerMaxWindowBits = 13, + ClientMaxWindowBits = 13 + } + } + }); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); + } + } + } + } + + [Theory] + [InlineData("permessage-deflate; server_max_window_bits=12")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] + [InlineData("permessage-deflate; client_max_window_bits=9; server_no_context_takeover")] + [InlineData("permessage-deflate; client_max_window_bits=11; client_no_context_takeover")] + public async Task OptionsObjectNotModified(string clientHeader) + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + var options = new WebSocketCreationOptions() + { + DangerousDeflateOptions = new WebSocketDeflateOptions() + { + ServerMaxWindowBits = 13, + ClientMaxWindowBits = 12, + }, + KeepAliveInterval = TimeSpan.FromSeconds(14), + SubProtocol = "test" + }; + var deflateOptions = options.DangerousDeflateOptions; + Assert.True(context.WebSockets.IsWebSocketRequest); + using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = options + }); + + // Verify passed in options object is not modified + Assert.Same(deflateOptions, options.DangerousDeflateOptions); + Assert.Equal(13, options.DangerousDeflateOptions.ServerMaxWindowBits); + Assert.Equal(12, options.DangerousDeflateOptions.ClientMaxWindowBits); + Assert.True(options.DangerousDeflateOptions.ServerContextTakeover); + Assert.True(options.DangerousDeflateOptions.ClientContextTakeover); + Assert.Equal(TimeSpan.FromSeconds(14), options.KeepAliveInterval); + Assert.Equal("test", options.SubProtocol); + })) + { + using (var client = new HttpClient()) + { + var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); + uri.Scheme = "http"; + + // Craft a valid WebSocket Upgrade request + using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) + { + SetGenericWebSocketRequest(request); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.True(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); + } + } + } + } + + // Smoke test that compression works, we aren't responsible for the specifics of the compression frames + [Fact] + public async Task CanSendAndReceiveCompressedData() + { + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + { + Assert.True(context.WebSockets.IsWebSocketRequest); + using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + WebSocketOptions = new WebSocketCreationOptions() + { + DangerousDeflateOptions = new WebSocketDeflateOptions() + { + ServerMaxWindowBits = 13, + ClientMaxWindowBits = 12, + } + } + }); + + var serverBuffer = new byte[1024]; + while (true) + { + var result = await webSocket.ReceiveAsync(serverBuffer, CancellationToken.None); + if (result.MessageType == WebSocketMessageType.Close) + { + break; + } + await webSocket.SendAsync(serverBuffer.AsMemory(0, result.Count), result.MessageType, result.EndOfMessage, default); + } + await webSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, null, default); + })) + { + using (var client = new ClientWebSocket()) + { + client.Options.DangerousDeflateOptions = new WebSocketDeflateOptions() + { + ServerMaxWindowBits = 12, + ClientMaxWindowBits = 11, + }; + await client.ConnectAsync(new Uri($"ws://127.0.0.1:{port}/"), CancellationToken.None); + var sendCount = 8193; + var clientBuf = new byte[sendCount]; + var receiveBuf = new byte[sendCount]; + Random.Shared.NextBytes(clientBuf); + while (sendCount > 0) + { + await client.SendAsync(clientBuf.AsMemory(0, sendCount), WebSocketMessageType.Binary, true, default); + var totalRecv = 0; + while (totalRecv < sendCount) + { + var result = await client.ReceiveAsync(receiveBuf.AsMemory(totalRecv), default); + totalRecv += result.Count; + if (result.EndOfMessage) + { + Assert.Equal(sendCount, totalRecv); + for (var i = 0; i < sendCount; ++i) + { + Assert.True(clientBuf[i] == receiveBuf[i], $"offset {i} not equal: {clientBuf[i]} == {receiveBuf[i]}"); + } + } + } + + sendCount -= 13; + } + + await client.CloseAsync(WebSocketCloseStatus.NormalClosure, null, default); + } + } + } + + private static void SetGenericWebSocketRequest(HttpRequestMessage request) + { + request.Headers.Connection.Clear(); + request.Headers.Connection.Add("Upgrade"); + request.Headers.Connection.Add("keep-alive"); + request.Headers.Upgrade.Add(new System.Net.Http.Headers.ProductHeaderValue("websocket")); + request.Headers.Add(HeaderNames.SecWebSocketVersion, "13"); + // SecWebSocketKey required to be 16 bytes + request.Headers.Add(HeaderNames.SecWebSocketKey, Convert.ToBase64String(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }, Base64FormattingOptions.None)); + } + } +} diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs index 7bab6130096b..4fe10f556e4d 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketMiddlewareTests.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Linq; using System.Net; using System.Net.Http; using System.Net.WebSockets; @@ -37,7 +36,7 @@ public async Task Connect_Success() [Fact] public async Task NegotiateSubProtocol_Success() { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); Assert.Equal("alpha, bravo, charlie", context.Request.Headers["Sec-WebSocket-Protocol"]); @@ -65,7 +64,7 @@ public async Task NegotiateSubProtocol_Success() [Fact] public async Task SendEmptyData_Success() { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -90,7 +89,7 @@ public async Task SendEmptyData_Success() public async Task SendShortData_Success() { var orriginalData = Encoding.UTF8.GetBytes("Hello World"); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -115,7 +114,7 @@ public async Task SendShortData_Success() public async Task SendMediumData_Success() { var orriginalData = Encoding.UTF8.GetBytes(new string('a', 130)); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -141,7 +140,7 @@ public async Task SendLongData_Success() { var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var orriginalData = Encoding.UTF8.GetBytes(new string('a', 0x1FFFF)); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -172,7 +171,7 @@ public async Task SendFragmentedData_Success() { var orriginalData = Encoding.UTF8.GetBytes("Hello World"); var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -221,7 +220,7 @@ public async Task SendFragmentedData_Success() public async Task ReceiveShortData_Success() { var orriginalData = Encoding.UTF8.GetBytes("Hello World"); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -246,7 +245,7 @@ public async Task ReceiveShortData_Success() public async Task ReceiveMediumData_Success() { var orriginalData = Encoding.UTF8.GetBytes(new string('a', 130)); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -271,7 +270,7 @@ public async Task ReceiveMediumData_Success() public async Task ReceiveLongData() { var orriginalData = Encoding.UTF8.GetBytes(new string('a', 0x1FFFF)); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -304,7 +303,7 @@ public async Task ReceiveLongData() public async Task ReceiveFragmentedData_Success() { var orriginalData = Encoding.UTF8.GetBytes("Hello World"); - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -347,7 +346,7 @@ public async Task ReceiveFragmentedData_Success() public async Task SendClose_Success() { string closeDescription = "Test Closed"; - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -375,7 +374,7 @@ public async Task SendClose_Success() public async Task ReceiveClose_Success() { string closeDescription = "Test Closed"; - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -403,7 +402,7 @@ public async Task ReceiveClose_Success() public async Task CloseFromOpen_Success() { string closeDescription = "Test Closed"; - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -433,7 +432,7 @@ public async Task CloseFromOpen_Success() public async Task CloseFromCloseSent_Success() { string closeDescription = "Test Closed"; - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -465,7 +464,7 @@ public async Task CloseFromCloseSent_Success() public async Task CloseFromCloseReceived_Success() { string closeDescription = "Test Closed"; - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -511,7 +510,7 @@ public async Task CloseFromCloseReceived_Success() [InlineData(HttpStatusCode.OK, "http://ExAmPLE.cOm")] public async Task OriginIsValidatedForWebSocketRequests(HttpStatusCode expectedCode, params string[] origins) { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, context => { Assert.True(context.WebSockets.IsWebSocketRequest); return Task.CompletedTask; @@ -554,7 +553,7 @@ public async Task OriginIsValidatedForWebSocketRequests(HttpStatusCode expectedC [Fact] public async Task OriginIsNotValidatedForNonWebSocketRequests() { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, context => { Assert.False(context.WebSockets.IsWebSocketRequest); return Task.CompletedTask; @@ -580,7 +579,7 @@ public async Task OriginIsNotValidatedForNonWebSocketRequests() [Fact] public async Task CommonHeadersAreSetToInternedStrings() { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -601,7 +600,7 @@ public async Task CommonHeadersAreSetToInternedStrings() [Fact] public async Task MultipleValueHeadersNotOverridden() { - await using(var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => + await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(); @@ -633,47 +632,5 @@ public async Task MultipleValueHeadersNotOverridden() } } } - - [Theory] - [InlineData("permessage-deflate", "permessage-deflate; server_max_window_bits=15")] - [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_max_window_bits=15; server_no_context_takeover")] - [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover; server_max_window_bits=15")] - [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9; server_max_window_bits=15")] - [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15; server_max_window_bits=15")] - [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] - [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] - [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] - public async Task CompressionNegotiationProducesCorrectHeader(string clientHeader, string expectedResponse) - { - await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => - { - Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(); - })) - { - using (var client = new HttpClient()) - { - var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); - uri.Scheme = "http"; - - // Craft a valid WebSocket Upgrade request - using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) - { - request.Headers.Connection.Clear(); - request.Headers.Connection.Add("Upgrade"); - request.Headers.Connection.Add("keep-alive"); - request.Headers.Upgrade.Add(new System.Net.Http.Headers.ProductHeaderValue("websocket")); - request.Headers.Add(HeaderNames.SecWebSocketVersion, "13"); - // SecWebSocketKey required to be 16 bytes - request.Headers.Add(HeaderNames.SecWebSocketKey, Convert.ToBase64String(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }, Base64FormattingOptions.None)); - request.Headers.Add("Sec-WebSocket-Extensions", clientHeader); - - var response = await client.SendAsync(request); - Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.Equal(expectedResponse, response.Headers.GetValues("Sec-WebSocket-Extensions").Aggregate((l, r) => $"{l}; {r}")); - } - } - } - } } } From c52fd43a11a730a7d33cabb2e7c375682f8240c0 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 12 May 2021 18:18:43 -0700 Subject: [PATCH 04/14] getting closer --- .../src/ExtendedWebSocketAcceptContext.cs | 12 +- .../WebSockets/src/HandshakeHelpers.cs | 73 +++++++-- .../WebSockets/src/PublicAPI.Unshipped.txt | 8 +- .../WebSockets/src/WebSocketMiddleware.cs | 96 ++++-------- .../WebSocketCompressionMiddlewareTests.cs | 146 +++++++----------- 5 files changed, 160 insertions(+), 175 deletions(-) diff --git a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs index 8cfced631c7b..86451af7dc5d 100644 --- a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs +++ b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs @@ -29,6 +29,16 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext /// /// /// - public WebSocketCreationOptions? WebSocketOptions { get; set; } + public bool DangerousEnableCompression { get; set; } + + /// + /// + /// + public bool ServerContextTakeover { get; set; } = true; + + /// + /// + /// + public int ServerMaxWindowBits { get; set; } = 15; } } diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index fe817f5967e3..8172b9fb5d03 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -76,8 +76,11 @@ public static string CreateResponseKey(string requestKey) return Convert.ToBase64String(hashedBytes); } + // https://datatracker.ietf.org/doc/html/rfc7692#section-7.1 public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions options, [NotNullWhen(true)] out string? response) { + bool hasServerMaxWindowBits = false; + bool hasClientMaxWindowBits = false; response = null; var builder = new StringBuilder(WebSocketDeflateConstants.MaxExtensionLength); builder.Append(WebSocketDeflateConstants.Extension); @@ -91,40 +94,58 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDe { if (value.SequenceEqual(WebSocketDeflateConstants.ClientNoContextTakeover)) { - // REVIEW: If someone specifies true for server options, do we allow client to override this? options.ClientContextTakeover = false; builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover); } else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) { - options.ServerContextTakeover = false; - builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover); + // REVIEW: Do we want to reject it? + // Client requests no context takeover but options passed in specified context takeover, so reject the negotiate offer + if (options.ServerContextTakeover) + { + return false; + } } else if (value.StartsWith(WebSocketDeflateConstants.ClientMaxWindowBits)) { var clientMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ClientMaxWindowBits); - if (clientMaxWindowBits > options.ClientMaxWindowBits) + // 8 is a valid value according to the spec, but our zlib implementation does not support it + if (clientMaxWindowBits == 8) { return false; } - // if client didn't send a value for ClientMaxWindowBits use the value the server set - options.ClientMaxWindowBits = clientMaxWindowBits ?? options.ClientMaxWindowBits; + + // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 + // the server may either ignore this + // value or use this value to avoid allocating an unnecessarily big LZ77 + // sliding window by including the "client_max_window_bits" extension + // parameter in the corresponding extension negotiation response to the + // offer with a value equal to or smaller than the received value. + options.ClientMaxWindowBits = Math.Min(clientMaxWindowBits ?? 15, options.ClientMaxWindowBits); + + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=') .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); } else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) { + hasServerMaxWindowBits = true; var serverMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ServerMaxWindowBits); - if (serverMaxWindowBits > options.ServerMaxWindowBits) + // 8 is a valid value according to the spec, but our zlib implementation does not support it + if (serverMaxWindowBits == 8) { return false; } - // if client didn't send a value for ServerMaxWindowBits use the value the server set - options.ServerMaxWindowBits = serverMaxWindowBits ?? options.ServerMaxWindowBits; - builder.Append("; ") - .Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=') - .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + // https://tools.ietf.org/html/rfc7692#section-7.1.2.1 + // A server accepts an extension negotiation offer with this parameter + // by including the "server_max_window_bits" extension parameter in the + // extension negotiation response to send back to the client with the + // same or smaller value as the offer. + options.ServerMaxWindowBits = Math.Min(serverMaxWindowBits ?? 15, options.ServerMaxWindowBits); } static int? ParseWindowBits(ReadOnlySpan value, string propertyName) @@ -138,7 +159,7 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDe } if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || - windowBits < 9 || + windowBits < 8 || windowBits > 15) { throw new WebSocketException(WebSocketError.HeaderError, $"invalid {propertyName} used: {value[(startIndex + 1)..].ToString()}"); @@ -155,6 +176,32 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDe extension = extension[(end + 1)..]; } + if (!options.ServerContextTakeover) + { + builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover); + } + + if (hasServerMaxWindowBits || options.ServerMaxWindowBits != 15) + { + builder.Append("; ") + .Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=') + .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + + // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. + // + // Absence of this extension parameter in an extension negotiation + // response indicates that the server can receive messages compressed + // using an LZ77 sliding window of up to 32,768 bytes. + if (!hasClientMaxWindowBits) + { + options.ClientMaxWindowBits = 15; + } + response = builder.ToString(); return true; diff --git a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt index d901bbf0bbe0..99d5a86ba159 100644 --- a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt @@ -1,7 +1,11 @@ #nullable enable Microsoft.AspNetCore.Builder.WebSocketOptions.AllowedOrigins.get -> System.Collections.Generic.IList! -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.get -> System.Net.WebSockets.WebSocketCreationOptions? -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.set -> void +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.get -> bool +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.set -> void +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.get -> bool +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.set -> void +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.get -> int +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.set -> void Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task! ~Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.WebSocketMiddleware(Microsoft.AspNetCore.Http.RequestDelegate! next, Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.get -> string? diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index 32c13db992f8..cb8ee9117560 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -144,17 +144,18 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) } TimeSpan keepAliveInterval = _options.KeepAliveInterval; - WebSocketCreationOptions? creationOptions = null; + bool enableCompression = false; + bool serverContextTakeover = true; + int serverMaxWindowBits = 15; if (acceptContext is ExtendedWebSocketAcceptContext advancedAcceptContext) { if (advancedAcceptContext.KeepAliveInterval.HasValue) { keepAliveInterval = advancedAcceptContext.KeepAliveInterval.Value; } - if (advancedAcceptContext.WebSocketOptions is not null) - { - creationOptions = advancedAcceptContext.WebSocketOptions; - } + enableCompression = advancedAcceptContext.DangerousEnableCompression; + serverContextTakeover = advancedAcceptContext.ServerContextTakeover; + serverMaxWindowBits = advancedAcceptContext.ServerMaxWindowBits; } string key = _context.Request.Headers.SecWebSocketKey; @@ -162,28 +163,28 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) HandshakeHelpers.GenerateResponseHeaders(key, subProtocol, _context.Response.Headers); WebSocketDeflateOptions? deflateOptions = null; - var ext = _context.Request.Headers.SecWebSocketExtensions; - if (ext.Count != 0) + if (enableCompression) { - // loop over each extension offer, extensions can have multiple offers we can accept any - foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions)) + var ext = _context.Request.Headers.SecWebSocketExtensions; + if (ext.Count != 0) { - if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal) - && creationOptions?.DangerousDeflateOptions is not null) + // loop over each extension offer, extensions can have multiple offers we can accept any + foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions)) { - // We do not want to modify the users options - deflateOptions = CloneWebSocketDeflateOptions(creationOptions.DangerousDeflateOptions); - if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions!, out var response)) + if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal)) { - if (CompareDeflateOptions(deflateOptions, creationOptions.DangerousDeflateOptions)) + deflateOptions = new WebSocketDeflateOptions() + { + ServerContextTakeover = serverContextTakeover, + ServerMaxWindowBits = serverMaxWindowBits + }; + if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions, out var response)) { - // avoids allocating a new WebSocketCreationOptions below when checking if we have new deflate options to apply - deflateOptions = null; + // If more extension types are added, this would need to be a header append + // and we wouldn't want to break out of the loop + _context.Response.Headers.SecWebSocketExtensions = response; + break; } - // If more extension types are added, this would need to be a header append - // and we wouldn't want to break out of the loop - _context.Response.Headers.SecWebSocketExtensions = response; - break; } } } @@ -191,41 +192,13 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) Stream opaqueTransport = await _upgradeFeature.UpgradeAsync(); // Sets status code to 101 - WebSocketCreationOptions? options = creationOptions; - if (options is null) - { - options = new WebSocketCreationOptions() - { - IsServer = true, - KeepAliveInterval = keepAliveInterval, - SubProtocol = subProtocol, - DangerousDeflateOptions = deflateOptions, - }; - } - else if (deflateOptions is not null) - { - // use a new options instance so we don't modify the users options - options = new WebSocketCreationOptions() - { - IsServer = true, - KeepAliveInterval = creationOptions!.KeepAliveInterval, - SubProtocol = creationOptions!.SubProtocol, - DangerousDeflateOptions = deflateOptions, - }; - } - else if (options.IsServer == false) + return WebSocket.CreateFromStream(opaqueTransport, new WebSocketCreationOptions() { - // use a new options instance so we don't modify the users options - options = new WebSocketCreationOptions() - { - IsServer = true, - KeepAliveInterval = creationOptions!.KeepAliveInterval, - SubProtocol = creationOptions!.SubProtocol, - DangerousDeflateOptions = deflateOptions, - }; - } - - return WebSocket.CreateFromStream(opaqueTransport, options); + IsServer = true, + KeepAliveInterval = keepAliveInterval, + SubProtocol = subProtocol, + DangerousDeflateOptions = deflateOptions + }); } private static WebSocketDeflateOptions? CloneWebSocketDeflateOptions([NotNullIfNotNull("options")] WebSocketDeflateOptions? options) @@ -244,19 +217,6 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) }; } - private static bool CompareDeflateOptions(WebSocketDeflateOptions? lhs, WebSocketDeflateOptions? rhs) - { - if (lhs is null || rhs is null) - { - return lhs == rhs; - } - - return lhs.ClientContextTakeover == rhs.ClientContextTakeover && - lhs.ClientMaxWindowBits == rhs.ClientMaxWindowBits && - lhs.ServerContextTakeover == rhs.ServerContextTakeover && - lhs.ServerMaxWindowBits == rhs.ServerMaxWindowBits; - } - public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictionary requestHeaders) { if (!HttpMethods.IsGet(method)) diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs index 3f50a73e088b..584cc19edf01 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs @@ -18,22 +18,22 @@ public class WebSocketCompressionMiddlewareTests : LoggedTest { [Theory] [InlineData("permessage-deflate", "permessage-deflate")] - [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover")] + //[InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover")] [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover")] [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9")] [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15")] [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] - [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] - [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12")] - public async Task CompressionNegotiationProducesCorrectHeader(string clientHeader, string expectedResponse) + //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] + //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12; server_max_window_bits=10")] + public async Task CompressionNegotiationProducesCorrectHeaderWithDefaultOptions(string clientHeader, string expectedResponse) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - WebSocketOptions = new WebSocketCreationOptions() { DangerousDeflateOptions = new WebSocketDeflateOptions() } + DangerousEnableCompression = true }); })) { @@ -56,13 +56,27 @@ public async Task CompressionNegotiationProducesCorrectHeader(string clientHeade } } - [Fact] - public async Task CompressionNegotiationIgnoredIfNotEnabledOnServer() + [Theory] + [InlineData("permessage-deflate", "permessage-deflate; server_max_window_bits=14")] + //[InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits=14", "permessage-deflate; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] + //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; client_no_context_takeover; client_max_window_bits=12; server_max_window_bits=10")] + public async Task CompressionNegotiationProducesCorrectHeaderWithCustomOptions(string clientHeader, string expectedResponse) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + { + DangerousEnableCompression = true, + ServerMaxWindowBits = 14 + }); })) { using (var client = new HttpClient()) @@ -74,35 +88,23 @@ public async Task CompressionNegotiationIgnoredIfNotEnabledOnServer() using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) { SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, "permessage-deflate"); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.False(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); + Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); } } } } - [Theory] - [InlineData("permessage-deflate; server_max_window_bits=12")] - [InlineData("permessage-deflate; client_max_window_bits=12")] - public async Task CompressionNegotiateNotAccepted(string clientHeader) + [Fact] + public async Task CompressionNegotiationIgnoredIfNotEnabledOnServer() { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() - { - WebSocketOptions = new WebSocketCreationOptions() - { - DangerousDeflateOptions = new WebSocketDeflateOptions() - { - ClientMaxWindowBits = 11, - ServerMaxWindowBits = 11, - } - } - }); + var webSocket = await context.WebSockets.AcceptWebSocketAsync(); })) { using (var client = new HttpClient()) @@ -114,7 +116,7 @@ public async Task CompressionNegotiateNotAccepted(string clientHeader) using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) { SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, "permessage-deflate"); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); @@ -125,23 +127,18 @@ public async Task CompressionNegotiateNotAccepted(string clientHeader) } [Theory] - [InlineData("permessage-deflate; server_max_window_bits=16", "invalid server_max_window_bits used: 16")] - [InlineData("permessage-deflate; server_max_window_bits=8", "invalid server_max_window_bits used: 8")] - [InlineData("permessage-deflate; client_max_window_bits=16", "invalid client_max_window_bits used: 16")] - [InlineData("permessage-deflate; client_max_window_bits=8", "invalid client_max_window_bits used: 8")] - public async Task InvalidCompressionNegotiateThrows(string clientHeader, string errorMessage) + [InlineData("permessage-deflate; server_max_window_bits=8")] + [InlineData("permessage-deflate; client_max_window_bits=8")] + public async Task CompressionNegotiateNotAccepted(string clientHeader) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var ex = await Assert.ThrowsAsync(() => context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - WebSocketOptions = new WebSocketCreationOptions() - { - DangerousDeflateOptions = new WebSocketDeflateOptions(), - } - })); - Assert.Equal(errorMessage, ex.Message); + DangerousEnableCompression = true, + ServerMaxWindowBits = 11 + }); })) { using (var client = new HttpClient()) @@ -156,32 +153,28 @@ public async Task InvalidCompressionNegotiateThrows(string clientHeader, string request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); + Assert.False(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); } } } } [Theory] - [InlineData("permessage-deflate; server_max_window_bits=15, permessage-deflate; server_max_window_bits=12", "permessage-deflate; server_max_window_bits=12")] - [InlineData("permessage-deflate; server_max_window_bits=11, permessage-deflate; server_max_window_bits=12", "permessage-deflate; server_max_window_bits=11")] - [InlineData("permessage-deflate; client_max_window_bits=15, permessage-deflate; client_max_window_bits=12", "permessage-deflate; client_max_window_bits=12")] - [InlineData("permessage-deflate; client_max_window_bits=11, permessage-deflate; client_max_window_bits=14", "permessage-deflate; client_max_window_bits=11")] - public async Task CompressionNegotiationCanChooseExtension(string clientHeader, string expectedResponse) + [InlineData("permessage-deflate; server_max_window_bits=16", "invalid server_max_window_bits used: 16")] + [InlineData("permessage-deflate; server_max_window_bits=7", "invalid server_max_window_bits used: 7")] + [InlineData("permessage-deflate; client_max_window_bits=16", "invalid client_max_window_bits used: 16")] + [InlineData("permessage-deflate; client_max_window_bits=7", "invalid client_max_window_bits used: 7")] + public async Task InvalidCompressionNegotiateThrows(string clientHeader, string errorMessage) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + var ex = await Assert.ThrowsAsync(() => context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - WebSocketOptions = new WebSocketCreationOptions() - { - DangerousDeflateOptions = new WebSocketDeflateOptions() - { - ServerMaxWindowBits = 13, - ClientMaxWindowBits = 13 - } - } - }); + DangerousEnableCompression = true + })); + Assert.Equal(errorMessage, ex.Message); })) { using (var client = new HttpClient()) @@ -196,47 +189,24 @@ public async Task CompressionNegotiationCanChooseExtension(string clientHeader, request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); var response = await client.SendAsync(request); - Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); } } } } [Theory] - [InlineData("permessage-deflate; server_max_window_bits=12")] - [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover")] - [InlineData("permessage-deflate; client_max_window_bits=9; server_no_context_takeover")] - [InlineData("permessage-deflate; client_max_window_bits=11; client_no_context_takeover")] - public async Task OptionsObjectNotModified(string clientHeader) + [InlineData("permessage-deflate; server_max_window_bits=8, permessage-deflate; server_max_window_bits=13", "permessage-deflate; server_max_window_bits=13")] + [InlineData("permessage-deflate; client_max_window_bits=8, permessage-deflate; client_max_window_bits=13", "permessage-deflate; client_max_window_bits=13; server_max_window_bits=13")] + public async Task CompressionNegotiationCanChooseExtension(string clientHeader, string expectedResponse) { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { - var options = new WebSocketCreationOptions() - { - DangerousDeflateOptions = new WebSocketDeflateOptions() - { - ServerMaxWindowBits = 13, - ClientMaxWindowBits = 12, - }, - KeepAliveInterval = TimeSpan.FromSeconds(14), - SubProtocol = "test" - }; - var deflateOptions = options.DangerousDeflateOptions; Assert.True(context.WebSockets.IsWebSocketRequest); - using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - WebSocketOptions = options + DangerousEnableCompression = true, + ServerMaxWindowBits = 13 }); - - // Verify passed in options object is not modified - Assert.Same(deflateOptions, options.DangerousDeflateOptions); - Assert.Equal(13, options.DangerousDeflateOptions.ServerMaxWindowBits); - Assert.Equal(12, options.DangerousDeflateOptions.ClientMaxWindowBits); - Assert.True(options.DangerousDeflateOptions.ServerContextTakeover); - Assert.True(options.DangerousDeflateOptions.ClientContextTakeover); - Assert.Equal(TimeSpan.FromSeconds(14), options.KeepAliveInterval); - Assert.Equal("test", options.SubProtocol); })) { using (var client = new HttpClient()) @@ -252,7 +222,7 @@ public async Task OptionsObjectNotModified(string clientHeader) var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.True(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); + Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); } } } @@ -267,14 +237,8 @@ public async Task CanSendAndReceiveCompressedData() Assert.True(context.WebSockets.IsWebSocketRequest); using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - WebSocketOptions = new WebSocketCreationOptions() - { - DangerousDeflateOptions = new WebSocketDeflateOptions() - { - ServerMaxWindowBits = 13, - ClientMaxWindowBits = 12, - } - } + DangerousEnableCompression = true, + ServerMaxWindowBits = 13 }); var serverBuffer = new byte[1024]; From e8566e04f5cee07cbfd8ccc9e7ede7ef325dfe3b Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 19 May 2021 19:59:36 -0700 Subject: [PATCH 05/14] update --- .../src/ExtendedWebSocketAcceptContext.cs | 2 +- .../WebSockets/src/HandshakeHelpers.cs | 216 +++++++++++------- .../WebSockets/src/PublicAPI.Unshipped.txt | 4 +- .../WebSockets/src/WebSocketMiddleware.cs | 30 +-- .../test/UnitTests/HandshakeTests.cs | 54 +++++ .../WebSocketCompressionMiddlewareTests.cs | 155 ++----------- 6 files changed, 211 insertions(+), 250 deletions(-) diff --git a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs index 86451af7dc5d..4725c1c8cfa1 100644 --- a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs +++ b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs @@ -34,7 +34,7 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext /// /// /// - public bool ServerContextTakeover { get; set; } = true; + public bool DisableServerContextTakeover { get; set; } = false; /// /// diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index 8172b9fb5d03..fe8e140bd980 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -77,11 +77,19 @@ public static string CreateResponseKey(string requestKey) } // https://datatracker.ietf.org/doc/html/rfc7692#section-7.1 - public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions options, [NotNullWhen(true)] out string? response) + public static bool ParseDeflateOptions(ReadOnlySpan extension, bool serverContextTakeover, + int serverMaxWindowBits, out WebSocketDeflateOptions parsedOptions, [NotNullWhen(true)] out string? response) { bool hasServerMaxWindowBits = false; bool hasClientMaxWindowBits = false; + bool hasClientNoContext = false; + bool hasServerNoContext = false; response = null; + parsedOptions = new WebSocketDeflateOptions() + { + ServerContextTakeover = serverContextTakeover, + ServerMaxWindowBits = serverMaxWindowBits + }; var builder = new StringBuilder(WebSocketDeflateConstants.MaxExtensionLength); builder.Append(WebSocketDeflateConstants.Extension); @@ -90,83 +98,139 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDe int end = extension.IndexOf(';'); ReadOnlySpan value = (end >= 0 ? extension[..end] : extension).Trim(); - if (value.Length > 0) + if (value.Length == 0) + { + break; + } + + if (value.SequenceEqual(WebSocketDeflateConstants.ClientNoContextTakeover)) + { + // https://datatracker.ietf.org/doc/html/rfc7692#section-7 + // MUST decline if: + // The negotiation offer contains multiple extension parameters with + // the same name. + if (hasClientNoContext) + { + return false; + } + + hasClientNoContext = true; + parsedOptions.ClientContextTakeover = false; + builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover); + } + else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) + { + // https://datatracker.ietf.org/doc/html/rfc7692#section-7 + // MUST decline if: + // The negotiation offer contains multiple extension parameters with + // the same name. + if (hasServerNoContext) + { + return false; + } + + hasServerNoContext = true; + parsedOptions.ServerContextTakeover = false; + } + else if (value.StartsWith(WebSocketDeflateConstants.ClientMaxWindowBits)) + { + // https://datatracker.ietf.org/doc/html/rfc7692#section-7 + // MUST decline if: + // The negotiation offer contains multiple extension parameters with + // the same name. + if (hasClientMaxWindowBits) + { + return false; + } + + hasClientMaxWindowBits = true; + if (!ParseWindowBits(value, WebSocketDeflateConstants.ClientMaxWindowBits, out var clientMaxWindowBits)) + { + return false; + } + + // 8 is a valid value according to the spec, but our zlib implementation does not support it + if (clientMaxWindowBits == 8) + { + return false; + } + + // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 + // the server may either ignore this + // value or use this value to avoid allocating an unnecessarily big LZ77 + // sliding window by including the "client_max_window_bits" extension + // parameter in the corresponding extension negotiation response to the + // offer with a value equal to or smaller than the received value. + parsedOptions.ClientMaxWindowBits = clientMaxWindowBits ?? 15; + + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. + builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=') + .Append(parsedOptions.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + } + else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) { - if (value.SequenceEqual(WebSocketDeflateConstants.ClientNoContextTakeover)) + // https://datatracker.ietf.org/doc/html/rfc7692#section-7 + // MUST decline if: + // The negotiation offer contains multiple extension parameters with + // the same name. + if (hasServerMaxWindowBits) { - options.ClientContextTakeover = false; - builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover); + return false; } - else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) + + hasServerMaxWindowBits = true; + if (!ParseWindowBits(value, WebSocketDeflateConstants.ServerMaxWindowBits, out var parsedServerMaxWindowBits)) { - // REVIEW: Do we want to reject it? - // Client requests no context takeover but options passed in specified context takeover, so reject the negotiate offer - if (options.ServerContextTakeover) - { - return false; - } + return false; } - else if (value.StartsWith(WebSocketDeflateConstants.ClientMaxWindowBits)) + + // 8 is a valid value according to the spec, but our zlib implementation does not support it + if (parsedServerMaxWindowBits == 8) { - var clientMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ClientMaxWindowBits); - // 8 is a valid value according to the spec, but our zlib implementation does not support it - if (clientMaxWindowBits == 8) - { - return false; - } - - // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 - // the server may either ignore this - // value or use this value to avoid allocating an unnecessarily big LZ77 - // sliding window by including the "client_max_window_bits" extension - // parameter in the corresponding extension negotiation response to the - // offer with a value equal to or smaller than the received value. - options.ClientMaxWindowBits = Math.Min(clientMaxWindowBits ?? 15, options.ClientMaxWindowBits); - - // If a received extension negotiation offer doesn't have the - // "client_max_window_bits" extension parameter, the corresponding - // extension negotiation response to the offer MUST NOT include the - // "client_max_window_bits" extension parameter. - builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=') - .Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + return false; } - else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) + + // https://tools.ietf.org/html/rfc7692#section-7.1.2.1 + // A server accepts an extension negotiation offer with this parameter + // by including the "server_max_window_bits" extension parameter in the + // extension negotiation response to send back to the client with the + // same or smaller value as the offer. + parsedOptions.ServerMaxWindowBits = Math.Min(parsedServerMaxWindowBits ?? 15, serverMaxWindowBits); + } + + static bool ParseWindowBits(ReadOnlySpan value, string propertyName, out int? parsedValue) + { + var startIndex = value.IndexOf('='); + + // parameters can be sent without a value by the client, we'll use the values set by the app developer or the default of 15 + if (startIndex < 0) + { + parsedValue = null; + return true; + } + + value = value[(startIndex + 1)..].TrimEnd(); + + // https://datatracker.ietf.org/doc/html/rfc7692#section-5.2 + // check for value in quotes and pull the value out without the quotes + if (value[0] == '"' && value.EndsWith("\"".AsSpan())) { - hasServerMaxWindowBits = true; - var serverMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ServerMaxWindowBits); - // 8 is a valid value according to the spec, but our zlib implementation does not support it - if (serverMaxWindowBits == 8) - { - return false; - } - - // https://tools.ietf.org/html/rfc7692#section-7.1.2.1 - // A server accepts an extension negotiation offer with this parameter - // by including the "server_max_window_bits" extension parameter in the - // extension negotiation response to send back to the client with the - // same or smaller value as the offer. - options.ServerMaxWindowBits = Math.Min(serverMaxWindowBits ?? 15, options.ServerMaxWindowBits); + value = value[1..^1]; } - static int? ParseWindowBits(ReadOnlySpan value, string propertyName) + if (!int.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || + windowBits < 8 || + windowBits > 15) { - var startIndex = value.IndexOf('='); - - // parameters can be sent without a value by the client, we'll use the values set by the app developer or the default of 15 - if (startIndex < 0) - { - return null; - } - - if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) || - windowBits < 8 || - windowBits > 15) - { - throw new WebSocketException(WebSocketError.HeaderError, $"invalid {propertyName} used: {value[(startIndex + 1)..].ToString()}"); - } - - return windowBits; + parsedValue = null; + return false; } + + parsedValue = windowBits; + return true; } if (end < 0) @@ -176,30 +240,16 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, WebSocketDe extension = extension[(end + 1)..]; } - if (!options.ServerContextTakeover) + if (!parsedOptions.ServerContextTakeover) { builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover); } - if (hasServerMaxWindowBits || options.ServerMaxWindowBits != 15) + if (hasServerMaxWindowBits || parsedOptions.ServerMaxWindowBits != 15) { builder.Append("; ") .Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=') - .Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); - } - - // https://tools.ietf.org/html/rfc7692#section-7.1.2.2 - // If a received extension negotiation offer doesn't have the - // "client_max_window_bits" extension parameter, the corresponding - // extension negotiation response to the offer MUST NOT include the - // "client_max_window_bits" extension parameter. - // - // Absence of this extension parameter in an extension negotiation - // response indicates that the server can receive messages compressed - // using an LZ77 sliding window of up to 32,768 bytes. - if (!hasClientMaxWindowBits) - { - options.ClientMaxWindowBits = 15; + .Append(parsedOptions.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); } response = builder.ToString(); diff --git a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt index 99d5a86ba159..e091fc16fc6f 100644 --- a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt @@ -2,8 +2,8 @@ Microsoft.AspNetCore.Builder.WebSocketOptions.AllowedOrigins.get -> System.Collections.Generic.IList! Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.get -> bool Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.set -> void -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.get -> bool -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.set -> void +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerContextTakeover.get -> bool +Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerContextTakeover.set -> void Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.get -> int Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.set -> void Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task! diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index cb8ee9117560..88cba2f23dbf 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -154,7 +154,7 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) keepAliveInterval = advancedAcceptContext.KeepAliveInterval.Value; } enableCompression = advancedAcceptContext.DangerousEnableCompression; - serverContextTakeover = advancedAcceptContext.ServerContextTakeover; + serverContextTakeover = !advancedAcceptContext.DisableServerContextTakeover; serverMaxWindowBits = advancedAcceptContext.ServerMaxWindowBits; } @@ -168,18 +168,14 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) var ext = _context.Request.Headers.SecWebSocketExtensions; if (ext.Count != 0) { - // loop over each extension offer, extensions can have multiple offers we can accept any + // loop over each extension offer, extensions can have multiple offers, we can accept any foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions)) { - if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal)) + if (extension.AsSpan().TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal)) { - deflateOptions = new WebSocketDeflateOptions() - { - ServerContextTakeover = serverContextTakeover, - ServerMaxWindowBits = serverMaxWindowBits - }; - if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions, out var response)) + if (HandshakeHelpers.ParseDeflateOptions(extension.AsSpan().TrimStart(), serverContextTakeover, serverMaxWindowBits, out var parsedOptions, out var response)) { + deflateOptions = parsedOptions; // If more extension types are added, this would need to be a header append // and we wouldn't want to break out of the loop _context.Response.Headers.SecWebSocketExtensions = response; @@ -201,22 +197,6 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) }); } - private static WebSocketDeflateOptions? CloneWebSocketDeflateOptions([NotNullIfNotNull("options")] WebSocketDeflateOptions? options) - { - if (options is null) - { - return null; - } - - return new WebSocketDeflateOptions() - { - ClientContextTakeover = options.ClientContextTakeover, - ClientMaxWindowBits = options.ClientMaxWindowBits, - ServerContextTakeover = options.ServerContextTakeover, - ServerMaxWindowBits = options.ServerMaxWindowBits - }; - } - public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictionary requestHeaders) { if (!HttpMethods.IsGet(method)) diff --git a/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs b/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs index ec19793ddc2e..721064678e77 100644 --- a/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using Xunit; namespace Microsoft.AspNetCore.WebSockets.Tests @@ -37,5 +38,58 @@ public void RejectsInvalidRequestKeys(string key) { Assert.False(HandshakeHelpers.IsRequestKeyValid(key)); } + + [Theory] + [InlineData("permessage-deflate", "permessage-deflate")] + [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover")] + [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover")] + [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9")] + [InlineData("permessage-deflate; client_max_window_bits=\"9\"", "permessage-deflate; client_max_window_bits=9")] + [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] + [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; client_no_context_takeover; client_max_window_bits=12; server_no_context_takeover; server_max_window_bits=10")] + public void CompressionNegotiationProducesCorrectHeaderWithDefaultOptions(string clientHeader, string expectedResponse) + { + Assert.True(HandshakeHelpers.ParseDeflateOptions(clientHeader.AsSpan(), serverContextTakeover: true, serverMaxWindowBits: 15, + out var _, out var response)); + Assert.Equal(expectedResponse, response); + } + + [Theory] + [InlineData("permessage-deflate", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits=14", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] + [InlineData("permessage-deflate; server_max_window_bits=10; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; client_no_context_takeover; client_max_window_bits=12; server_no_context_takeover; server_max_window_bits=10")] + public void CompressionNegotiationProducesCorrectHeaderWithCustomOptions(string clientHeader, string expectedResponse) + { + Assert.True(HandshakeHelpers.ParseDeflateOptions(clientHeader.AsSpan(), serverContextTakeover: false, serverMaxWindowBits: 14, + out var _, out var response)); + Assert.Equal(expectedResponse, response); + } + + [Theory] + [InlineData("permessage-deflate; server_max_window_bits=8")] + [InlineData("permessage-deflate; client_max_window_bits=8")] + [InlineData("permessage-deflate; server_max_window_bits=16")] + [InlineData("permessage-deflate; client_max_window_bits=16")] + [InlineData("permessage-deflate; client_max_window_bits=\"15")] + [InlineData("permessage-deflate; client_max_window_bits=14\"")] + [InlineData("permessage-deflate; client_max_window_bits=14; client_max_window_bits=14")] + [InlineData("permessage-deflate; server_max_window_bits=14; server_max_window_bits=14")] + [InlineData("permessage-deflate; server_no_context_takeover; server_no_context_takeover")] + [InlineData("permessage-deflate; client_no_context_takeover; client_no_context_takeover")] + public void CompressionNegotiateNotAccepted(string clientHeader) + { + Assert.False(HandshakeHelpers.ParseDeflateOptions(clientHeader.AsSpan(), serverContextTakeover: true, serverMaxWindowBits: 15, + out var _, out var response)); + } } } diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs index 584cc19edf01..d29147eeff01 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs @@ -16,66 +16,16 @@ namespace Microsoft.AspNetCore.WebSockets.Test { public class WebSocketCompressionMiddlewareTests : LoggedTest { - [Theory] - [InlineData("permessage-deflate", "permessage-deflate")] - //[InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover")] - [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover")] - [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9")] - [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15")] - [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=15")] - [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] - //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] - //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; server_no_context_takeover; client_no_context_takeover; client_max_window_bits=12; server_max_window_bits=10")] - public async Task CompressionNegotiationProducesCorrectHeaderWithDefaultOptions(string clientHeader, string expectedResponse) - { - await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => - { - Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() - { - DangerousEnableCompression = true - }); - })) - { - using (var client = new HttpClient()) - { - var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); - uri.Scheme = "http"; - - // Craft a valid WebSocket Upgrade request - using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) - { - SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); - - var response = await client.SendAsync(request); - Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); - } - } - } - } - - [Theory] - [InlineData("permessage-deflate", "permessage-deflate; server_max_window_bits=14")] - //[InlineData("permessage-deflate; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=14")] - [InlineData("permessage-deflate; client_no_context_takeover", "permessage-deflate; client_no_context_takeover; server_max_window_bits=14")] - [InlineData("permessage-deflate; client_max_window_bits=9", "permessage-deflate; client_max_window_bits=9; server_max_window_bits=14")] - [InlineData("permessage-deflate; client_max_window_bits", "permessage-deflate; client_max_window_bits=15; server_max_window_bits=14")] - [InlineData("permessage-deflate; server_max_window_bits", "permessage-deflate; server_max_window_bits=14")] - [InlineData("permessage-deflate; server_max_window_bits=14", "permessage-deflate; server_max_window_bits=14")] - [InlineData("permessage-deflate; server_max_window_bits=10", "permessage-deflate; server_max_window_bits=10")] - //[InlineData("permessage-deflate; server_max_window_bits=10; server_no_context_takeover", "permessage-deflate; server_no_context_takeover; server_max_window_bits=10")] - [InlineData("permessage-deflate; server_max_window_bits=10; client_no_context_takeover; client_max_window_bits=12", "permessage-deflate; client_no_context_takeover; client_max_window_bits=12; server_max_window_bits=10")] - public async Task CompressionNegotiationProducesCorrectHeaderWithCustomOptions(string clientHeader, string expectedResponse) + [Fact] + public async Task CompressionNegotiationServerCanChooseSevrverNoContextTakeover() { await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() { - DangerousEnableCompression = true, - ServerMaxWindowBits = 14 + DangerousEnableCompression = true, + DisableServerContextTakeover = true }); })) { @@ -88,11 +38,11 @@ public async Task CompressionNegotiationProducesCorrectHeaderWithCustomOptions(s using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) { SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); + request.Headers.Add(HeaderNames.SecWebSocketExtensions, "permessage-deflate"); var response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.Equal(expectedResponse, response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); + Assert.Equal("permessage-deflate; server_no_context_takeover", response.Headers.GetValues(HeaderNames.SecWebSocketExtensions).Aggregate((l, r) => $"{l}; {r}")); } } } @@ -127,75 +77,7 @@ public async Task CompressionNegotiationIgnoredIfNotEnabledOnServer() } [Theory] - [InlineData("permessage-deflate; server_max_window_bits=8")] - [InlineData("permessage-deflate; client_max_window_bits=8")] - public async Task CompressionNegotiateNotAccepted(string clientHeader) - { - await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => - { - Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() - { - DangerousEnableCompression = true, - ServerMaxWindowBits = 11 - }); - })) - { - using (var client = new HttpClient()) - { - var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); - uri.Scheme = "http"; - - // Craft a valid WebSocket Upgrade request - using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) - { - SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); - - var response = await client.SendAsync(request); - Assert.Equal(HttpStatusCode.SwitchingProtocols, response.StatusCode); - Assert.False(response.Headers.Contains(HeaderNames.SecWebSocketExtensions)); - } - } - } - } - - [Theory] - [InlineData("permessage-deflate; server_max_window_bits=16", "invalid server_max_window_bits used: 16")] - [InlineData("permessage-deflate; server_max_window_bits=7", "invalid server_max_window_bits used: 7")] - [InlineData("permessage-deflate; client_max_window_bits=16", "invalid client_max_window_bits used: 16")] - [InlineData("permessage-deflate; client_max_window_bits=7", "invalid client_max_window_bits used: 7")] - public async Task InvalidCompressionNegotiateThrows(string clientHeader, string errorMessage) - { - await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => - { - Assert.True(context.WebSockets.IsWebSocketRequest); - var ex = await Assert.ThrowsAsync(() => context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() - { - DangerousEnableCompression = true - })); - Assert.Equal(errorMessage, ex.Message); - })) - { - using (var client = new HttpClient()) - { - var uri = new UriBuilder(new Uri($"ws://127.0.0.1:{port}/")); - uri.Scheme = "http"; - - // Craft a valid WebSocket Upgrade request - using (var request = new HttpRequestMessage(HttpMethod.Get, uri.ToString())) - { - SetGenericWebSocketRequest(request); - request.Headers.Add(HeaderNames.SecWebSocketExtensions, clientHeader); - - var response = await client.SendAsync(request); - } - } - } - } - - [Theory] - [InlineData("permessage-deflate; server_max_window_bits=8, permessage-deflate; server_max_window_bits=13", "permessage-deflate; server_max_window_bits=13")] + [InlineData("permessage-deflate; server_max_window_bits=14, permessage-deflate; server_max_window_bits=13", "permessage-deflate; server_max_window_bits=13")] [InlineData("permessage-deflate; client_max_window_bits=8, permessage-deflate; client_max_window_bits=13", "permessage-deflate; client_max_window_bits=13; server_max_window_bits=13")] public async Task CompressionNegotiationCanChooseExtension(string clientHeader, string expectedResponse) { @@ -266,25 +148,20 @@ public async Task CanSendAndReceiveCompressedData() var clientBuf = new byte[sendCount]; var receiveBuf = new byte[sendCount]; Random.Shared.NextBytes(clientBuf); - while (sendCount > 0) + await client.SendAsync(clientBuf.AsMemory(0, sendCount), WebSocketMessageType.Binary, true, default); + var totalRecv = 0; + while (totalRecv < sendCount) { - await client.SendAsync(clientBuf.AsMemory(0, sendCount), WebSocketMessageType.Binary, true, default); - var totalRecv = 0; - while (totalRecv < sendCount) + var result = await client.ReceiveAsync(receiveBuf.AsMemory(totalRecv), default); + totalRecv += result.Count; + if (result.EndOfMessage) { - var result = await client.ReceiveAsync(receiveBuf.AsMemory(totalRecv), default); - totalRecv += result.Count; - if (result.EndOfMessage) + Assert.Equal(sendCount, totalRecv); + for (var i = 0; i < sendCount; ++i) { - Assert.Equal(sendCount, totalRecv); - for (var i = 0; i < sendCount; ++i) - { - Assert.True(clientBuf[i] == receiveBuf[i], $"offset {i} not equal: {clientBuf[i]} == {receiveBuf[i]}"); - } + Assert.True(clientBuf[i] == receiveBuf[i], $"offset {i} not equal: {clientBuf[i]} == {receiveBuf[i]}"); } } - - sendCount -= 13; } await client.CloseAsync(WebSocketCloseStatus.NormalClosure, null, default); From 2dd6b1761971866f5fe307d5d2ef203d510de6ca Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 20 May 2021 10:05:20 -0700 Subject: [PATCH 06/14] generated --- .../Internal/Http/HttpHeaders.Generated.cs | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs index 5760be03b633..fa499588ba0e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs @@ -101,7 +101,7 @@ internal enum KnownHeaderType internal partial class HttpHeaders { - private readonly static HashSet _internedHeaderNames = new HashSet(95, StringComparer.OrdinalIgnoreCase) + private readonly static HashSet _internedHeaderNames = new HashSet(96, StringComparer.OrdinalIgnoreCase) { HeaderNames.Accept, HeaderNames.AcceptCharset, @@ -174,6 +174,7 @@ internal partial class HttpHeaders HeaderNames.SecWebSocketKey, HeaderNames.SecWebSocketProtocol, HeaderNames.SecWebSocketVersion, + HeaderNames.SecWebSocketExtensions, HeaderNames.Server, HeaderNames.SetCookie, HeaderNames.Status, @@ -2127,6 +2128,24 @@ StringValues IHeaderDictionary.SecWebSocketVersion SetValueUnknown(HeaderNames.SecWebSocketVersion, value); } } + StringValues IHeaderDictionary.SecWebSocketExtensions + { + get + { + StringValues value = default; + if (!TryGetUnknown(HeaderNames.SecWebSocketExtensions, ref value)) + { + value = default; + } + return value; + } + set + { + if (_isReadOnly) { ThrowHeadersReadOnlyException(); } + + SetValueUnknown(HeaderNames.SecWebSocketExtensions, value); + } + } StringValues IHeaderDictionary.Server { get @@ -10012,6 +10031,24 @@ StringValues IHeaderDictionary.SecWebSocketVersion SetValueUnknown(HeaderNames.SecWebSocketVersion, value); } } + StringValues IHeaderDictionary.SecWebSocketExtensions + { + get + { + StringValues value = default; + if (!TryGetUnknown(HeaderNames.SecWebSocketExtensions, ref value)) + { + value = default; + } + return value; + } + set + { + if (_isReadOnly) { ThrowHeadersReadOnlyException(); } + + SetValueUnknown(HeaderNames.SecWebSocketExtensions, value); + } + } StringValues IHeaderDictionary.StrictTransportSecurity { get @@ -16387,6 +16424,24 @@ StringValues IHeaderDictionary.SecWebSocketVersion SetValueUnknown(HeaderNames.SecWebSocketVersion, value); } } + StringValues IHeaderDictionary.SecWebSocketExtensions + { + get + { + StringValues value = default; + if (!TryGetUnknown(HeaderNames.SecWebSocketExtensions, ref value)) + { + value = default; + } + return value; + } + set + { + if (_isReadOnly) { ThrowHeadersReadOnlyException(); } + + SetValueUnknown(HeaderNames.SecWebSocketExtensions, value); + } + } StringValues IHeaderDictionary.Server { get From 5706de59dec895fd162c2aa464ef3a4d5a2c0de6 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 20 May 2021 17:07:56 -0700 Subject: [PATCH 07/14] fb --- .../src/ExtendedWebSocketAcceptContext.cs | 23 ++++++++++-- .../WebSockets/src/HandshakeHelpers.cs | 8 ++++- .../WebSockets/src/WebSocketMiddleware.cs | 36 +++++++++++++++---- .../test/UnitTests/HandshakeTests.cs | 4 +++ 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs index 4725c1c8cfa1..f6342ef30bd2 100644 --- a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs +++ b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs @@ -27,18 +27,35 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext public TimeSpan? KeepAliveInterval { get; set; } /// - /// + /// Enables support for the 'permessage-deflate' WebSocket extension. + /// Be aware that enabling compression makes the application subject to CRIME/BREACH type attacks. + /// It is strongly advised to turn off compression when sending data containing secrets by + /// specifying when sending such messages. /// public bool DangerousEnableCompression { get; set; } /// - /// + /// Disables server context takeover when using compression. /// + /// + /// This property does nothing when is false, + /// or when the client does not use compression. + /// + /// + /// false + /// public bool DisableServerContextTakeover { get; set; } = false; /// - /// + /// Sets the maximum base-2 logarithm of the LZ77 sliding window size that can be used for compression. /// + /// + /// This property does nothing when is false, + /// or when the client does not use compression. + /// + /// + /// 15 + /// public int ServerMaxWindowBits { get; set; } = 15; } } diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index fe8e140bd980..61064fee7779 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -214,9 +214,15 @@ static bool ParseWindowBits(ReadOnlySpan value, string propertyName, out i value = value[(startIndex + 1)..].TrimEnd(); + if (value.Length == 0) + { + parsedValue = null; + return false; + } + // https://datatracker.ietf.org/doc/html/rfc7692#section-5.2 // check for value in quotes and pull the value out without the quotes - if (value[0] == '"' && value.EndsWith("\"".AsSpan())) + if (value[0] == '"' && value.EndsWith("\"".AsSpan()) && value.Length > 1) { value = value[1..^1]; } diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index 88cba2f23dbf..3900a79a3cd0 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -3,14 +3,9 @@ using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.IO; using System.Linq; -using System.Net.Http.Headers; using System.Net.WebSockets; -using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -73,7 +68,7 @@ public Task Invoke(HttpContext context) var upgradeFeature = context.Features.Get(); if (upgradeFeature != null && context.Features.Get() == null) { - var webSocketFeature = new UpgradeHandshake(context, upgradeFeature, _options); + var webSocketFeature = new UpgradeHandshake(context, upgradeFeature, _options, _logger); context.Features.Set(webSocketFeature); if (!_anyOriginAllowed) @@ -102,13 +97,15 @@ private class UpgradeHandshake : IHttpWebSocketFeature private readonly HttpContext _context; private readonly IHttpUpgradeFeature _upgradeFeature; private readonly WebSocketOptions _options; + private readonly ILogger _logger; private bool? _isWebSocketRequest; - public UpgradeHandshake(HttpContext context, IHttpUpgradeFeature upgradeFeature, WebSocketOptions options) + public UpgradeHandshake(HttpContext context, IHttpUpgradeFeature upgradeFeature, WebSocketOptions options, ILogger logger) { _context = context; _upgradeFeature = upgradeFeature; _options = options; + _logger = logger; } public bool IsWebSocketRequest @@ -175,6 +172,7 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) { if (HandshakeHelpers.ParseDeflateOptions(extension.AsSpan().TrimStart(), serverContextTakeover, serverMaxWindowBits, out var parsedOptions, out var response)) { + Log.CompressionAccepted(_logger, response); deflateOptions = parsedOptions; // If more extension types are added, this would need to be a header append // and we wouldn't want to break out of the loop @@ -183,6 +181,11 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) } } } + + if (deflateOptions is null) + { + Log.CompressionNotAccepted(_logger); + } } } @@ -268,5 +271,24 @@ public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictiona return HandshakeHelpers.IsRequestKeyValid(requestHeaders.SecWebSocketKey.ToString()); } } + + private static class Log + { + private static readonly Action _compressionAccepted = + LoggerMessage.Define(LogLevel.Debug, new EventId(1, "CompressionAccepted"), "WebSocket compression negotiation accepted with values '{CompressionResponse}'."); + + private static readonly Action _compressionNotAccepted = + LoggerMessage.Define(LogLevel.Debug, new EventId(2, "CompressionNotAccepted"), "Compression negotiation not accepted by server."); + + public static void CompressionAccepted(ILogger logger, string response) + { + _compressionAccepted(logger, response, null); + } + + public static void CompressionNotAccepted(ILogger logger) + { + _compressionNotAccepted(logger, null); + } + } } } diff --git a/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs b/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs index 721064678e77..163fc71db4c3 100644 --- a/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/HandshakeTests.cs @@ -82,6 +82,10 @@ public void CompressionNegotiationProducesCorrectHeaderWithCustomOptions(string [InlineData("permessage-deflate; client_max_window_bits=16")] [InlineData("permessage-deflate; client_max_window_bits=\"15")] [InlineData("permessage-deflate; client_max_window_bits=14\"")] + [InlineData("permessage-deflate; client_max_window_bits=\"")] + [InlineData("permessage-deflate; client_max_window_bits=\"13")] + [InlineData("permessage-deflate; client_max_window_bits=")] + [InlineData("permessage-deflate; client_max_window_bits=\"\"")] [InlineData("permessage-deflate; client_max_window_bits=14; client_max_window_bits=14")] [InlineData("permessage-deflate; server_max_window_bits=14; server_max_window_bits=14")] [InlineData("permessage-deflate; server_no_context_takeover; server_no_context_takeover")] From 6925abdde407d8debf55d68fada914aee8866032 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 20 May 2021 18:08:39 -0700 Subject: [PATCH 08/14] extension --- .../WebSockets/src/PublicAPI.Unshipped.txt | 2 ++ .../src/WebSocketManagerExtensions.cs | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs diff --git a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt index e091fc16fc6f..494bf1a28141 100644 --- a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt @@ -6,7 +6,9 @@ Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerCont Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerContextTakeover.set -> void Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.get -> int Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.set -> void +Microsoft.AspNetCore.Http.WebSocketManagerExtensions Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task! +static Microsoft.AspNetCore.Http.WebSocketManagerExtensions.AcceptWebSocketAsync(this Microsoft.AspNetCore.Http.WebSocketManager! webSocketManager, Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext! acceptContext) -> System.Threading.Tasks.Task! ~Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.WebSocketMiddleware(Microsoft.AspNetCore.Http.RequestDelegate! next, Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.get -> string? override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.set -> void diff --git a/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs b/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs new file mode 100644 index 000000000000..5a8a9d41708d --- /dev/null +++ b/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs @@ -0,0 +1,26 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Net.WebSockets; +using System.Threading.Tasks; +using Microsoft.AspNetCore.WebSockets; + +namespace Microsoft.AspNetCore.Http +{ + /// + /// Extensions methods to accept WebSocket connections. + /// + public static class WebSocketManagerExtensions + { + /// + /// Transitions the request to a WebSocket connection using the specified options from the . + /// + /// The to accept the WebSocket with. + /// Options to use when accepting the WebSocket. + /// A WebSocket for bi-directional communication. + public static Task AcceptWebSocketAsync(this WebSocketManager webSocketManager, ExtendedWebSocketAcceptContext acceptContext) + { + return webSocketManager.AcceptWebSocketAsync(acceptContext); + } + } +} From fcbf8b6fdd98d3eb3dfbef0524fff7afee870ec0 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 24 May 2021 19:19:18 -0700 Subject: [PATCH 09/14] api review --- .../Http.Features/src/PublicAPI.Unshipped.txt | 8 ++++ .../src/WebSocketAcceptContext.cs | 40 ++++++++++++++++++- .../src/ExtendedWebSocketAcceptContext.cs | 36 +---------------- .../WebSockets/src/PublicAPI.Unshipped.txt | 8 ---- .../src/WebSocketManagerExtensions.cs | 26 ------------ .../WebSockets/src/WebSocketMiddleware.cs | 17 ++++---- .../WebSocketCompressionMiddlewareTests.cs | 7 ++-- 7 files changed, 63 insertions(+), 79 deletions(-) delete mode 100644 src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs diff --git a/src/Http/Http.Features/src/PublicAPI.Unshipped.txt b/src/Http/Http.Features/src/PublicAPI.Unshipped.txt index 7fe313271429..a391a2f049ad 100644 --- a/src/Http/Http.Features/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Features/src/PublicAPI.Unshipped.txt @@ -234,6 +234,14 @@ Microsoft.AspNetCore.Http.Features.FeatureCollection.IsReadOnly.get -> bool (for Microsoft.AspNetCore.Http.Features.FeatureCollection.Set(TFeature? instance) -> void (forwarded, contained in Microsoft.Extensions.Features) Microsoft.AspNetCore.Http.Features.FeatureCollection.this[System.Type! key].get -> object? (forwarded, contained in Microsoft.Extensions.Features) Microsoft.AspNetCore.Http.Features.FeatureCollection.this[System.Type! key].set -> void (forwarded, contained in Microsoft.Extensions.Features) +Microsoft.AspNetCore.Http.WebSocketAcceptContext.DangerousEnableCompression.get -> bool +Microsoft.AspNetCore.Http.WebSocketAcceptContext.DangerousEnableCompression.set -> void +Microsoft.AspNetCore.Http.WebSocketAcceptContext.DisableServerContextTakeover.get -> bool +Microsoft.AspNetCore.Http.WebSocketAcceptContext.DisableServerContextTakeover.set -> void +Microsoft.AspNetCore.Http.WebSocketAcceptContext.ServerMaxWindowBits.get -> int +Microsoft.AspNetCore.Http.WebSocketAcceptContext.ServerMaxWindowBits.set -> void virtual Microsoft.AspNetCore.Http.Features.FeatureCollection.Revision.get -> int (forwarded, contained in Microsoft.Extensions.Features) +virtual Microsoft.AspNetCore.Http.WebSocketAcceptContext.KeepAliveInterval.get -> System.TimeSpan? +virtual Microsoft.AspNetCore.Http.WebSocketAcceptContext.KeepAliveInterval.set -> void ~Microsoft.AspNetCore.Http.Features.FeatureReference<> (forwarded, contained in Microsoft.Extensions.Features) ~Microsoft.AspNetCore.Http.Features.FeatureReferences<> (forwarded, contained in Microsoft.Extensions.Features) diff --git a/src/Http/Http.Features/src/WebSocketAcceptContext.cs b/src/Http/Http.Features/src/WebSocketAcceptContext.cs index b293ad874be7..184e578a66b1 100644 --- a/src/Http/Http.Features/src/WebSocketAcceptContext.cs +++ b/src/Http/Http.Features/src/WebSocketAcceptContext.cs @@ -1,7 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.AspNetCore.Http.Features; +using System; +using System.Net.WebSockets; namespace Microsoft.AspNetCore.Http { @@ -14,5 +15,42 @@ public class WebSocketAcceptContext /// Gets or sets the subprotocol being negotiated. /// public virtual string? SubProtocol { get; set; } + + /// + /// The interval to send pong frames. This is a heart-beat that keeps the connection alive. + /// + public virtual TimeSpan? KeepAliveInterval { get; set; } + + /// + /// Enables support for the 'permessage-deflate' WebSocket extension. + /// Be aware that enabling compression makes the application subject to CRIME/BREACH type attacks. + /// It is strongly advised to turn off compression when sending data containing secrets by + /// specifying when sending such messages. + /// + public bool DangerousEnableCompression { get; set; } + + /// + /// Disables server context takeover when using compression. + /// + /// + /// This property does nothing when is false, + /// or when the client does not use compression. + /// + /// + /// false + /// + public bool DisableServerContextTakeover { get; set; } = false; + + /// + /// Sets the maximum base-2 logarithm of the LZ77 sliding window size that can be used for compression. + /// + /// + /// This property does nothing when is false, + /// or when the client does not use compression. + /// + /// + /// 15 + /// + public int ServerMaxWindowBits { get; set; } = 15; } } diff --git a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs index f6342ef30bd2..2e85ef2d41f8 100644 --- a/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs +++ b/src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Net.WebSockets; using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.WebSockets @@ -10,6 +9,7 @@ namespace Microsoft.AspNetCore.WebSockets /// /// Extends the class with additional properties. /// + [Obsolete("This type is obsolete and will be removed in a future version. The recommended alternative is Microsoft.AspNetCore.Http.WebSocketAcceptContext.")] public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext { /// @@ -24,38 +24,6 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext /// /// The interval to send pong frames. This is a heart-beat that keeps the connection alive. /// - public TimeSpan? KeepAliveInterval { get; set; } - - /// - /// Enables support for the 'permessage-deflate' WebSocket extension. - /// Be aware that enabling compression makes the application subject to CRIME/BREACH type attacks. - /// It is strongly advised to turn off compression when sending data containing secrets by - /// specifying when sending such messages. - /// - public bool DangerousEnableCompression { get; set; } - - /// - /// Disables server context takeover when using compression. - /// - /// - /// This property does nothing when is false, - /// or when the client does not use compression. - /// - /// - /// false - /// - public bool DisableServerContextTakeover { get; set; } = false; - - /// - /// Sets the maximum base-2 logarithm of the LZ77 sliding window size that can be used for compression. - /// - /// - /// This property does nothing when is false, - /// or when the client does not use compression. - /// - /// - /// 15 - /// - public int ServerMaxWindowBits { get; set; } = 15; + public new TimeSpan? KeepAliveInterval { get; set; } } } diff --git a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt index 494bf1a28141..cdb19672009b 100644 --- a/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt +++ b/src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt @@ -1,14 +1,6 @@ #nullable enable Microsoft.AspNetCore.Builder.WebSocketOptions.AllowedOrigins.get -> System.Collections.Generic.IList! -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.get -> bool -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.set -> void -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerContextTakeover.get -> bool -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DisableServerContextTakeover.set -> void -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.get -> int -Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.set -> void -Microsoft.AspNetCore.Http.WebSocketManagerExtensions Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task! -static Microsoft.AspNetCore.Http.WebSocketManagerExtensions.AcceptWebSocketAsync(this Microsoft.AspNetCore.Http.WebSocketManager! webSocketManager, Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext! acceptContext) -> System.Threading.Tasks.Task! ~Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.WebSocketMiddleware(Microsoft.AspNetCore.Http.RequestDelegate! next, Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.get -> string? override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.set -> void diff --git a/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs b/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs deleted file mode 100644 index 5a8a9d41708d..000000000000 --- a/src/Middleware/WebSockets/src/WebSocketManagerExtensions.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Net.WebSockets; -using System.Threading.Tasks; -using Microsoft.AspNetCore.WebSockets; - -namespace Microsoft.AspNetCore.Http -{ - /// - /// Extensions methods to accept WebSocket connections. - /// - public static class WebSocketManagerExtensions - { - /// - /// Transitions the request to a WebSocket connection using the specified options from the . - /// - /// The to accept the WebSocket with. - /// Options to use when accepting the WebSocket. - /// A WebSocket for bi-directional communication. - public static Task AcceptWebSocketAsync(this WebSocketManager webSocketManager, ExtendedWebSocketAcceptContext acceptContext) - { - return webSocketManager.AcceptWebSocketAsync(acceptContext); - } - } -} diff --git a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs index 3900a79a3cd0..c640ed4b368d 100644 --- a/src/Middleware/WebSockets/src/WebSocketMiddleware.cs +++ b/src/Middleware/WebSockets/src/WebSocketMiddleware.cs @@ -135,24 +135,27 @@ public async Task AcceptAsync(WebSocketAcceptContext acceptContext) } string? subProtocol = null; + bool enableCompression = false; + bool serverContextTakeover = true; + int serverMaxWindowBits = 15; + TimeSpan keepAliveInterval = _options.KeepAliveInterval; if (acceptContext != null) { subProtocol = acceptContext.SubProtocol; + enableCompression = acceptContext.DangerousEnableCompression; + serverContextTakeover = !acceptContext.DisableServerContextTakeover; + serverMaxWindowBits = acceptContext.ServerMaxWindowBits; + keepAliveInterval = acceptContext.KeepAliveInterval ?? keepAliveInterval; } - TimeSpan keepAliveInterval = _options.KeepAliveInterval; - bool enableCompression = false; - bool serverContextTakeover = true; - int serverMaxWindowBits = 15; +#pragma warning disable CS0618 // Type or member is obsolete if (acceptContext is ExtendedWebSocketAcceptContext advancedAcceptContext) +#pragma warning restore CS0618 // Type or member is obsolete { if (advancedAcceptContext.KeepAliveInterval.HasValue) { keepAliveInterval = advancedAcceptContext.KeepAliveInterval.Value; } - enableCompression = advancedAcceptContext.DangerousEnableCompression; - serverContextTakeover = !advancedAcceptContext.DisableServerContextTakeover; - serverMaxWindowBits = advancedAcceptContext.ServerMaxWindowBits; } string key = _context.Request.Headers.SecWebSocketKey; diff --git a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs index d29147eeff01..3803e52f9808 100644 --- a/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs +++ b/src/Middleware/WebSockets/test/UnitTests/WebSocketCompressionMiddlewareTests.cs @@ -8,6 +8,7 @@ using System.Net.WebSockets; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Testing; using Microsoft.Net.Http.Headers; using Xunit; @@ -22,7 +23,7 @@ public async Task CompressionNegotiationServerCanChooseSevrverNoContextTakeover( await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new WebSocketAcceptContext() { DangerousEnableCompression = true, DisableServerContextTakeover = true @@ -84,7 +85,7 @@ public async Task CompressionNegotiationCanChooseExtension(string clientHeader, await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + var webSocket = await context.WebSockets.AcceptWebSocketAsync(new WebSocketAcceptContext() { DangerousEnableCompression = true, ServerMaxWindowBits = 13 @@ -117,7 +118,7 @@ public async Task CanSendAndReceiveCompressedData() await using (var server = KestrelWebSocketHelpers.CreateServer(LoggerFactory, out var port, async context => { Assert.True(context.WebSockets.IsWebSocketRequest); - using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new ExtendedWebSocketAcceptContext() + using var webSocket = await context.WebSockets.AcceptWebSocketAsync(new WebSocketAcceptContext() { DangerousEnableCompression = true, ServerMaxWindowBits = 13 From 10dbd40f1c1f6f59d4311e359e04bc96f7bc09bc Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 24 May 2021 20:08:25 -0700 Subject: [PATCH 10/14] fix --- src/Http/Http.Features/src/WebSocketAcceptContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Features/src/WebSocketAcceptContext.cs b/src/Http/Http.Features/src/WebSocketAcceptContext.cs index 184e578a66b1..793c152af4d3 100644 --- a/src/Http/Http.Features/src/WebSocketAcceptContext.cs +++ b/src/Http/Http.Features/src/WebSocketAcceptContext.cs @@ -39,7 +39,7 @@ public class WebSocketAcceptContext /// /// false /// - public bool DisableServerContextTakeover { get; set; } = false; + public bool DisableServerContextTakeover { get; set; } /// /// Sets the maximum base-2 logarithm of the LZ77 sliding window size that can be used for compression. From 1038445ae58fb0282da8031fe76fdf1adb0cfd41 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 25 May 2021 15:12:15 -0700 Subject: [PATCH 11/14] fb --- .../src/WebSocketAcceptContext.cs | 21 ++++- .../src/Internal/DefaultWebSocketManager.cs | 5 +- .../WebSockets/src/HandshakeHelpers.cs | 80 +++++++++++++++---- .../src/WebSocketDeflateConstants.cs | 9 +-- .../AutobahnTestApp/Startup.cs | 5 +- 5 files changed, 94 insertions(+), 26 deletions(-) diff --git a/src/Http/Http.Features/src/WebSocketAcceptContext.cs b/src/Http/Http.Features/src/WebSocketAcceptContext.cs index 793c152af4d3..5f2c9d7a049a 100644 --- a/src/Http/Http.Features/src/WebSocketAcceptContext.cs +++ b/src/Http/Http.Features/src/WebSocketAcceptContext.cs @@ -11,6 +11,8 @@ namespace Microsoft.AspNetCore.Http /// public class WebSocketAcceptContext { + private int _serverMaxWindowBits = 15; + /// /// Gets or sets the subprotocol being negotiated. /// @@ -23,7 +25,7 @@ public class WebSocketAcceptContext /// /// Enables support for the 'permessage-deflate' WebSocket extension. - /// Be aware that enabling compression makes the application subject to CRIME/BREACH type attacks. + /// Be aware that enabling compression over encrypted connections makes the application subject to CRIME/BREACH type attacks. /// It is strongly advised to turn off compression when sending data containing secrets by /// specifying when sending such messages. /// @@ -31,6 +33,7 @@ public class WebSocketAcceptContext /// /// Disables server context takeover when using compression. + /// This setting reduces the memory overhead of compression at the cost of a potentially worse compresson ratio. /// /// /// This property does nothing when is false, @@ -43,14 +46,28 @@ public class WebSocketAcceptContext /// /// Sets the maximum base-2 logarithm of the LZ77 sliding window size that can be used for compression. + /// This setting reduces the memory overhead of compression at the cost of a potentially worse compresson ratio. /// /// /// This property does nothing when is false, /// or when the client does not use compression. + /// Valid values are 9 through 15. /// /// /// 15 /// - public int ServerMaxWindowBits { get; set; } = 15; + public int ServerMaxWindowBits + { + get => _serverMaxWindowBits; + set + { + if (value < 9 || value > 15) + { + throw new ArgumentOutOfRangeException(nameof(ServerMaxWindowBits), + "The argument must be a value from 9 to 15."); + } + _serverMaxWindowBits = value; + } + } } } diff --git a/src/Http/Http/src/Internal/DefaultWebSocketManager.cs b/src/Http/Http/src/Internal/DefaultWebSocketManager.cs index 1645351159fd..8697904b952c 100644 --- a/src/Http/Http/src/Internal/DefaultWebSocketManager.cs +++ b/src/Http/Http/src/Internal/DefaultWebSocketManager.cs @@ -17,6 +17,7 @@ internal sealed class DefaultWebSocketManager : WebSocketManager private readonly static Func _nullWebSocketFeature = f => null; private FeatureReferences _features; + private readonly static WebSocketAcceptContext _defaultWebSocketAcceptContext = new WebSocketAcceptContext(); public DefaultWebSocketManager(IFeatureCollection features) { @@ -62,7 +63,9 @@ public override IList WebSocketRequestedProtocols public override Task AcceptWebSocketAsync(string? subProtocol) { - return AcceptWebSocketAsync(new WebSocketAcceptContext() { SubProtocol = subProtocol }); + var acceptContext = subProtocol is null ? _defaultWebSocketAcceptContext : + new WebSocketAcceptContext() { SubProtocol = subProtocol }; + return AcceptWebSocketAsync(acceptContext); } public override Task AcceptWebSocketAsync(WebSocketAcceptContext acceptContext) diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index 61064fee7779..3ce15e9b0bb6 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Net.WebSockets; @@ -90,8 +91,8 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server ServerContextTakeover = serverContextTakeover, ServerMaxWindowBits = serverMaxWindowBits }; - var builder = new StringBuilder(WebSocketDeflateConstants.MaxExtensionLength); - builder.Append(WebSocketDeflateConstants.Extension); + + var responseLength = WebSocketDeflateConstants.Extension.Length; while (true) { @@ -116,7 +117,8 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server hasClientNoContext = true; parsedOptions.ClientContextTakeover = false; - builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover); + // 2 = '; ' + responseLength += 2 + WebSocketDeflateConstants.ClientNoContextTakeover.Length; } else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) { @@ -162,13 +164,9 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server // parameter in the corresponding extension negotiation response to the // offer with a value equal to or smaller than the received value. parsedOptions.ClientMaxWindowBits = clientMaxWindowBits ?? 15; - - // If a received extension negotiation offer doesn't have the - // "client_max_window_bits" extension parameter, the corresponding - // extension negotiation response to the offer MUST NOT include the - // "client_max_window_bits" extension parameter. - builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=') - .Append(parsedOptions.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + // 2 = '; ', 1 = '=' + responseLength += 2 + WebSocketDeflateConstants.ClientMaxWindowBits.Length + 1 + + ((parsedOptions.ClientMaxWindowBits > 9) ? 2 : 1); } else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) { @@ -248,17 +246,69 @@ static bool ParseWindowBits(ReadOnlySpan value, string propertyName, out i if (!parsedOptions.ServerContextTakeover) { - builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover); + // 2 = '; ' + responseLength += 2 + WebSocketDeflateConstants.ServerNoContextTakeover.Length; } if (hasServerMaxWindowBits || parsedOptions.ServerMaxWindowBits != 15) { - builder.Append("; ") - .Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=') - .Append(parsedOptions.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture)); + // 2 = '; ', 1 = '=' + responseLength += 2 + WebSocketDeflateConstants.ServerMaxWindowBits.Length + 1 + + ((parsedOptions.ServerMaxWindowBits > 9) ? 2 : 1); } - response = builder.ToString(); + response = string.Create(responseLength, (parsedOptions, hasClientMaxWindowBits, hasServerMaxWindowBits, hasClientNoContext), + static (span, state) => + { + WebSocketDeflateConstants.Extension.AsSpan().CopyTo(span); + span = span.Slice(WebSocketDeflateConstants.Extension.Length); + if (state.hasClientNoContext) + { + span[0] = ';'; + span[1] = ' '; + span = span.Slice(2); + WebSocketDeflateConstants.ClientNoContextTakeover.AsSpan().CopyTo(span); + span = span.Slice(WebSocketDeflateConstants.ClientNoContextTakeover.Length); + } + if (state.hasClientMaxWindowBits) + { + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. + span[0] = ';'; + span[1] = ' '; + span = span.Slice(2); + WebSocketDeflateConstants.ClientMaxWindowBits.AsSpan().CopyTo(span); + span = span.Slice(WebSocketDeflateConstants.ClientMaxWindowBits.Length); + span[0] = '='; + span = span.Slice(1); + var ret = state.parsedOptions.ClientMaxWindowBits.TryFormat(span, out var written); + Debug.Assert(ret); + span = span.Slice(written); + } + if (!state.parsedOptions.ServerContextTakeover) + { + span[0] = ';'; + span[1] = ' '; + span = span.Slice(2); + WebSocketDeflateConstants.ServerNoContextTakeover.AsSpan().CopyTo(span); + span = span.Slice(WebSocketDeflateConstants.ServerNoContextTakeover.Length); + } + if (state.hasServerMaxWindowBits || state.parsedOptions.ServerMaxWindowBits != 15) + { + span[0] = ';'; + span[1] = ' '; + span = span.Slice(2); + WebSocketDeflateConstants.ServerMaxWindowBits.AsSpan().CopyTo(span); + span = span.Slice(WebSocketDeflateConstants.ServerMaxWindowBits.Length); + span[0] = '='; + span = span.Slice(1); + var ret = state.parsedOptions.ServerMaxWindowBits.TryFormat(span, out var written); + Debug.Assert(ret); + span = span.Slice(written); + } + }); return true; } diff --git a/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs index aff0a93dccf4..19a63dd06169 100644 --- a/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs +++ b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs @@ -1,17 +1,12 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; + namespace Microsoft.AspNetCore.WebSockets { internal static class WebSocketDeflateConstants { - /// - /// The maximum length that this extension can have, assuming that we're not using extra white space. - /// - /// "permessage-deflate; client_max_window_bits=15; client_no_context_takeover; server_max_window_bits=15; server_no_context_takeover" - /// - public const int MaxExtensionLength = 128; - public const string Extension = "permessage-deflate"; public const string ClientMaxWindowBits = "client_max_window_bits"; diff --git a/src/Middleware/WebSockets/test/ConformanceTests/AutobahnTestApp/Startup.cs b/src/Middleware/WebSockets/test/ConformanceTests/AutobahnTestApp/Startup.cs index f8b75268c28b..c351ae285e6f 100644 --- a/src/Middleware/WebSockets/test/ConformanceTests/AutobahnTestApp/Startup.cs +++ b/src/Middleware/WebSockets/test/ConformanceTests/AutobahnTestApp/Startup.cs @@ -22,7 +22,10 @@ public void Configure(IApplicationBuilder app, ILoggerFactory loggerFactory) if (context.WebSockets.IsWebSocketRequest) { logger.LogInformation("Received WebSocket request"); - using (var webSocket = await context.WebSockets.AcceptWebSocketAsync()) + using (var webSocket = await context.WebSockets.AcceptWebSocketAsync(new WebSocketAcceptContext() + { + DangerousEnableCompression = true + })) { await Echo(webSocket, context.RequestAborted); } From 1436b6c5b3a851956bacaf2f532439070bc205f5 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 26 May 2021 16:29:39 -0700 Subject: [PATCH 12/14] wip --- .../Microsoft.AspNetCore.WebSockets.csproj | 4 + .../ValueStringBuilder/ValueStringBuilder.cs | 321 ++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 src/Shared/ValueStringBuilder/ValueStringBuilder.cs diff --git a/src/Middleware/WebSockets/src/Microsoft.AspNetCore.WebSockets.csproj b/src/Middleware/WebSockets/src/Microsoft.AspNetCore.WebSockets.csproj index 931295880832..1443b5fae957 100644 --- a/src/Middleware/WebSockets/src/Microsoft.AspNetCore.WebSockets.csproj +++ b/src/Middleware/WebSockets/src/Microsoft.AspNetCore.WebSockets.csproj @@ -17,6 +17,10 @@ + + + + diff --git a/src/Shared/ValueStringBuilder/ValueStringBuilder.cs b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs new file mode 100644 index 000000000000..1a054907f795 --- /dev/null +++ b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs @@ -0,0 +1,321 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +namespace System.Text +{ + internal ref partial struct ValueStringBuilder + { + private char[]? _arrayToReturnToPool; + private Span _chars; + private int _pos; + + public ValueStringBuilder(Span initialBuffer) + { + _arrayToReturnToPool = null; + _chars = initialBuffer; + _pos = 0; + } + + public ValueStringBuilder(int initialCapacity) + { + _arrayToReturnToPool = ArrayPool.Shared.Rent(initialCapacity); + _chars = _arrayToReturnToPool; + _pos = 0; + } + + public int Length + { + get => _pos; + set + { + Debug.Assert(value >= 0); + Debug.Assert(value <= _chars.Length); + _pos = value; + } + } + + public int Capacity => _chars.Length; + + public void EnsureCapacity(int capacity) + { + // This is not expected to be called this with negative capacity + Debug.Assert(capacity >= 0); + + // If the caller has a bug and calls this with negative capacity, make sure to call Grow to throw an exception. + if ((uint)capacity > (uint)_chars.Length) + Grow(capacity - _pos); + } + + /// + /// Get a pinnable reference to the builder. + /// Does not ensure there is a null char after + /// This overload is pattern matched in the C# 7.3+ compiler so you can omit + /// the explicit method call, and write eg "fixed (char* c = builder)" + /// + public ref char GetPinnableReference() + { + return ref MemoryMarshal.GetReference(_chars); + } + + /// + /// Get a pinnable reference to the builder. + /// + /// Ensures that the builder has a null char after + public ref char GetPinnableReference(bool terminate) + { + if (terminate) + { + EnsureCapacity(Length + 1); + _chars[Length] = '\0'; + } + return ref MemoryMarshal.GetReference(_chars); + } + + public ref char this[int index] + { + get + { + Debug.Assert(index < _pos); + return ref _chars[index]; + } + } + + public override string ToString() + { + string s = _chars.Slice(0, _pos).ToString(); + Dispose(); + return s; + } + + /// Returns the underlying storage of the builder. + public Span RawChars => _chars; + + /// + /// Returns a span around the contents of the builder. + /// + /// Ensures that the builder has a null char after + public ReadOnlySpan AsSpan(bool terminate) + { + if (terminate) + { + EnsureCapacity(Length + 1); + _chars[Length] = '\0'; + } + return _chars.Slice(0, _pos); + } + + public ReadOnlySpan AsSpan() => _chars.Slice(0, _pos); + public ReadOnlySpan AsSpan(int start) => _chars.Slice(start, _pos - start); + public ReadOnlySpan AsSpan(int start, int length) => _chars.Slice(start, length); + + public bool TryCopyTo(Span destination, out int charsWritten) + { + if (_chars.Slice(0, _pos).TryCopyTo(destination)) + { + charsWritten = _pos; + Dispose(); + return true; + } + else + { + charsWritten = 0; + Dispose(); + return false; + } + } + + public void Insert(int index, char value, int count) + { + if (_pos > _chars.Length - count) + { + Grow(count); + } + + int remaining = _pos - index; + _chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count)); + _chars.Slice(index, count).Fill(value); + _pos += count; + } + + public void Insert(int index, string? s) + { + if (s == null) + { + return; + } + + int count = s.Length; + + if (_pos > (_chars.Length - count)) + { + Grow(count); + } + + int remaining = _pos - index; + _chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count)); + s +#if !NET6_0_OR_GREATER + .AsSpan() +#endif + .CopyTo(_chars.Slice(index)); + _pos += count; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Append(char c) + { + int pos = _pos; + if ((uint)pos < (uint)_chars.Length) + { + _chars[pos] = c; + _pos = pos + 1; + } + else + { + GrowAndAppend(c); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Append(string? s) + { + if (s == null) + { + return; + } + + int pos = _pos; + if (s.Length == 1 && (uint)pos < (uint)_chars.Length) // very common case, e.g. appending strings from NumberFormatInfo like separators, percent symbols, etc. + { + _chars[pos] = s[0]; + _pos = pos + 1; + } + else + { + AppendSlow(s); + } + } + + private void AppendSlow(string s) + { + int pos = _pos; + if (pos > _chars.Length - s.Length) + { + Grow(s.Length); + } + + s +#if !NET6_0_OR_GREATER + .AsSpan() +#endif + .CopyTo(_chars.Slice(pos)); + _pos += s.Length; + } + + public void Append(char c, int count) + { + if (_pos > _chars.Length - count) + { + Grow(count); + } + + Span dst = _chars.Slice(_pos, count); + for (int i = 0; i < dst.Length; i++) + { + dst[i] = c; + } + _pos += count; + } + + public unsafe void Append(char* value, int length) + { + int pos = _pos; + if (pos > _chars.Length - length) + { + Grow(length); + } + + Span dst = _chars.Slice(_pos, length); + for (int i = 0; i < dst.Length; i++) + { + dst[i] = *value++; + } + _pos += length; + } + + public void Append(ReadOnlySpan value) + { + int pos = _pos; + if (pos > _chars.Length - value.Length) + { + Grow(value.Length); + } + + value.CopyTo(_chars.Slice(_pos)); + _pos += value.Length; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Span AppendSpan(int length) + { + int origPos = _pos; + if (origPos > _chars.Length - length) + { + Grow(length); + } + + _pos = origPos + length; + return _chars.Slice(origPos, length); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void GrowAndAppend(char c) + { + Grow(1); + Append(c); + } + + /// + /// Resize the internal buffer either by doubling current buffer size or + /// by adding to + /// whichever is greater. + /// + /// + /// Number of chars requested beyond current position. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + private void Grow(int additionalCapacityBeyondPos) + { + Debug.Assert(additionalCapacityBeyondPos > 0); + Debug.Assert(_pos > _chars.Length - additionalCapacityBeyondPos, "Grow called incorrectly, no resize is needed."); + + // Make sure to let Rent throw an exception if the caller has a bug and the desired capacity is negative + char[] poolArray = ArrayPool.Shared.Rent((int)Math.Max((uint)(_pos + additionalCapacityBeyondPos), (uint)_chars.Length * 2)); + + _chars.Slice(0, _pos).CopyTo(poolArray); + + char[]? toReturn = _arrayToReturnToPool; + _chars = _arrayToReturnToPool = poolArray; + if (toReturn != null) + { + ArrayPool.Shared.Return(toReturn); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void Dispose() + { + char[]? toReturn = _arrayToReturnToPool; + this = default; // for safety, to avoid using pooled array if this instance is erroneously appended to again + if (toReturn != null) + { + ArrayPool.Shared.Return(toReturn); + } + } + } +} \ No newline at end of file From e6a5f358b6cd35ebd5c03ea45cdfe4e0b2b6a443 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 26 May 2021 16:59:08 -0700 Subject: [PATCH 13/14] valuestringbuilder --- .../WebSockets/src/HandshakeHelpers.cs | 95 +++++++------------ .../src/WebSocketDeflateConstants.cs | 9 +- .../ValueStringBuilder/ValueStringBuilder.cs | 14 +-- 3 files changed, 42 insertions(+), 76 deletions(-) diff --git a/src/Middleware/WebSockets/src/HandshakeHelpers.cs b/src/Middleware/WebSockets/src/HandshakeHelpers.cs index 3ce15e9b0bb6..a242a281154a 100644 --- a/src/Middleware/WebSockets/src/HandshakeHelpers.cs +++ b/src/Middleware/WebSockets/src/HandshakeHelpers.cs @@ -92,7 +92,8 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server ServerMaxWindowBits = serverMaxWindowBits }; - var responseLength = WebSocketDeflateConstants.Extension.Length; + using var builder = new ValueStringBuilder(WebSocketDeflateConstants.MaxExtensionLength); + builder.Append(WebSocketDeflateConstants.Extension); while (true) { @@ -117,8 +118,9 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server hasClientNoContext = true; parsedOptions.ClientContextTakeover = false; - // 2 = '; ' - responseLength += 2 + WebSocketDeflateConstants.ClientNoContextTakeover.Length; + builder.Append(';'); + builder.Append(' '); + builder.Append(WebSocketDeflateConstants.ClientNoContextTakeover); } else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover)) { @@ -164,9 +166,20 @@ public static bool ParseDeflateOptions(ReadOnlySpan extension, bool server // parameter in the corresponding extension negotiation response to the // offer with a value equal to or smaller than the received value. parsedOptions.ClientMaxWindowBits = clientMaxWindowBits ?? 15; - // 2 = '; ', 1 = '=' - responseLength += 2 + WebSocketDeflateConstants.ClientMaxWindowBits.Length + 1 + - ((parsedOptions.ClientMaxWindowBits > 9) ? 2 : 1); + + // If a received extension negotiation offer doesn't have the + // "client_max_window_bits" extension parameter, the corresponding + // extension negotiation response to the offer MUST NOT include the + // "client_max_window_bits" extension parameter. + builder.Append(';'); + builder.Append(' '); + builder.Append(WebSocketDeflateConstants.ClientMaxWindowBits); + builder.Append('='); + var len = (parsedOptions.ClientMaxWindowBits > 9) ? 2 : 1; + var span = builder.AppendSpan(len); + var ret = parsedOptions.ClientMaxWindowBits.TryFormat(span, out var written); + Debug.Assert(ret); + Debug.Assert(written == len); } else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits)) { @@ -246,69 +259,25 @@ static bool ParseWindowBits(ReadOnlySpan value, string propertyName, out i if (!parsedOptions.ServerContextTakeover) { - // 2 = '; ' - responseLength += 2 + WebSocketDeflateConstants.ServerNoContextTakeover.Length; + builder.Append(';'); + builder.Append(' '); + builder.Append(WebSocketDeflateConstants.ServerNoContextTakeover); } if (hasServerMaxWindowBits || parsedOptions.ServerMaxWindowBits != 15) { - // 2 = '; ', 1 = '=' - responseLength += 2 + WebSocketDeflateConstants.ServerMaxWindowBits.Length + 1 + - ((parsedOptions.ServerMaxWindowBits > 9) ? 2 : 1); + builder.Append(';'); + builder.Append(' '); + builder.Append(WebSocketDeflateConstants.ServerMaxWindowBits); + builder.Append('='); + var len = (parsedOptions.ServerMaxWindowBits > 9) ? 2 : 1; + var span = builder.AppendSpan(len); + var ret = parsedOptions.ServerMaxWindowBits.TryFormat(span, out var written); + Debug.Assert(ret); + Debug.Assert(written == len); } - response = string.Create(responseLength, (parsedOptions, hasClientMaxWindowBits, hasServerMaxWindowBits, hasClientNoContext), - static (span, state) => - { - WebSocketDeflateConstants.Extension.AsSpan().CopyTo(span); - span = span.Slice(WebSocketDeflateConstants.Extension.Length); - if (state.hasClientNoContext) - { - span[0] = ';'; - span[1] = ' '; - span = span.Slice(2); - WebSocketDeflateConstants.ClientNoContextTakeover.AsSpan().CopyTo(span); - span = span.Slice(WebSocketDeflateConstants.ClientNoContextTakeover.Length); - } - if (state.hasClientMaxWindowBits) - { - // If a received extension negotiation offer doesn't have the - // "client_max_window_bits" extension parameter, the corresponding - // extension negotiation response to the offer MUST NOT include the - // "client_max_window_bits" extension parameter. - span[0] = ';'; - span[1] = ' '; - span = span.Slice(2); - WebSocketDeflateConstants.ClientMaxWindowBits.AsSpan().CopyTo(span); - span = span.Slice(WebSocketDeflateConstants.ClientMaxWindowBits.Length); - span[0] = '='; - span = span.Slice(1); - var ret = state.parsedOptions.ClientMaxWindowBits.TryFormat(span, out var written); - Debug.Assert(ret); - span = span.Slice(written); - } - if (!state.parsedOptions.ServerContextTakeover) - { - span[0] = ';'; - span[1] = ' '; - span = span.Slice(2); - WebSocketDeflateConstants.ServerNoContextTakeover.AsSpan().CopyTo(span); - span = span.Slice(WebSocketDeflateConstants.ServerNoContextTakeover.Length); - } - if (state.hasServerMaxWindowBits || state.parsedOptions.ServerMaxWindowBits != 15) - { - span[0] = ';'; - span[1] = ' '; - span = span.Slice(2); - WebSocketDeflateConstants.ServerMaxWindowBits.AsSpan().CopyTo(span); - span = span.Slice(WebSocketDeflateConstants.ServerMaxWindowBits.Length); - span[0] = '='; - span = span.Slice(1); - var ret = state.parsedOptions.ServerMaxWindowBits.TryFormat(span, out var written); - Debug.Assert(ret); - span = span.Slice(written); - } - }); + response = builder.ToString(); return true; } diff --git a/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs index 19a63dd06169..aff0a93dccf4 100644 --- a/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs +++ b/src/Middleware/WebSockets/src/WebSocketDeflateConstants.cs @@ -1,12 +1,17 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; - namespace Microsoft.AspNetCore.WebSockets { internal static class WebSocketDeflateConstants { + /// + /// The maximum length that this extension can have, assuming that we're not using extra white space. + /// + /// "permessage-deflate; client_max_window_bits=15; client_no_context_takeover; server_max_window_bits=15; server_no_context_takeover" + /// + public const int MaxExtensionLength = 128; + public const string Extension = "permessage-deflate"; public const string ClientMaxWindowBits = "client_max_window_bits"; diff --git a/src/Shared/ValueStringBuilder/ValueStringBuilder.cs b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs index 1a054907f795..c4c4ea1ba478 100644 --- a/src/Shared/ValueStringBuilder/ValueStringBuilder.cs +++ b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs @@ -158,11 +158,7 @@ public void Insert(int index, string? s) int remaining = _pos - index; _chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count)); - s -#if !NET6_0_OR_GREATER - .AsSpan() -#endif - .CopyTo(_chars.Slice(index)); + s.AsSpan().CopyTo(_chars.Slice(index)); _pos += count; } @@ -209,11 +205,7 @@ private void AppendSlow(string s) Grow(s.Length); } - s -#if !NET6_0_OR_GREATER - .AsSpan() -#endif - .CopyTo(_chars.Slice(pos)); + s.AsSpan().CopyTo(_chars.Slice(pos)); _pos += s.Length; } @@ -318,4 +310,4 @@ public void Dispose() } } } -} \ No newline at end of file +} From fffcdea3dabd06640c3daafcb098f75729e13238 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 26 May 2021 17:10:07 -0700 Subject: [PATCH 14/14] copied from --- src/Shared/ValueStringBuilder/ValueStringBuilder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Shared/ValueStringBuilder/ValueStringBuilder.cs b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs index c4c4ea1ba478..90c65f9cfd72 100644 --- a/src/Shared/ValueStringBuilder/ValueStringBuilder.cs +++ b/src/Shared/ValueStringBuilder/ValueStringBuilder.cs @@ -8,6 +8,7 @@ namespace System.Text { + // Copied from https://github.com/dotnet/runtime/blob/a9c5eadd951dcba73167f72cc624eb790573663a/src/libraries/Common/src/System/Text/ValueStringBuilder.cs internal ref partial struct ValueStringBuilder { private char[]? _arrayToReturnToPool;