fix: Honor HTTP client timeouts in redirects

[sb123sb123]

Fixes #4584

Cause

roundTripWithOptionalFollowRedirect called the configured transport's RoundTrip method directly. That preserves access to 301/302 responses, but bypasses http.Client.Do, so the configured http.Client.Timeout was never applied to these redirect-aware requests.

Fix

Use the existing no-redirect http.Client for the request. It shares the configured transport and timeout and returns the first redirect response because its CheckRedirect returns http.ErrUseLastResponse; the existing redirect and host checks remain unchanged.

Tests

• go test -modfile=go.test.mod ./github -run 'Test(RoundTripWithOptionalFollowRedirect|ActionsService_DownloadArtifact|ActionsService_GetWorkflowJobLogs|ActionsService_GetWorkflowRun.*Logs|RepositoriesService_GetArchiveLink|RepositoriesService_GetBranch)' -count=1
• go test -modfile=go.test.mod ./... -count=1
• go test -v -tags=integration -run='^$' -modfile=go.test.mod ./test/integration
• go vet -modfile=go.test.mod ./...
• gofmt -d github/github.go github/github_timeout_test.go
• git diff --check

The temporary go.test.mod used an uncommitted local replacement for github.com/google/go-querystring because this Windows host could not reach proxy.golang.org; it was removed after each run.

Limitations

The required repository-wide race run was attempted, but this host has CGO_ENABLED=0 and no available C compiler. The all-module non-race script, custom linter bootstrap, generated-file check, schema check, and OpenAPI validation were also attempted; external module/TLS downloads and the custom-linter installer were unavailable on this host. The logs are retained under /g/OSS-PR-200/work/evidence.

AI assistance was used to investigate, reproduce, implement, test, and prepare this PR. I reviewed the complete diff and the reported test results; the change is intentionally limited to the timeout behavior and its regression test.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.59%. Comparing base (9dc5ad3) to head (5985a00).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4585   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files         197      197           
  Lines       18326    18323    -3     
=======================================
- Hits        18068    18066    -2     
+ Misses        258      257    -1     

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

Thanks for the diagnosis - the missing timeout needs to be addressed, but not in the manner that you are proposing, because this implementation causes 3 more issues:

• Transport errors are now wrapped in *url.Error, so err == mySentinel, error types, and error strings all change.
• Do attaches the client's http.CookieJar: these requests now send jar cookies and write response Set-Cookie values into the shared jar.
• Do parses Location before our checkRedirectHost gets to run, so an unparseable Location now returns (nil, err) instead of the redirect response and callers lose the *Response.

I believe the safer approach is to revert the changes you made and add the following BEFORE line 2381:

if t := c.client.Timeout; t > 0 {
    var cancel context.CancelFunc
    ctx, cancel = context.WithTimeout(ctx, t)
    defer cancel()
}

Also, please move your new test to github/github_test.go instead of making a new file.

And finally, please do not use time.Sleep in any unit tests - look at the other examples and take advantage of synctest and other testutils to make the unit test independent of time, for when these tests are run on extremely slow runners.

Once you fix the CLA issues, we can move forward with this PR.