Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ private async Task<string> GetStringAsyncCore(HttpRequestMessage request, Cancel

(CancellationTokenSource cts, bool disposeCts, CancellationTokenSource pendingRequestsCts) = PrepareCancellationTokenSource(cancellationToken);
HttpResponseMessage? response = null;
HttpContent.LimitArrayPoolWriteStream? buffer = null;
try
{
// Wait for the response message and make sure it completed successfully.
Expand All @@ -199,7 +200,7 @@ private async Task<string> GetStringAsyncCore(HttpRequestMessage request, Cancel

// Since the underlying byte[] will never be exposed, we use an ArrayPool-backed
// stream to which we copy all of the data from the response.
using var buffer = new HttpContent.LimitArrayPoolWriteStream(
buffer = new HttpContent.LimitArrayPoolWriteStream(
_maxResponseContentBufferSize,
c.Headers.ContentLength.GetValueOrDefault(),
getFinalSizeFromPool: true);
Expand All @@ -224,6 +225,7 @@ private async Task<string> GetStringAsyncCore(HttpRequestMessage request, Cancel
}
finally
{
buffer?.ReturnAllPooledBuffers();
FinishSend(response, cts, disposeCts, telemetryStarted, responseContentTelemetryStarted);
}
}
Expand Down Expand Up @@ -254,6 +256,7 @@ private async Task<byte[]> GetByteArrayAsyncCore(HttpRequestMessage request, Can

(CancellationTokenSource cts, bool disposeCts, CancellationTokenSource pendingRequestsCts) = PrepareCancellationTokenSource(cancellationToken);
HttpResponseMessage? response = null;
HttpContent.LimitArrayPoolWriteStream? buffer = null;
try
{
// Wait for the response message and make sure it completed successfully.
Expand All @@ -275,7 +278,7 @@ private async Task<byte[]> GetByteArrayAsyncCore(HttpRequestMessage request, Can
// the buffer potentially several times and that it's unlikely the underlying buffer
// at the end will be the exact size needed, in which case it's more beneficial to use
// ArrayPool buffers and copy out to a new array at the end.
using var buffer = new HttpContent.LimitArrayPoolWriteStream(
buffer = new HttpContent.LimitArrayPoolWriteStream(
_maxResponseContentBufferSize,
c.Headers.ContentLength.GetValueOrDefault(),
getFinalSizeFromPool: false);
Expand All @@ -299,6 +302,7 @@ private async Task<byte[]> GetByteArrayAsyncCore(HttpRequestMessage request, Can
}
finally
{
buffer?.ReturnAllPooledBuffers();
FinishSend(response, cts, disposeCts, telemetryStarted, responseContentTelemetryStarted);
}
}
Expand Down
17 changes: 10 additions & 7 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ internal void LoadIntoBuffer(long maxBufferSize, CancellationToken cancellationT
}
catch (Exception e)
{
tempBuffer.Dispose();
tempBuffer.ReturnAllPooledBuffers();

if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, e);

Expand Down Expand Up @@ -499,7 +499,7 @@ public Task LoadIntoBufferAsync(long maxBufferSize, CancellationToken cancellati
}
catch (Exception e)
{
tempBuffer.Dispose();
tempBuffer.ReturnAllPooledBuffers();

if (StreamCopyExceptionNeedsWrapping(e))
{
Expand All @@ -519,7 +519,7 @@ private async Task LoadIntoBufferAsyncCore(Task serializeToStreamTask, LimitArra
}
catch (Exception e)
{
tempBuffer.Dispose(); // Cleanup partially filled stream.
tempBuffer.ReturnAllPooledBuffers(); // Cleanup partially filled stream.
Exception we = GetStreamCopyException(e);
if (we != e) throw we;
throw;
Expand Down Expand Up @@ -653,7 +653,7 @@ protected virtual void Dispose(bool disposing)

if (IsBuffered)
{
_bufferedContent.Dispose();
_bufferedContent.ReturnAllPooledBuffers();
}
}
}
Expand Down Expand Up @@ -843,8 +843,11 @@ public LimitArrayPoolWriteStream(int maxBufferSize, long expectedFinalSize, bool

protected override void Dispose(bool disposing)
{
ReturnAllPooledBuffers();
base.Dispose(disposing);
// User code must never dispose this stream. It is an internal implementation detail
// exposed to user-provided HttpContent.SerializeToStream(Async) overrides, and the
// lifetime of the underlying pooled buffers is owned by HttpContent, not the user.
// All internal cleanup goes through ReturnAllPooledBuffers directly.
throw new InvalidOperationException();
Comment thread
MihaZupan marked this conversation as resolved.
Comment thread
MihaZupan marked this conversation as resolved.
}

/// <summary>Should only be called once.</summary>
Expand Down Expand Up @@ -1075,7 +1078,7 @@ public void CopyToCore(Span<byte> destination)
_lastBuffer.AsSpan(0, _lastBufferOffset).CopyTo(destination);
}

private void ReturnAllPooledBuffers()
internal void ReturnAllPooledBuffers()
{
if (_pooledBuffers is byte[]?[] buffers)
{
Expand Down
22 changes: 22 additions & 0 deletions src/libraries/System.Net.Http/tests/UnitTests/HttpContentTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ public async Task Dispose_BufferContentThenDisposeContent_BufferedStreamGetsDisp
Assert.Null(bufferedStream.GetSingleBuffer());
}

[Fact]
public async Task SerializeToStreamAsync_UserDisposesBufferedStream_Throws()
{
var content = new DisposeBufferedStreamContent();
await Assert.ThrowsAsync<InvalidOperationException>(() => content.LoadIntoBufferAsync());
}

private sealed class DisposeBufferedStreamContent : HttpContent
{
protected override Task SerializeToStreamAsync(Stream stream, TransportContext context)
{
stream.Dispose();
return Task.CompletedTask;
}

protected internal override bool TryComputeLength(out long length)
{
length = 0;
return false;
}
}

[Theory]
[InlineData(1, 100, 99, 1)]
[InlineData(1, 100, 50, 99)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public async Task MemoryStream_To_LimitMemoryStream_NoCapacity(bool sourceIsExpo
{
byte[] input = CreateByteArray(8192);
MemoryStream source = CreateSourceMemoryStream(sourceIsExposable, input);
using var destination = new HttpContent.LimitArrayPoolWriteStream(int.MaxValue, 0, getFinalSizeFromPool: false);
var destination = new HttpContent.LimitArrayPoolWriteStream(int.MaxValue, 0, getFinalSizeFromPool: false);

await StreamToStreamCopy.CopyAsync(source, destination, 4096, disposeSource);

Expand All @@ -128,7 +128,7 @@ public async Task MemoryStream_To_LimitMemoryStream_EqualCapacity(bool sourceIsE
{
byte[] input = CreateByteArray(8192);
MemoryStream source = CreateSourceMemoryStream(sourceIsExposable, input);
using var destination = new HttpContent.LimitArrayPoolWriteStream(int.MaxValue, input.Length, getFinalSizeFromPool);
var destination = new HttpContent.LimitArrayPoolWriteStream(int.MaxValue, input.Length, getFinalSizeFromPool);
Comment thread
MihaZupan marked this conversation as resolved.

await StreamToStreamCopy.CopyAsync(source, destination, 4096, disposeSource);

Expand Down Expand Up @@ -292,13 +292,14 @@ public void LimitMemoryStream_ResizingLogicWorks(bool getFinalSizeFromPool)
}
else
{
using var smallDestination = new HttpContent.LimitArrayPoolWriteStream(maxBufferSize: actualSize - 1, expectedSize, getFinalSizeFromPool);
var smallDestination = new HttpContent.LimitArrayPoolWriteStream(maxBufferSize: actualSize - 1, expectedSize, getFinalSizeFromPool);
capacityEx = Assert.Throws<HttpRequestException>(() => WriteChunks(smallDestination, actualSize));
smallDestination.ReturnAllPooledBuffers();
}

Assert.Equal(HttpRequestError.ConfigurationLimitExceeded, capacityEx.HttpRequestError);

using var destination = new HttpContent.LimitArrayPoolWriteStream(maxBufferSize: actualSize + 42, expectedSize, getFinalSizeFromPool);
var destination = new HttpContent.LimitArrayPoolWriteStream(maxBufferSize: actualSize + 42, expectedSize, getFinalSizeFromPool);
WriteChunks(destination, actualSize);

if (!getFinalSizeFromPool && expectedSize == actualSize)
Expand All @@ -323,6 +324,8 @@ public void LimitMemoryStream_ResizingLogicWorks(bool getFinalSizeFromPool)
{
Assert.True(actualSize <= destination.GetSingleBuffer().Length);
}

destination.ReturnAllPooledBuffers();
}
}

Expand Down
Loading