diff --git a/pkg/github/client.go b/pkg/github/client.go index e328f9e66..765dc43da 100644 --- a/pkg/github/client.go +++ b/pkg/github/client.go @@ -2028,19 +2028,24 @@ func (c *client) readPaginatedResultsWithValuesWithContext(ctx context.Context, // * next: /api/v3/repositories/22/pulls?per_page=100&page=2 // * prefix will be "/api/v3" and we strip it so we don't duplicate it // when prepending c.bases[hostIndex] - // Example for a redirect (e.g. repo rename): - // * initial call: api.github.com/repos/old-org/old-repo/pulls?per_page=100 - // * resp.Request.URL (after redirect): api.github.com/repos/new-org/new-repo/pulls?per_page=100 - // * next: api.github.com/repos/new-org/new-repo/pulls?per_page=100&page=2 - // * prefix will be empty; we compare only Path (not full RequestURI) so - // the differing response URL doesn't break the suffix match - pathOnly := strings.SplitN(pagedPath, "?", 2)[0] - prefix := strings.TrimSuffix(resp.Request.URL.Path, pathOnly) - + // Example for a redirect (e.g. repo rename), with or without GHE: + // * initial call: /repos/old-org/old-repo/pulls?per_page=100 + // * resp.Request.URL (after redirect): /repos/new-org/new-repo/pulls?per_page=100 + // * next: /repos/new-org/new-repo/pulls?per_page=100&page=2 + // * prefix is determined from c.bases by matching the Link host, so + // redirects that change the path don't affect prefix detection u, err := url.Parse(link) if err != nil { return fmt.Errorf("failed to parse 'next' link: %w", err) } + var prefix string + for _, base := range c.bases { + baseURL, err := url.Parse(base) + if err == nil && baseURL.Host == u.Host { + prefix = baseURL.Path + break + } + } pagedPath = strings.TrimPrefix(u.RequestURI(), prefix) if len(pagedPath) == 0 || pagedPath[0] != '/' { pagedPath = u.RequestURI() diff --git a/pkg/github/client_test.go b/pkg/github/client_test.go index 63dff37a7..68d557395 100644 --- a/pkg/github/client_test.go +++ b/pkg/github/client_test.go @@ -1492,6 +1492,69 @@ func TestReadPaginatedResultsWithRedirect(t *testing.T) { } } +func TestReadPaginatedResultsWithGHEAndRedirect(t *testing.T) { + requestCount := 0 + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + switch r.URL.Path { + case "/api/v3/repos/old-org/old-repo/branches": + newURL := fmt.Sprintf("https://%s/api/v3/repos/new-org/new-repo/branches", r.Host) + if r.URL.RawQuery != "" { + newURL += "?" + r.URL.RawQuery + } + http.Redirect(w, r, newURL, http.StatusMovedPermanently) + case "/api/v3/repos/new-org/new-repo/branches": + var labels []Label + if r.URL.Query().Get("page") == "" { + labels = []Label{{Name: "page1"}} + w.Header().Set("Link", fmt.Sprintf( + `; rel="next"`, + r.Host, r.URL.RawQuery)) + } else { + labels = []Label{{Name: "page2"}} + } + b, err := json.Marshal(labels) + if err != nil { + t.Errorf("Failed to marshal: %v", err) + } + fmt.Fprint(w, string(b)) + default: + t.Errorf("Unexpected request path: %s (full: %s)", r.URL.Path, r.URL.String()) + http.Error(w, "not found", http.StatusNotFound) + } + })) + defer ts.Close() + + c := getClient(ts.URL) + c.bases[0] = c.bases[0] + "/api/v3" + var labels []Label + err := c.readPaginatedResultsWithValues( + "/repos/old-org/old-repo/branches", + url.Values{ + "per_page": []string{"100"}, + "protected": []string{"false"}, + }, + "", + "", + func() interface{} { + return &[]Label{} + }, + func(obj interface{}) { + labels = append(labels, *(obj.(*[]Label))...) + }, + ) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expected := []Label{{Name: "page1"}, {Name: "page2"}} + if !reflect.DeepEqual(labels, expected) { + t.Errorf("Expected %v, got %v", expected, labels) + } + if requestCount != 3 { + t.Errorf("Expected 3 requests (redirect + page1 + page2), got %d", requestCount) + } +} + func TestListPullRequestComments(t *testing.T) { ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet {