Skip to content

Commit 11e2816

Browse files
BrennanConroyBrennanConroy
committed
getting closer
1 parent cb6d83a commit 11e2816

File tree

5 files changed

+160
-175
lines changed

5 files changed

+160
-175
lines changed

‎src/Middleware/WebSockets/src/ExtendedWebSocketAcceptContext.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ public class ExtendedWebSocketAcceptContext : WebSocketAcceptContext
2929
/// <summary>
3030
///
3131
/// </summary>
32-
public WebSocketCreationOptions? WebSocketOptions { get; set; }
32+
public bool DangerousEnableCompression { get; set; }
33+
34+
/// <summary>
35+
///
36+
/// </summary>
37+
public bool ServerContextTakeover { get; set; } = true;
38+
39+
/// <summary>
40+
///
41+
/// </summary>
42+
public int ServerMaxWindowBits { get; set; } = 15;
3343
}
3444
}

‎src/Middleware/WebSockets/src/HandshakeHelpers.cs‎

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ public static string CreateResponseKey(string requestKey)
7676
return Convert.ToBase64String(hashedBytes);
7777
}
7878

79+
// https://datatracker.ietf.org/doc/html/rfc7692#section-7.1
7980
public static bool ParseDeflateOptions(ReadOnlySpan<char> extension, WebSocketDeflateOptions options, [NotNullWhen(true)] out string? response)
8081
{
82+
bool hasServerMaxWindowBits = false;
83+
bool hasClientMaxWindowBits = false;
8184
response = null;
8285
var builder = new StringBuilder(WebSocketDeflateConstants.MaxExtensionLength);
8386
builder.Append(WebSocketDeflateConstants.Extension);
@@ -91,40 +94,58 @@ public static bool ParseDeflateOptions(ReadOnlySpan<char> extension, WebSocketDe
9194
{
9295
if (value.SequenceEqual(WebSocketDeflateConstants.ClientNoContextTakeover))
9396
{
94-
// REVIEW: If someone specifies true for server options, do we allow client to override this?
9597
options.ClientContextTakeover = false;
9698
builder.Append("; ").Append(WebSocketDeflateConstants.ClientNoContextTakeover);
9799
}
98100
else if (value.SequenceEqual(WebSocketDeflateConstants.ServerNoContextTakeover))
99101
{
100-
options.ServerContextTakeover = false;
101-
builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover);
102+
// REVIEW: Do we want to reject it?
103+
// Client requests no context takeover but options passed in specified context takeover, so reject the negotiate offer
104+
if (options.ServerContextTakeover)
105+
{
106+
return false;
107+
}
102108
}
103109
else if (value.StartsWith(WebSocketDeflateConstants.ClientMaxWindowBits))
104110
{
105111
var clientMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ClientMaxWindowBits);
106-
if (clientMaxWindowBits > options.ClientMaxWindowBits)
112+
// 8 is a valid value according to the spec, but our zlib implementation does not support it
113+
if (clientMaxWindowBits == 8)
107114
{
108115
return false;
109116
}
110-
// if client didn't send a value for ClientMaxWindowBits use the value the server set
111-
options.ClientMaxWindowBits = clientMaxWindowBits ?? options.ClientMaxWindowBits;
117+
118+
// https://tools.ietf.org/html/rfc7692#section-7.1.2.2
119+
// the server may either ignore this
120+
// value or use this value to avoid allocating an unnecessarily big LZ77
121+
// sliding window by including the "client_max_window_bits" extension
122+
// parameter in the corresponding extension negotiation response to the
123+
// offer with a value equal to or smaller than the received value.
124+
options.ClientMaxWindowBits = Math.Min(clientMaxWindowBits ?? 15, options.ClientMaxWindowBits);
125+
126+
// If a received extension negotiation offer doesn't have the
127+
// "client_max_window_bits" extension parameter, the corresponding
128+
// extension negotiation response to the offer MUST NOT include the
129+
// "client_max_window_bits" extension parameter.
112130
builder.Append("; ").Append(WebSocketDeflateConstants.ClientMaxWindowBits).Append('=')
113131
.Append(options.ClientMaxWindowBits.ToString(CultureInfo.InvariantCulture));
114132
}
115133
else if (value.StartsWith(WebSocketDeflateConstants.ServerMaxWindowBits))
116134
{
135+
hasServerMaxWindowBits = true;
117136
var serverMaxWindowBits = ParseWindowBits(value, WebSocketDeflateConstants.ServerMaxWindowBits);
118-
if (serverMaxWindowBits > options.ServerMaxWindowBits)
137+
// 8 is a valid value according to the spec, but our zlib implementation does not support it
138+
if (serverMaxWindowBits == 8)
119139
{
120140
return false;
121141
}
122-
// if client didn't send a value for ServerMaxWindowBits use the value the server set
123-
options.ServerMaxWindowBits = serverMaxWindowBits ?? options.ServerMaxWindowBits;
124142

125-
builder.Append("; ")
126-
.Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=')
127-
.Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture));
143+
// https://tools.ietf.org/html/rfc7692#section-7.1.2.1
144+
// A server accepts an extension negotiation offer with this parameter
145+
// by including the "server_max_window_bits" extension parameter in the
146+
// extension negotiation response to send back to the client with the
147+
// same or smaller value as the offer.
148+
options.ServerMaxWindowBits = Math.Min(serverMaxWindowBits ?? 15, options.ServerMaxWindowBits);
128149
}
129150

