fix(api): refuse redirects that turn a write into a GET
Description
When a proxy in front of GitLab answers an http request with a redirect to https, glab release create fails with cannot unmarshal array into Go value of type gitlab.Release, which is the error in #8095 (closed). net/http follows a 301, 302 or 303 answer to a POST, PUT, PATCH or DELETE by sending a GET to the new location without the body, so the create reaches the list releases endpoint instead. An update is worse: the GET reaches the release itself, decodes into the expected type, and glab prints Release updated although nothing was changed.
This MR gives the HTTP client built in internal/api/client.go a CheckRedirect policy, in the new internal/api/redirect.go:
- A redirect that keeps the method is followed as before: every redirect of a GET or HEAD, and a 307 or 308 of a write, which resends the body.
- A redirect that would change the method is refused, with an error that names the status and the URL that redirected.
- When that redirect moves the same host from
httptohttps, on the port the host reaches overhttps(443 when it names none), the error also says how to setapi_protocoltohttps:glab config set api_protocol https --host <host>, with the port when the host has one, orGLAB_API_PROTOCOL=https. In a CI job like the one in #8095 (closed) that is theGLAB_API_PROTOCOL: httpsvariable. With CI auto-login on (GLAB_ENABLE_CI_AUTOLOGIN=true), glab reads the protocol fromCI_SERVER_PROTOCOLalone and neither of those helps, so the error says so and names what does: anexternal_urlwithhttpson the instance, as the triage note says, orCI_SERVER_PROTOCOLset tohttpsin the job. - The limit of 10 redirects is kept, because setting
CheckRedirectreplaces the default policy that enforced it.
Everything that shares this client gets the policy, for example: client-go, the OAuth2 login flows and token refresh, and glab api. GitLab's own redirects are not affected: it answers a write to a moved project with 405 rather than a redirect (lib/api/helpers.rb in gitlab-org/gitlab), and when it forwards a package registry POST it uses 307 (lib/api/helpers/packages/dependency_proxy_helpers.rb), which keeps the method.
Before and after, with the built binary
I built glab from main (8ae0c0ef) and from this branch, and ran glab release create v1.0.0 -R <host>/group/project --notes "new notes" --name v1.0.0 with GLAB_API_PROTOCOL=http against a stand-in GitLab on https, behind a stand-in proxy on the same address that answers every http request with a 301 to the same URL on https.
The release does not exist yet, so glab creates it.
main, exit 1. After the proxy's 301, glab sent GET .../releases in place of the POST:
ERROR
Json: cannot unmarshal array into Go value of type gitlab.Release.This branch, exit 1, and the POST never reaches GitLab (the error as glab prints it, unwrapped):
Post "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases, which would resend this POST request as a GET without its body; the server redirects http to https, so set api_protocol to https with `glab config set api_protocol https --host 127.0.0.1:40783` or GLAB_API_PROTOCOL=https.The release exists, so glab updates it.
main, exit 0, and GitLab received GET .../releases/v1%2E0%2E0 in place of the PUT:
✓ Release updated url=https://example.invalid/r
✓ Release succeeded after 0.00 seconds.This branch, exit 1:
Put "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases/v1%2E0%2E0": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases/v1%2E0%2E0, which would resend this PUT request as a GET without its body; the server redirects http to https, so set api_protocol to https with `glab config set api_protocol https --host 127.0.0.1:40783` or GLAB_API_PROTOCOL=https.Under CI auto-login (GLAB_ENABLE_CI_AUTOLOGIN=true, GITLAB_CI=true and CI_SERVER_PROTOCOL=http), this branch, exit 1, for the create:
Post "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases, which would resend this POST request as a GET without its body; the server redirects http to https, and CI auto-login reads api_protocol from CI_SERVER_PROTOCOL, so give the instance an https external_url, or set CI_SERVER_PROTOCOL to https in the job.In separate runs, these created the release: the glab config set command from the error, with api_protocol set to http for the host before it; GLAB_API_PROTOCOL=https with that same config; and under CI auto-login, CI_SERVER_PROTOCOL=https. Under CI auto-login, GLAB_API_PROTOCOL=https with api_protocol set to https for the host still got the refusal above.
Related Issues
Resolves #8095 (closed)
The same failure was reported for glab mr create in #7728 (closed), closed for inactivity, and in gitlab-org/api/client-go#2094. glab mr create goes through the same client, so it gets the refusal too; client-go itself does not change here.
How has this been tested?
internal/api/redirect_test.go drives the client NewClient builds against a real httptest server:
- A create answered with 301 and an update answered with 302 are refused, and nothing reaches the redirect target. Both fail on
main: the create with the error from #8095 (closed), the update with no error at all. - A GET answered with 301 is followed, and a create answered with 308 is followed with its body intact.
- A redirect loop stops after 10 redirects.
- The
api_protocolhint appears forhttptohttpson the same host and port, and not for another host, another port or a redirect that keeps the scheme; for a host without a port it appears whether the redirect leaves the port out or writes:443, the form Omnibus GitLab sends. It names theglab config setcommand for the host andGLAB_API_PROTOCOL, and under CI auto-loginCI_SERVER_PROTOCOLand the instance'sexternal_urlinstead; the two hint texts and the other-port case failed before the second commit. This test fails onmaintoo.
Checks I ran with Go 1.27.3 on the second commit:
make test: 5520 tests, all passing apart from the 17 the suite skipsgo test -raceoninternal/api/...andinternal/config/...golangci-lint runwith 2.14.0: no findings in the changed filesgo run ./scripts/comment-overlap -threshold 1.0 -max-findings 0 ./cmd/... ./internal/...: 0 findingsmake build,make check-embedandmake check-args- the login line GitLab's release job runs (
glab auth login --job-token ... --api-protocol $CI_SERVER_PROTOCOL) withhttp, against the stand-in above:glab api versionthen usedhttp, withGLAB_API_PROTOCOL=httpsit usedhttps, and withGLAB_API_PROTOCOL=httpsunder CI auto-login it still usedhttp - commitlint with
scripts/commit-lint's rules on both commit messages and on this title