Uh oh!
There was an error while loading. Please reload this page.
[WebSocket] Fix handling of BFinal in WebSocket deflate. - #132201
Conversation
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR addresses a permessage-deflate edge case where a DEFLATE BFINAL=1 block can cause the managed WebSocket inflater to stop consuming input and potentially hang receive loops, and it expands the fuzzing infrastructure by adding a WebSocket-focused fuzzer and OneFuzz deployment wiring.
Changes:
- Detect DEFLATE stream termination (
BFINAL=1) with unconsumed trailing compressed bytes and reject the message to avoid infinite no-progress inflation. - Add regression tests covering
BFINAL=1-terminated compressed frames. - Add a new
WebSocketFuzzer, improve local fuzzing ergonomics, and deploy the new fuzzer to OneFuzz.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs | Adds regression tests for BFINAL=1-terminated compressed messages. |
| src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs | Minor adjustment in receive-buffer copy path (assert removal). |
| src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs | Adds BFINAL/stream-end detection and throws to prevent non-terminating inflation. |
| src/libraries/System.Net.WebSockets/src/Resources/Strings.resx | Adds a new resource string for the new rejection condition. |
| src/libraries/Fuzzing/README.md | Updates local fuzzing example invocation. |
| src/libraries/Fuzzing/DotnetFuzzing/Program.cs | Prints PID for file-based runs; adds a default -timeout to generated local-run scripts. |
| src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/WebSocketFuzzer.cs | Introduces a new WebSocket fuzzer harness. |
| eng/pipelines/libraries/fuzzing/deploy-to-onefuzz.yml | Adds OneFuzz deployment step for the new WebSocket fuzzer. |
Suppressed comments (2)
src/libraries/System.Net.WebSockets/tests/WebSocketDeflateTests.cs:706
- Same as above: prefer asserting
SR.net_WebSockets_DataAfterBFinal(andWebSocketErrorCode) rather than a substring match, to align with other WebSocket error-message tests.
var exception = await Assert.ThrowsAsync<WebSocketException>(
async () => await websocket.ReceiveAsync(buffer, CancellationToken));
Assert.Contains("BFINAL", exception.Message);
}
src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/WebSocketFuzzer.cs:112
- The harness uses
cts.CancelAfterto prevent hangs, but it only catchesWebSocketException. If the timeout triggers,OperationCanceledExceptionwill escape and be treated as a crash. Similarly, disposing inside the loop can causeObjectDisposedExceptionto escape.
}
catch (WebSocketException)
{
// Expected
}
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ManickaP
commented
Aug 12, 2026
/ba-g BA is borked |
+fuzzing