dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.28k stars 9.96k forks source link

[Feature Request] SignalR response compression with new websockets compression in Asp.Net Core 6 #38230

Open douglasg14b opened 2 years ago

douglasg14b commented 2 years ago

Please describe.

Response compression, not much else to say there. It's pretty necessary for mobile devices and the like. Especially in data-intensive scenarios.

A 8MB JSON response being compressed down to 700KB is nearly required for mobile devices.

Asp.Net Core 6 seems to have shipped with websockets compression support (#2715), will signalR be taking advantage of this?

adityamandaleeka commented 2 years ago

Triage: we should discuss this further. We'd either need to modify our abstractions to support this, or just have a global option to enable compression on the transport. There are tradeoffs to consider here so it would require more design/discussion.

@douglasg14b You may also want to take a look at https://docs.microsoft.com/aspnet/core/signalr/messagepackhubprotocol?view=aspnetcore-6.0

douglasg14b commented 2 years ago

@adityamandaleeka

Thanks for the response!

A glance at that page shows that MessagePack would produce smaller responses than JSON, however, it doesn't mention actually compressing the responses? I'm guessing it's because it saves on syntax?

In my case at least, the response size is because of repetitive data that compresses pretty well (coordinates and the like).

I look forward to hearing more.

adityamandaleeka commented 2 years ago

@douglasg14b In the doc I linked above, there's a part that says:

To customize how MessagePack formats data, AddMessagePackProtocol takes a delegate for configuring options. In that delegate, the SerializerOptions property is used to configure MessagePack serialization options. For more information on how the resolvers work, visit the MessagePack library at MessagePack-CSharp. Attributes can be used on the objects you want to serialize to define how they should be handled.

There's some example code there too for how the configuration might look.

That MessagePack-CSharp repo shows more info about configuration options, including the option to enable LZ4 compression: https://github.com/neuecc/MessagePack-CSharp#lz4-compression

douglasg14b commented 2 years ago

Gotcha, the doc could be a bit clearer then, I Ctrl+F for compression and didn't find any hits, and the above quote doesn't allude to it either unless you already have prior knowledge of messagepack configuration.

Either way, thanks for the info.

luryus commented 2 years ago

We also have a SignalR application that would greatly benefit from message compression. Unfortunately the MessagePack LZ4 compression is specific to the MessagePack-CSharp library and as far as I know, it does not work with JavaScript SignalR clients. (https://stackoverflow.com/questions/65087721/net-5-signalr-messagepack-lz4-error-unable-to-find-ext-type-98)

We'd either need to modify our abstractions to support this, or just have a global option to enable compression on the transport.

Couldn't compression support just be configured in the hub specific Websocket options? Maybe something like this:

endpoints.MapHub<SampleHub>("/samplehub", opts =>
{
    opts.WebSockets.EnableCompression = true;
});
nicolasmohamed commented 2 years ago

Hi @adityamandaleeka any news regarding SignalR websockets compression?

Thanks

kolesnick commented 2 years ago

Vote + for this one, because SignalR + MessagePack doesn't work in Unity projects involving IL2CPP.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

douglasg14b commented 1 year ago

Hi, even if this won't be natively supported is there a recommended API surface area with the JS client and C# server to transparently compress/decompress data?

It seems like we can compress all responses with a Filter on the C# side, but on the JS side how can we run every call through a decompression function of our own writing? Can we inject some code between the call & the routing to a function JS side? Without building our own micro-framework client-side.

luryus commented 1 year ago

Any chance this could still be included in .NET 8?

I just prototyped the SignalR changes required here, and they are quite minimal. Only one new boolean flag has to be added to WebSocketOptions: https://github.com/luryus/aspnetcore/commit/cef2a7b25f0081b41e36e67c1d707af97c572a1a

With this enabled the JS SignalR library just automatically changes to use compressed messages. With the C# client some additional configuration is needed, but it can already be done with the current library version.

Cost:L seems a bit excessive? Or am I missing some considerations here?

@adityamandaleeka

BrennanConroy commented 1 year ago

https://devblogs.microsoft.com/dotnet/announcing-net-6/#websocket-compression:

Compression used with encryption may lead to attacks, like CRIME and BREACH. It means that a secret cannot be sent together with user-generated data in a single compression context, otherwise that secret could be extracted. To bring a user’s attention to these implications and help them weigh the risks, we named one of the key APIs DangerousDeflateOptions. We also added the ability to turn off compression for specific messages, so if the user would want to send a secret, they could do that securely without compression.

The reason it's marked Cost:L is because we would want to enable the ability to turn off compression per message to avoid CRIME and BREACH attacks on sensitive data. Just having a global on switch without the option to turn it off is not good. Additionally, it looks like browsers don't even support per message opt-out of compression which makes it even more dangerous to enable this feature without some consideration for preventing browsers from using compression.

Other thoughts; should pings be compressed? They are only 11 bytes in json, 3 bytes in msgpack. How much would the message actually be compressed? Is the cpu cost worth the small size savings? Should we also compress other small messages or find some threshold where we think it becomes useful to compress the message?

douglasg14b commented 1 year ago

@BrennanConroy Couldn't we have 1st party layer 6 compression support instead? Which provides the benefits for most, and avoids both the issue you described, and browser compatibility problems?

luryus commented 1 year ago

The reason it's marked Cost:L is because we would want to enable the ability to turn off compression per message to avoid CRIME and BREACH attacks on sensitive data. Just having a global on switch without the option to turn it off is not good.

I get your point, and those are certainly valid considerations.

However, I feel that not exposing the compression feature unnecessarily closes out many valid and secure use cases for SignalR. In our case, we're using it to stream a lot of structural, well-compressing data over websockets. There's nothing sensitive in the messages so CRIME, BREACH etc. do not really apply. And the compression overhead for pings etc. would be negligible.

We're currently working around this by compressing the data manually before sending, and decompressing it in the browser using a JavaScript library. This technically works, but it's a bit of an awkward and suboptimal solution when there's already native support for compression both in the browser and the server.

Wouldn't it be sufficient to just have the field name clearly indicate that enabling the compression might cause security issues in some scenarios (DangerousEnableCompression)? That way the SignalR applications could make the decision based on their own usage patterns and take advantage of the existing features for compression. This would be an awesome first step while the more granular options are designed and implemented some time later.

FWIW SignalR already can do message compression with long-polling if the ResponseCompression middleware is enabled. I could see the websocket compression as a similar feature to this: compression can be used if the user explicitly enables the compression and bypasses all the "dangerous" warnings.

claudiuciumedean commented 1 year ago

I was wondering what is the status of this, of course I can still see that feature request is still open but is there any plan on implementing and shipping it in the foreseeable future (perhaps in a minor version of .NET 8)?

I am asking since we are currently using SignalR for our WS communication and doing our own custom compression / decompression of messages, and would benefit greatly if SignalR implemented it as in the case of plain .NET WS implementation by using DangerousEnableCompression flag.