fix: TagAsync to buffer stream for authentication retry by wangxiaoxuan273 · Pull Request #355 · oras-project/oras-dotnet

wangxiaoxuan273 · 2026-03-03T01:57:01Z

Fixes #327
Fixes #333

Root cause of the bug:
The TagAsync passes a non-seekable HTTP response stream directly to DoPushAsync, which creates an HTTP PUT request with that stream as the body. When the initial request receives a 401 Unauthorized response, the Client.SendCoreAsync method attempts to retry the request with authentication credentials by cloning the original request. Cloning requires rewinding the content stream, but HTTP response streams (network streams) are non-seekable, causing the exception.

The fix:
This PR wraps the original stream with a rewindable stream before sending it out.

Copilot

Pull request overview

This PR fixes ManifestStore.TagAsync failing during authentication retries by ensuring the pushed manifest content is seekable (buffered) so Auth.Client can rewind/clone the request content when a 401 triggers retry.

Changes:

Buffer FetchAsync’s (non-seekable) response stream into a MemoryStream before calling DoPushAsync in TagAsync.
Add unit tests covering TagAsync behavior with auth retry, buffering integrity across retries, and the no-retry path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`src/OrasProject.Oras/Registry/Remote/ManifestStore.cs`	Buffers fetched manifest content into a seekable stream before pushing to support auth retry.
`tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs`	Adds new TagAsync tests (auth retry, buffering integrity, and basic success without retry).

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

codecov · 2026-03-03T02:08:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.77%. Comparing base (5953fb1) to head (99b289e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #355      +/-   ##
==========================================
+ Coverage   91.76%   91.77%   +0.01%     
==========================================
  Files          64       64              
  Lines        2755     2759       +4     
  Branches      364      365       +1     
==========================================
+ Hits         2528     2532       +4     
  Misses        138      138              
  Partials       89       89

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

The TagAsync method was passing a non-seekable HTTP response stream to DoPushAsync. When authentication retry was needed (e.g., with Azure Container Registry), Auth.Client would clone the request which fails for non-seekable streams. This fix: - Enforces size limit early (before FetchAsync) to fail fast - Buffers the content stream into a seekable MemoryStream using ReadAllAsync - Ensures retries can successfully rewind and re-read the stream content Also adds comprehensive unit tests for TagAsync auth retry scenarios. Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>

Copilot

Pull request overview

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

akashsinghal

Caution

This review was generated by Copilot using Claude Opus 4.6, mimicking the review style of shizhMSFT. This is not the real shizhMSFT.

The fix is correct and the approach is consistent with PushWithIndexingAsync. SGTM overall.

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs

akashsinghal · 2026-03-03T18:42:25Z

Overall, PR looks good to me. @wangxiaoxuan273 can you confirm if you've verified manually that the APIs that were affected by the 2 issues you mentioned are actually resolved now?

wangxiaoxuan273 · 2026-03-04T05:58:33Z

Overall, PR looks good to me. @wangxiaoxuan273 can you confirm if you've verified manually that the APIs that were affected by the 2 issues you mentioned are actually resolved now?

Yes, I have verified manually that this change can fix the two issues.

…note SHA-256 limitation - Consolidate ManifestStore_TagAsync_WithAuthRetry and ManifestStore_TagAsync_BuffersStreamForRetry into a single test that captures content from both PUT attempts and verifies retry count and content integrity - Switch ManifestStore_TagAsync_SucceedsWithoutRetry to use NonSeekableStream for GET response to validate buffering path - Add comment noting ReadAllAsync SHA-256 limitation is pre-existing and tracked separately - Shorten fully-qualified Auth types using using directive - Ensure all new/modified lines stay within 120-column limit Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>

Copilot

Pull request overview

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

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

akashsinghal · 2026-03-04T22:48:03Z

Caution

This review was generated by Copilot using Claude Opus 4.6, mimicking the review style of shizhMSFT. This is not the real shizhMSFT.

Second-round review of PR #355

All 20 prior review threads addressed — the fix is clean and the tests are comprehensive. Two observations:

1. Missing negative test for size guard (ManifestStore.cs:423)
LimitSize(Repository.Options.MaxMetadataBytes) is a new guard added by this PR, but no test exercises the rejection path. Consider adding a test where descriptor.Size > MaxMetadataBytes to verify that TagAsync throws SizeLimitExceededException before attempting to fetch.

2. Inline SHA-256 comment should reference an issue (ManifestStore.cs:431-432)
The comment "ReadAllAsync currently hardcodes SHA-256 for digest verification. This is a pre-existing limitation tracked separately." says "tracked separately" but doesn't reference an issue number. If there's a tracking issue, link it (e.g., // See #NNN). If not, file one. A comment claiming something is "tracked" without a link is not actionable.

Otherwise SGTM — nice work addressing all the previous feedback cleanly.

Copilot AI review requested due to automatic review settings March 3, 2026 01:57

wangxiaoxuan273 requested review from Wwwsylvia, sajayantony and shizhMSFT as code owners March 3, 2026 01:57

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 01:57 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

wangxiaoxuan273 requested a review from Copilot March 3, 2026 02:08

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 02:08 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

wangxiaoxuan273 changed the title ~~Fix TagAsync to buffer stream for authentication retry~~ fix: TagAsync to buffer stream for authentication retry Mar 3, 2026

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 340cce4 to 86358ec Compare March 3, 2026 02:23

Copilot AI review requested due to automatic review settings March 3, 2026 02:23

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 02:24 View session

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 86358ec to 6d343d4 Compare March 3, 2026 02:26

Copilot AI reviewed Mar 3, 2026

View reviewed changes

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs Outdated Show resolved Hide resolved

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 6d343d4 to 0adacb9 Compare March 3, 2026 02:32

wangxiaoxuan273 requested a review from Copilot March 3, 2026 02:40

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 02:41 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 0adacb9 to 89bfa09 Compare March 3, 2026 02:52

wangxiaoxuan273 requested a review from Copilot March 3, 2026 02:55

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 02:56 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs Show resolved Hide resolved

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 89bfa09 to 56cab2b Compare March 3, 2026 03:01

wangxiaoxuan273 requested a review from Copilot March 3, 2026 03:04

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 03:04 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs Outdated Show resolved Hide resolved

wangxiaoxuan273 force-pushed the fix-tagasync-nonseekable-stream branch from 56cab2b to 9f08a4b Compare March 3, 2026 03:09

wangxiaoxuan273 requested a review from Copilot March 3, 2026 03:10

Copilot started reviewing on behalf of wangxiaoxuan273 March 3, 2026 03:11 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

akashsinghal reviewed Mar 3, 2026

View reviewed changes

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs Outdated Show resolved Hide resolved

akashsinghal reviewed Mar 3, 2026

View reviewed changes

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs Outdated Show resolved Hide resolved

akashsinghal reviewed Mar 3, 2026

View reviewed changes

tests/OrasProject.Oras.Tests/Registry/Remote/ManifestStoreTest.cs Show resolved Hide resolved

wangxiaoxuan273 requested a review from Copilot March 4, 2026 06:14

Copilot started reviewing on behalf of wangxiaoxuan273 March 4, 2026 06:15 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Conversation

wangxiaoxuan273 commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

akashsinghal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akashsinghal commented Mar 3, 2026

Uh oh!

wangxiaoxuan273 commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

akashsinghal commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wangxiaoxuan273 commented Mar 3, 2026 •

edited

Loading

codecov bot commented Mar 3, 2026 •

edited

Loading