fix(registry): resolve relative redirect Location in GetBlobLocationAsync

[akashsinghal]

What this PR does / why we need it

BlobStore.GetBlobLocationAsync rejected redirect responses whose Location header was a relative reference, throwing HttpIOException("redirect Location header must be an absolute URI").

That was stricter than HTTP semantics. RFC 9110 §10.2.2 defines Location = URI-reference and requires a relative reference to be resolved against the effective request URI. The OCI Distribution Spec v1.1.1 does not describe blob-pull redirects at all (it has no mention of redirects or any 3xx status code), but it does explicitly permit a relative Location for upload sessions, citing RFC 7231 §7.1.2 — and this library already resolves those in PushAsync and MountAsync.

This PR makes the blob redirect path consistent with both:

var blobLocation = location.IsAbsoluteUri ? location : new Uri(url, location);

The base is the request URI built earlier in the method, which the existing auto-redirect guard already proves equal to response.RequestMessage.RequestUri. HTTPS validation now runs against the resolved URI.

Behavioral change

A relative Location now returns a resolved URL instead of throwing. No API signatures change, so no major version bump appears necessary, but the error-path behavior change is worth a release note.

Three consequences worth reviewer attention:

• The HTTPS guard can no longer fire for a relative Location, because a resolved relative reference always inherits the registry's scheme. It still guards absolute http:// locations.
• A protocol-relative reference such as //storage.example.com/blob reports IsAbsoluteUri == false yet resolves to a different host, so it is now accepted where it previously threw. This is not a new capability for a hostile registry, which could already return an absolute https://evil.example.com/blob; restricting only the relative branch to same-origin would be asymmetric and would reintroduce the RFC non-conformance. The behavior is pinned by a test rather than left implicit.
• Because a relative location resolves onto the registry host, the returned URL may require registry credentials rather than being the direct storage-backend URL the API is normally used for. The XML docs on BlobStore and IBlobLocationProvider now state this.

Tests

The relative-URI case moved out of BlobStore_GetBlobLocationAsync_Errors into a new [Theory] BlobStore_GetBlobLocationAsync_RelativeLocation:

Location	PlainHttp	Resolved
/storage/blobs/test	true	http://localhost:5000/storage/blobs/test
/storage/blobs/test	false	https://localhost:5000/storage/blobs/test
//storage.example.com/blob	false	https://storage.example.com/blob
?token=xyz	false	https://localhost:5000/v2/test/blobs/{digest}?token=xyz

The last two cover the cross-host form discussed above and the pre-signed-token form registries commonly emit. Existing negative cases (missing Location, non-HTTPS absolute location, 404) are unchanged. Full suite: 637 passing.

Follow-up: HTTP(S) scheme validation

Per review feedback, the scheme check previously ran only when PlainHttp was disabled, so with PlainHttp = true a Location of file:///c:/secret, javascript:alert(1), or \\host\share (which parses as an absolute file:// URI with a foreign host) was returned to the caller unvalidated. The resolved location must now be HTTPS, or HTTP when PlainHttp is enabled. The check lives in the remote BlobStore rather than the IBlobLocationProvider contract, so a local or third-party implementation remains free to return other schemes.

Additional tests: the redirect target is never contacted (exactly one request, to the registry blob endpoint), non-HTTP(S) schemes are rejected in both PlainHttp modes, and explicit ports plus malformed relative values are pinned.

Which issue(s) this PR resolves / fixes

N/A

Please check the following list

• Does the affected code have corresponding tests, e.g. unit test, E2E test?
• Does this change require a documentation update? — XML docs on BlobStore.GetBlobLocationAsync and IBlobLocationProvider.GetBlobLocationAsync updated.
• Does this introduce breaking changes that would require an announcement or bumping the major version? — Behavioral only: a previously throwing path now returns a value. No API surface change; flagging for an announcement note rather than a major bump.
• Do all new files have an appropriate license header? — No new files.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (2e4b98d) to head (4a6c2a2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
+ Coverage   93.32%   93.34%   +0.01%     
==========================================
  Files          69       69              
  Lines        3463     3470       +7     
  Branches      428      431       +3     
==========================================
+ Hits         3232     3239       +7     
  Misses        139      139              
  Partials       92       92              

☔ View full report in Codecov by Harness.
📢 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

This PR fixes BlobStore.GetBlobLocationAsync redirect handling to accept relative Location headers by resolving them against the effective request URI (RFC 9110 §10.2.2), aligning blob redirect behavior with existing upload-session redirect handling in PushAsync/MountAsync.

Changes:

• Resolve relative Location values to an absolute Uri before returning and before enforcing the HTTPS requirement.
• Update XML docs on BlobStore.GetBlobLocationAsync and IBlobLocationProvider.GetBlobLocationAsync to note relative-location resolution and credential implications.
• Add focused theory-based test coverage for several relative Location shapes (path-absolute, query-only, and protocol-relative) and remove the old “relative location throws” error-case.

Reviewed changes

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

File	Description
src/OrasProject.Oras/Registry/Remote/BlobStore.cs	Resolves relative redirect Location values against the request URI and applies HTTPS validation to the resolved URI.
src/OrasProject.Oras/Registry/IBlobLocationProvider.cs	Documents the resolved-relative behavior and that the result may require registry credentials.
tests/OrasProject.Oras.Tests/Registry/Remote/RepositoryTest.cs	Adds theory coverage for relative Location redirect behavior and updates the error test to remove the prior “relative throws” expectation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sajayantony]

Consider if you want to incorporate this

AI Feedback

Recommended non-blocking tests

