fix(client): skip a Link entry whose first parameter has no value

What does this MR do?

Response.populateLinkValues takes the relation type of each Link header entry from the first parameter after the URL, as strings.Split(parts[1], "=")[1] (gitlab.go line 1217). RFC 8288 lets a link parameter have no value, and then that split has no second element, so <url>; anchor; rel="next" and <url>; rel make newResponse panic with index out of range [1] with length 1. Client.Do calls newResponse (line 1284) before it looks at the status code, so the panic escapes from whichever API call received the response.

This MR skips such an entry instead of indexing past the end. Nothing else changes: the relation type still comes from the first parameter, so every Link header that parses today parses the same way. The change is the length check in populateLinkValues, plus a table-driven test of that function and a test that lists a page through ListProjects, both in gitlab_test.go.

GitLab's own Link headers carry rel as their only parameter (offset_header_builder.rb, keyset/header_builder.rb), so this needs something between the client and GitLab that adds a parameter, such as a reverse proxy or a CDN. A library should still not panic on a valid header.

An earlier version of this MR also read rel from any parameter, as RFC 8288 does. As agreed in the review thread, that part is left out: the relation type keeps coming from the first parameter, and a change like that one can be looked at again if users report proxies that add parameters.

It comes from maintaining gitlab-mcp-server, an MCP server built on this library that reads Response.NextLink to continue keyset lists.

How this relates to other open MRs

!3065 and !3147 (both mine) also change gitlab.go and gitlab_test.go, in other functions. git merge-tree finds no conflict between this branch and either of them or the current main, so they can land in any order.

Is this a breaking change?

No. No exported name, type or signature changes, and every Link header that parses today parses the same way. Only the headers that used to panic behave differently: the entry with the parameter without a value is skipped, and the other entries are read as before.

How was this tested?

  • TestPaginationPopulateLinkValues builds a Response with newResponse for eleven Link headers and checks PreviousLink, NextLink, FirstLink and LastLink. Eight of them parse today and pin how: the offset and keyset links in the format GitLab sends, REL=next, two rel parameters, and four headers that RFC 8288 reads differently and that keep today's reading (another parameter before rel, title="next" alone, whitespace around =, whitespace after the value). The other three carry a parameter without a value: before rel, as rel itself, and in the first of two links. On main without the fix the eight pass and the three panic; with the fix all eleven pass, and turning the new continue into a break fails the two-link case.
  • TestClientDo_LinkParameterWithoutValue lists a keyset page through Projects.ListProjects against the test server, with each of the two forms in its Link header. On main without the fix both panic from Client.Do; with the fix both return the page, with NextLink empty.
  • go test work -race (the make test target) and go vet work pass on Go 1.26.10 and 1.27.3. golangci-lint over both workspace modules, as make lint runs it, reports 0 issues with 2.14.0 (this branch's .tool-versions) and 2.15.0 (main's); gofumpt -l (0.8.0) lists nothing; buf format and buf lint are clean (no .proto file changes). On this branch merged with the current main, go test work -race on Go 1.27.3 and golangci-lint 2.15.0 pass as well.
Test output

Without the fix, with gitlab.go at 4075b0f3 (one of the five panics, paths shortened):

=== NAME  TestClientDo_LinkParameterWithoutValue/rel_without_a_value
        Error:  func (assert.PanicTestFunc)(0x112fb40) should not panic
                Panic value:  runtime error: index out of range [1] with length 1
                ...
                gitlab.com/gitlab-org/api/client-go/v3.(*Response).populateLinkValues-range1(...)
                    gitlab.go:1214 +0x335
                gitlab.com/gitlab-org/api/client-go/v3.newResponse(0xc00034e5a0)
                    gitlab.go:1163 +0xd0
                gitlab.com/gitlab-org/api/client-go/v3.(*Client).Do(...)
                    gitlab.go:1281 +0xc05
                gitlab.com/gitlab-org/api/client-go/v3.(*ProjectsService).ListProjects(...)
                    projects.go:770 +0x24b
--- FAIL: TestPaginationPopulateLinkValues (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/whitespace_around_the_equals_sign (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/whitespace_after_the_value (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/unquoted_value_and_a_name_in_another_case (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/two_rel_parameters (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/another_parameter_before_rel (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/another_parameter_whose_value_is_a_relation_type (0.00s)
    --- FAIL: TestPaginationPopulateLinkValues/rel_without_a_value (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/offset_links_in_the_format_GitLab_sends (0.00s)
    --- FAIL: TestPaginationPopulateLinkValues/parameter_without_a_value_in_one_of_two_links (0.00s)
    --- PASS: TestPaginationPopulateLinkValues/keyset_link_in_the_format_GitLab_sends (0.00s)
    --- FAIL: TestPaginationPopulateLinkValues/parameter_without_a_value_before_rel (0.00s)
--- FAIL: TestClientDo_LinkParameterWithoutValue (0.00s)
    --- FAIL: TestClientDo_LinkParameterWithoutValue/rel_without_a_value (0.00s)
    --- FAIL: TestClientDo_LinkParameterWithoutValue/parameter_without_a_value_before_rel (0.00s)

With the fix:

--- PASS: TestPaginationPopulateLinkValues (0.00s)
--- PASS: TestClientDo_LinkParameterWithoutValue (0.00s)
ok  	gitlab.com/gitlab-org/api/client-go/v3	1.032s

gitlab-mcp-server keeps a record of what it finds and contributes in the projects it depends on, and in sibling projects, in upstream-bugs.md.

Edited by José M. Requena Plens

Merge request reports

Loading
Loading