130151
static int? ParseWindowBits(ReadOnlySpan<char> value, string propertyName)
@@ -138,7 +159,7 @@ public static bool ParseDeflateOptions(ReadOnlySpan<char> extension, WebSocketDe
138159
}
139160

140161
if (!int.TryParse(value[(startIndex + 1)..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int windowBits) ||
141-
windowBits < 9 ||
162+
windowBits < 8 ||
142163
windowBits > 15)
143164
{
144165
throw new WebSocketException(WebSocketError.HeaderError, $"invalid {propertyName} used: {value[(startIndex + 1)..].ToString()}");
@@ -155,6 +176,32 @@ public static bool ParseDeflateOptions(ReadOnlySpan<char> extension, WebSocketDe
155176
extension = extension[(end + 1)..];
156177
}
157178

179+
if (!options.ServerContextTakeover)
180+
{
181+
builder.Append("; ").Append(WebSocketDeflateConstants.ServerNoContextTakeover);
182+
}
183+
184+
if (hasServerMaxWindowBits || options.ServerMaxWindowBits != 15)
185+
{
186+
builder.Append("; ")
187+
.Append(WebSocketDeflateConstants.ServerMaxWindowBits).Append('=')
188+
.Append(options.ServerMaxWindowBits.ToString(CultureInfo.InvariantCulture));
189+
}
190+
191+
// https://tools.ietf.org/html/rfc7692#section-7.1.2.2
192+
// If a received extension negotiation offer doesn't have the
193+
// "client_max_window_bits" extension parameter, the corresponding
194+
// extension negotiation response to the offer MUST NOT include the
195+
// "client_max_window_bits" extension parameter.
196+
//
197+
// Absence of this extension parameter in an extension negotiation
198+
// response indicates that the server can receive messages compressed
199+
// using an LZ77 sliding window of up to 32,768 bytes.
200+
if (!hasClientMaxWindowBits)
201+
{
202+
options.ClientMaxWindowBits = 15;
203+
}
204+
158205
response = builder.ToString();
159206

160207
return true;

‎src/Middleware/WebSockets/src/PublicAPI.Unshipped.txt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
#nullable enable
22
Microsoft.AspNetCore.Builder.WebSocketOptions.AllowedOrigins.get -> System.Collections.Generic.IList<string!>!
3-
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.get -> System.Net.WebSockets.WebSocketCreationOptions?
4-
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.WebSocketOptions.set -> void
3+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.get -> bool
4+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.DangerousEnableCompression.set -> void
5+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.get -> bool
6+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerContextTakeover.set -> void
7+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.get -> int
8+
Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.ServerMaxWindowBits.set -> void
59
Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.Invoke(Microsoft.AspNetCore.Http.HttpContext! context) -> System.Threading.Tasks.Task!
610
~Microsoft.AspNetCore.WebSockets.WebSocketMiddleware.WebSocketMiddleware(Microsoft.AspNetCore.Http.RequestDelegate! next, Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Builder.WebSocketOptions!>! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void
711
override Microsoft.AspNetCore.WebSockets.ExtendedWebSocketAcceptContext.SubProtocol.get -> string?

‎src/Middleware/WebSockets/src/WebSocketMiddleware.cs‎

Lines changed: 28 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -144,88 +144,61 @@ public async Task<WebSocket> AcceptAsync(WebSocketAcceptContext acceptContext)
144144
}
145145

146146
TimeSpan keepAliveInterval = _options.KeepAliveInterval;
147-
WebSocketCreationOptions? creationOptions = null;
147+
bool enableCompression = false;
148+
bool serverContextTakeover = true;
149+
int serverMaxWindowBits = 15;
148150
if (acceptContext is ExtendedWebSocketAcceptContext advancedAcceptContext)
149151
{
150152
if (advancedAcceptContext.KeepAliveInterval.HasValue)
151153
{
152154
keepAliveInterval = advancedAcceptContext.KeepAliveInterval.Value;
153155
}
154-
if (advancedAcceptContext.WebSocketOptions is not null)
155-
{
156-
creationOptions = advancedAcceptContext.WebSocketOptions;
157-
}
156+
enableCompression = advancedAcceptContext.DangerousEnableCompression;
157+
serverContextTakeover = advancedAcceptContext.ServerContextTakeover;
158+
serverMaxWindowBits = advancedAcceptContext.ServerMaxWindowBits;
158159
}
159160

160161
string key = _context.Request.Headers.SecWebSocketKey;
161162

162163
HandshakeHelpers.GenerateResponseHeaders(key, subProtocol, _context.Response.Headers);
163164

164165
WebSocketDeflateOptions? deflateOptions = null;
165-
var ext = _context.Request.Headers.SecWebSocketExtensions;
166-
if (ext.Count != 0)
166+
if (enableCompression)
167167
{
168-
// loop over each extension offer, extensions can have multiple offers we can accept any
169-
foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions))
168+
var ext = _context.Request.Headers.SecWebSocketExtensions;
169+
if (ext.Count != 0)
170170
{
171-
if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal)
172-
&& creationOptions?.DangerousDeflateOptions is not null)
171+
// loop over each extension offer, extensions can have multiple offers we can accept any
172+
foreach (var extension in _context.Request.Headers.GetCommaSeparatedValues(HeaderNames.SecWebSocketExtensions))
173173
{
174-
// We do not want to modify the users options
175-
deflateOptions = CloneWebSocketDeflateOptions(creationOptions.DangerousDeflateOptions);
176-
if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions!, out var response))
174+
if (extension.TrimStart().StartsWith("permessage-deflate", StringComparison.Ordinal))
177175
{
178-
if (CompareDeflateOptions(deflateOptions, creationOptions.DangerousDeflateOptions))
176+
deflateOptions = new WebSocketDeflateOptions()
177+
{
178+
ServerContextTakeover = serverContextTakeover,
179+
ServerMaxWindowBits = serverMaxWindowBits
180+
};
181+
if (HandshakeHelpers.ParseDeflateOptions(extension, deflateOptions, out var response))
179182
{
180-
// avoids allocating a new WebSocketCreationOptions below when checking if we have new deflate options to apply
181-
deflateOptions = null;
183+
// If more extension types are added, this would need to be a header append
184+
// and we wouldn't want to break out of the loop
185+
_context.Response.Headers.SecWebSocketExtensions = response;
186+
break;
182187
}
183-
// If more extension types are added, this would need to be a header append
184-
// and we wouldn't want to break out of the loop
185-
_context.Response.Headers.SecWebSocketExtensions = response;
186-
break;
187188
}
188189
}
189190
}
190191
}
191192

