Enforce HttpClient limits on GetFromJsonAsync

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.

[MihaZupan]

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

[ghost]

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

Issue Details

---

Enforces HttpClient.Timeout and HttpClient.MaxResponseContentBufferSize on the whole response (including the content transfer) to match HttpClient.Get{String/ByteArray}Async.

Author:	MihaZupan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[stephentoub]

Can we avoid allocating this wrapper stream if MaxResponseContentBufferSize is the default int.MaxValue? Not just about this allocation, but about the increased cost on every individual read operation.

[MihaZupan]

Sure.
Looks like the DeleteAsync overload doesn't use ResponseHeadersRead, so we can avoid all of this overhead there as well.

While on the topic, wrapper streams like these are the primary reason why I opened #77935. The per-call allocation overhead disappears with zero-byte reads.
Another similar example is the WebSockets telemetry in YARP where we intercept a read stream like this, but it adds almost no overhead because YARP does zero-byte reads by default

[MihaZupan]

Looking at this again, I don't think we should avoid this wrapper - we'd still want to prevent the deserialize from reading more than 2 GB in that case.

[stephentoub]

The per-call allocation overhead disappears with zero-byte reads.

How so?

[MihaZupan]

The read in the wrapper stream is implemented like so:

ValueTask<int>readTask=_innerStream.ReadAsync(buffer,cancellationToken);if(buffer.IsEmpty){returnreadTask;}if(readTask.IsCompletedSuccessfully){intread=readTask.Result;CheckLengthLimit(read);returnnewValueTask<int>(read);}returnCore(readTask);// Allocate

Assuming the caller is using zero-byte reads, you will alternate between

• The buffer.IsEmpty branch for the zero-byte read where we'll just pass through the task as we're not interested in the result (it won't affect the read limit)
• The readTask.IsCompletedSuccessfully branch after a zero-byte read as data is available immediately

You'll only hit the allocating path if the underlying stream doesn't support zero-byte reads, or if you hit an exception.

The WebSocket telemetry stream I mentioned is using exactly the same pattern. The result of the zero-byte read doesn't affect us as there isn't any data to parse.