dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.42k stars 4.76k forks source link

[Network] Provide an API to set the max buffer size to be used in a HttpMessageHandler. #109991

Open mandel-macaque opened 2 days ago

mandel-macaque commented 2 days ago

Describe the bug

Currently, in the donet network stack, when handling uploads, there is no way for a developer to limit the maximum amount of memory used by a memory stream to avoid excessive memory consumption on devices with limited resources, for example, when using the Microsoft.iOS workload. The HttpContent.MaxBufferSize is an internal variable.

Describe the solution you'd like

Provide a public API so that an implementation of HttpMessageHandler can check the maximum buffers size desired to be used by the HttpClient caller.

Expected Behaviour:

This new API can be used by HttpMessageHandler implementations to ensure that we can have the same behaviour in the different platforms and limit the platform specific code.

Additional context

This came to light when working on issue https://github.com/xamarin/xamarin-macios/issues/21537 in the Microsoft.iOS workload. A user found an unexpected behaviour because the iOS implementation of the message hander uses long.MaxValue instead of int.MaxValue, there is also a complain that there is no way to se max buffer size

We have created PR https://github.com/xamarin/xamarin-macios/pull/21660 to use int.Max as the HttpContent.MaxBufferSize does, but if the value changes the Microsoft.iOS implementation will be out of line.

dotnet-policy-service[bot] commented 2 days ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

MihaZupan commented 2 days ago

HttpClient on its own will not buffer the request body unless you explicitly call HttpContent.LoadIntoBufferAsync. You are free to send a TB of data through HttpClient/SocketsHttpHandler via StreamContent for example, and it'll not get buffered up front. HttpContent.MaxBufferSize (or the parameter to LoadIntoBufferAsync) also isn't a suggestion for how much to buffer, it's a hard limit after which the request/response fails.

It is up to the HttpMessageHandler to decide whether to buffer everything or not. It's generally not a great idea to do so due to the memory usage drawbacks you mentioned. I think the solution here is to figure out why the iOS implementation is buffering everything upfront in the first place, and if it can avoid doing so. If it has a good reason to buffer requests upfront, any limits imposed on that are an implementation detail specific to that handler, not the overall abstraction (if it were shared configuration, it would default to 0, as that's the behavior you'll see with other handlers).

dotnet-policy-service[bot] commented 2 days ago

Tagging subscribers to 'os-ios': @vitek-karas, @kotlarmilos, @ivanpovazan, @steveisok, @akoeplinger See info in area-owners.md if you want to be subscribed.