Big-endian fix: ApplyMask in System.Net.WebSockets.ManagedWebSocket #55422
Conversation
* Fix mask shifting logic for big-endian systems
ghost
added
the
|
Tagging subscribers to this area: @dotnet/ncl Issue Details
The mask shifting logic assumed little-endian byte order, which could lead to incorrectly decoded messages depending on buffer alignment. This caused failures in some aspnetcore tests (Microsoft.AspNetCore.SignalR.Client.FunctionalTests), which are fixed by this patch.
|
That's very interesting, those tests use Kestrel which doesn't support BigEndian, so not sure how they can pass with this change |
Ah, of course. I should have mentioned that I'm using a stack of patches on top of the aspnetcore code base as well, to fix endian issues in particular (mostly around string handling in Kestrel, including all that generated KnownHeaders code, but also around SockAddr processing, the RenderBatch stuff, and a few others). I'm planning to submit those once I've cleaned them up a bit, but I wanted to go through the test suite failures first to make sure I'm not missing anything. That's when I ran into this issue, which is actually in the runtime, so I submitted this PR now just to get that out of the way. |
|
What's the testing plan to avoid/catch big endian regressions in the future? |
Well, ideally we could get (big-endian) s390x machines into your CI environment. We have made machines available, and there's currently CI running on the s390x feature branch in runtimelab. Over time, we hope this can be moved over to the main runtime repo, and then also get enabled for some of the other major dotnet components, certainly including aspnetcore. There's still a ways to go to get there; in particular we don't have automated binary builds of the runtime/SDK for s390x yet - we're in discussions how to get that going. As a fallback, we can always do regular runs of the test suites offline on our own machines as well. |
The mask shifting logic assumed little-endian byte order, which could lead to incorrectly decoded messages depending on buffer alignment. This caused failures in some aspnetcore tests (Microsoft.AspNetCore.SignalR.Client.FunctionalTests), which are fixed by this patch.