192193
Stream opaqueTransport = await _upgradeFeature.UpgradeAsync(); // Sets status code to 101
193194

194-
WebSocketCreationOptions? options = creationOptions;
195-
if (options is null)
196-
{
197-
options = new WebSocketCreationOptions()
198-
{
199-
IsServer = true,
200-
KeepAliveInterval = keepAliveInterval,
201-
SubProtocol = subProtocol,
202-
DangerousDeflateOptions = deflateOptions,
203-
};
204-
}
205-
else if (deflateOptions is not null)
206-
{
207-
// use a new options instance so we don't modify the users options
208-
options = new WebSocketCreationOptions()
209-
{
210-
IsServer = true,
211-
KeepAliveInterval = creationOptions!.KeepAliveInterval,
212-
SubProtocol = creationOptions!.SubProtocol,
213-
DangerousDeflateOptions = deflateOptions,
214-
};
215-
}
216-
else if (options.IsServer == false)
195+
return WebSocket.CreateFromStream(opaqueTransport, new WebSocketCreationOptions()
217196
{
218-
// use a new options instance so we don't modify the users options
219-
options = new WebSocketCreationOptions()
220-
{
221-
IsServer = true,
222-
KeepAliveInterval = creationOptions!.KeepAliveInterval,
223-
SubProtocol = creationOptions!.SubProtocol,
224-
DangerousDeflateOptions = deflateOptions,
225-
};
226-
}
227-
228-
return WebSocket.CreateFromStream(opaqueTransport, options);
197+
IsServer = true,
198+
KeepAliveInterval = keepAliveInterval,
199+
SubProtocol = subProtocol,
200+
DangerousDeflateOptions = deflateOptions
201+
});
229202
}
230203

231204
private static WebSocketDeflateOptions? CloneWebSocketDeflateOptions([NotNullIfNotNull("options")] WebSocketDeflateOptions? options)
@@ -244,19 +217,6 @@ public async Task<WebSocket> AcceptAsync(WebSocketAcceptContext acceptContext)
244217
};
245218
}
246219

247-
private static bool CompareDeflateOptions(WebSocketDeflateOptions? lhs, WebSocketDeflateOptions? rhs)
248-
{
249-
if (lhs is null || rhs is null)
250-
{
251-
return lhs == rhs;
252-
}
253-
254-
return lhs.ClientContextTakeover == rhs.ClientContextTakeover &&
255-
lhs.ClientMaxWindowBits == rhs.ClientMaxWindowBits &&
256-
lhs.ServerContextTakeover == rhs.ServerContextTakeover &&
257-
lhs.ServerMaxWindowBits == rhs.ServerMaxWindowBits;
258-
}
259-
260220
public static bool CheckSupportedWebSocketRequest(string method, IHeaderDictionary requestHeaders)
261221
{
262222
if (!HttpMethods.IsGet(method))

0 commit comments

Comments
 (0)