Use two real HTTP endpoints to verify the default authenticated client makes exactly one registry request and never contacts an absolute or scheme-relative redirect target.
Verify the target endpoint receives no Authorization, cookies, or custom secret headers.
Cover malformed raw Location values, HTTPS downgrade rejection, explicit ports, userinfo, and multi-hop redirects.
Document the injected-client hazard with a deliberately misconfigured auto-redirect client, showing that Authorization is removed but a custom test secret header can be forwarded.

[akashsinghal]

Thanks — I dug into each item. Took three, skipping the rest with reasoning below.

Added in cb5511a

1. Scheme allowlist — this found a real gap. The check was if (!PlainHttp && scheme != "https"), so with PlainHttp = true there was no scheme validation at all. Verified against actual Uri parsing:

raw Location	IsAbsoluteUri	returned before, with PlainHttp = true
\\evil.example.com\blob	true	file://evil.example.com/blob
javascript:alert(1)	true	returned as-is
file:///c:/secret	true	returned as-is

The resolved location must now be https, or http when PlainHttp is set. The check lives in the remote BlobStore rather than the IBlobLocationProvider XML contract, so a future local/oci-layout store or third-party IRepository stays free to return other schemes.

2. Redirect target is never contacted. BlobStore_GetBlobLocationAsync_DoesNotContactRedirectTarget records every request URI and asserts exactly one, against the registry blob endpoint — pinning the API's core "don't download the blob" promise.

3. Explicit port and malformed values pinned in the theory: //storage.example.com:8443/blob, plus ht!tp://[bad, which parses as a relative reference and so stays on the registry host instead of escaping cross-origin.

Skipped, with reasons

• Two real HTTP endpoints — the suite has no real-server infrastructure (no Kestrel/HttpListener/TestServer); every test uses a Moq DelegatingHandler. The two assertions this would buy (exactly one request; no Authorization at the target) are already provable with the mock, because the target is never contacted at all.
• Multi-hop redirects — GetBlobLocationAsync never follows a redirect, so there is no multi-hop path to exercise.
• No Authorization/cookies at the target — trivially true for the same reason.
• Injected-client secret-header forwarding — a genuine hazard, but pre-existing and orthogonal to relative-Location resolution. When a consumer supplies AllowAutoRedirect = true, .NET follows the redirect and strips Authorization cross-origin but forwards custom headers, and our guard only detects this after the request has gone out. I'd rather file a follow-up issue against the redirect model than grow this PR — happy to do that if you agree.
• Userinfo — deliberately skipped; pinning it would add a user:pass@ literal to the test source for very little signal.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/OrasProject.Oras/Registry/Remote/BlobStore.cs:365

• The invalid-scheme error message formats the rejected URI as {scheme}://{host}, which drops important details like the port (and can be misleading for non-authority schemes). Including the full resolved Location URI makes diagnostics more accurate.

                        var expectedSchemes = Repository.Options.PlainHttp ? "HTTP or HTTPS" : "HTTPS";
                        throw new HttpIOException(HttpRequestError.InvalidResponse,
                            $"{response.RequestMessage?.Method} {response.RequestMessage?.RequestUri}: " +
                            $"redirect location must use {expectedSchemes}, " +
                            $"got {blobLocation.Scheme}://{blobLocation.Host}");

tests/OrasProject.Oras.Tests/Registry/Remote/RepositoryTest.cs:1326

• This assertion only checks for the substring "HTTPS" even when plainHttp is true (where the expected message is "HTTP or HTTPS"). That makes the test less precise and could miss regressions in the PlainHttp branch.

        var exception = await Assert.ThrowsAsync<HttpIOException>(async () =>
            await store.GetBlobLocationAsync(blobDesc, new CancellationToken()));
        Assert.Contains("HTTPS", exception.Message);
    }

[akashsinghal]

Extra context on why this matters in practice: relative Location headers on blob redirects are not hypothetical. Google Artifact Registry returns them today, so a client that requires an absolute URI here simply fails against those registries.

That makes this less a spec-conformance cleanup and more an interoperability fix — resolving per RFC 9110 §10.2.2 is what lets us work with the registries that are already doing this.

[akashsinghal]

One more consistency pass (4a6c2a2). HttpResponseMessageExtensions already establishes how this SDK handles a malformed URI reference from a registry — the Link pagination header is validated with Uri.IsWellFormedUriString and rejected before it is resolved. The redirect path was not following that convention, so a value like ht!tp://[bad (no legal scheme, therefore parsed as a relative reference) was resolved into a bogus URL on the registry host instead of being rejected.

The Location is now validated with IsWellFormedOriginalString() before resolution, so the three steps are: validate well-formedness → resolve relative references → enforce the HTTP(S) scheme.

Checked that this does not reject anything real — Azure SAS, S3 pre-signed (including +, /, = in signatures), and GCS URLs are all well-formed. Only genuinely broken values fail, such as unescaped brackets or spaces, or a UNC path.

The resolution itself is unchanged and remains identical to the existing upload session handling in PushAsync and MountAsync: location.IsAbsoluteUri ? location : new Uri(url, location).

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/OrasProject.Oras/Registry/Remote/BlobStore.cs:361

• Relative Location resolution should use the effective request URI (response.RequestMessage.RequestUri) as the base rather than the precomputed url. This matches RFC 9110 wording (“effective request URI”) and is consistent with HttpResponseMessageExtensions.ParseLink() which resolves relative header values against response.RequestMessage.RequestUri. It also avoids subtle mismatches if a handler normalizes or rewrites the request URI before sending.

                    var blobLocation = location.IsAbsoluteUri ? location : new Uri(url, location);