Skip to content

Commit

Permalink
[Core] Don't throw from Response.Content if memory stream is private
Browse files Browse the repository at this point in the history
Relax our constraint that the underlying buffer of the MemoryStream
backing a buffered response be publicly visible. There are cases today
where `RequestBodyPolicy` will not allocate a new MemoryStream. One
example is if the response stream is already seekable (as is the case
in our PlaybackTransport) nd we've observed that there are likely
cases where `HttpClient` itself may use a MemoryStream as the response
stream without allowing the underling buffer to be exposed.

In cases where we can not extract the underlying body, just make a
copy of it. Since we know the underlying stream is a memory stream, we
don't need to worry about hidden IO.

Fixes #21048
  • Loading branch information
ellismg committed May 26, 2021
1 parent 2fc1976 commit 2520f57
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
4 changes: 4 additions & 0 deletions sdk/core/Azure.Core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Types to represent `GeoJson` primitives.

### Changed

- `Response.Content` no longer throws `InvalidOperationException` when the response is backed by a `MemoryStream` with a non publicly visible buffer.

## 1.14.0 (2021-05-11)

### Features Added
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/Azure.Core/src/Response.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public virtual BinaryData Content
}
else
{
throw new InvalidOperationException($"The {nameof(ContentStream)}'s internal buffer cannot be accessed.");
return new BinaryData(memoryContent.ToArray());
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions sdk/core/Azure.Core/tests/ResponseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,15 @@ public void ContentPropertyThrowsForNonMemoryStream()
Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
}

public void ContentPropertyThrowsForNotExposableMemoryStream()
[Test]
public void ContentPropertyWorksForMemoryStreamsWithPrivateBuffers()
{
var response = new MockResponse(200);
response.ContentStream = new MemoryStream(new byte[100], 0, 100, writable: false, publiclyVisible: false);
var responseBody = new byte[100];
response.ContentStream = new MemoryStream(responseBody, 0, responseBody.Length, writable: false, publiclyVisible: false);

Assert.Throws<InvalidOperationException>(() => { BinaryData d = response.Content; });
Assert.DoesNotThrow(() => { BinaryData d = response.Content; });
CollectionAssert.AreEqual(responseBody, response.Content.ToArray());
}

internal class TestPayload
Expand Down

0 comments on commit 2520f57

Please sign in to comment.