Skip to content

Make HostedFileDownloadStream explicitly read-only#7394

Merged
jozkee merged 3 commits intomainfrom
copilot/update-hosted-file-download-stream
Mar 16, 2026
Merged

Make HostedFileDownloadStream explicitly read-only#7394
jozkee merged 3 commits intomainfrom
copilot/update-hosted-file-download-stream

Conversation

Copy link
Contributor

Copilot AI commented Mar 14, 2026

HostedFileDownloadStream did not consistently advertise or enforce its read-only contract at the base type. This change makes the abstraction explicitly non-writable so all implementations inherit the same behavior by default.

  • Base stream contract

    • Override CanWrite in HostedFileDownloadStream to always return false
    • Make all write-entry points throw NotSupportedException at the base class:
      • SetLength
      • BeginWrite / EndWrite
      • WriteByte
      • Write(byte[], ...)
      • WriteAsync(byte[], ...)
      • Write(ReadOnlySpan<byte>)
      • WriteAsync(ReadOnlyMemory<byte>)
  • Implementation cleanup

    • Remove duplicated write-path overrides from OpenAIFileDownloadStream
    • Let the concrete implementation inherit the shared read-only behavior from the base type
  • Coverage

    • Add focused tests asserting:
      • CanWrite == false
      • write-related APIs throw NotSupportedException
public override bool CanWrite => false;

public override void Write(byte[] buffer, int offset, int count) =>
    throw new NotSupportedException();

public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
    throw new NotSupportedException();
Microsoft Reviewers: Open in CodeFlow

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes HostedFileDownloadStream explicitly read-only at the base Stream contract level, so all download-stream implementations consistently advertise and enforce non-writability.

Changes:

  • Override CanWrite to always return false on HostedFileDownloadStream and make all write entry points throw NotSupportedException.
  • Remove redundant write-related overrides from OpenAIFileDownloadStream so it inherits the base read-only behavior.
  • Add unit tests verifying CanWrite == false and that write APIs throw NotSupportedException.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Files/HostedFileDownloadStreamTests.cs Adds coverage asserting the base stream is read-only and write APIs throw.
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIFileDownloadStream.cs Removes duplicated write-path overrides to rely on the base read-only contract.
src/Libraries/Microsoft.Extensions.AI.Abstractions/Files/HostedFileDownloadStream.cs Centralizes read-only enforcement in the abstraction by overriding write members to throw.

You can also share your feedback on Copilot code review. Take the survey.

@github-actions github-actions bot added the area-ai Microsoft.Extensions.AI libraries label Mar 14, 2026
@jozkee jozkee merged commit c15aba0 into main Mar 16, 2026
12 checks passed
@jozkee jozkee deleted the copilot/update-hosted-file-download-stream branch March 16, 2026 16:15
jeffhandley pushed a commit to jeffhandley/extensions that referenced this pull request Mar 17, 2026
* Initial plan

* Make HostedFileDownloadStream read-only by default

Co-authored-by: stephentoub <[email protected]>

* Fix HostedFileDownloadStream write overload ordering

Co-authored-by: stephentoub <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: stephentoub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants