Skip to content

fix: Resolve relative redirect locations - #4605

Open
DorianChn wants to merge 1 commit into
google:masterfrom
DorianChn:shanchuanzhi/fix-relative-redirect-resolution
Open

DorianChn wants to merge 1 commit into
google:masterfrom
DorianChn:shanchuanzhi/fix-relative-redirect-resolution

Conversation

@DorianChn

Copy link
Copy Markdown

Summary

  • Resolve relative redirect locations against the request URL that received the response, not the API base URL.
  • Apply the behavior to 301 redirects and artifact, workflow-log, and archive download links.
  • Preserve redirect validation and add regression coverage for rate-limited and non-rate-limited artifact downloads.

Why

Relative Location values are resolved against the responding request URL; using the API base URL can discard the endpoint path or return an unusable relative URL. This follows http.Response.Location.

Validation

  • bash script/test.sh ./... — all 11 modules passed.
  • go vet ./... — passed.
  • Root custom golangci-lint — 0 issues; gofmt and git diff --check passed.
  • script/lint.sh reached the generated-file check, which reports the local go.sum CRLF/LF mismatch; no unrelated line-ending change is included.

AI assistance

AI assistance helped draft the implementation and regression tests; the contributor reviewed the diff and ran the listed validations.

@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.60%. Comparing base (4a73e54) to head (9210066).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
github/github.go 92.30% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4605   +/-   ##
=======================================
  Coverage   98.60%   98.60%           
=======================================
  Files         198      198           
  Lines       18552    18555    +3     
=======================================
+ Hits        18294    18297    +3     
  Misses        258      258           

☔ 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.

@gmlewis

gmlewis commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Is this PR fixing a hypothetical issue that has never been emperically found in the wild, or has this problem ever been actually seen when using this client library?

When would a relative URL ever be returned by GitHub?

How has this client library been working for decades without ever encountering a relative URL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants