Fix vip-next GitHub release pagination

internal/update/github.go

@@ -64,24 +64,26 @@ func (g GitHub) Releases(ctx context.Context) ([]Release, error) {
 		}
 		releases = append(releases, batch...)
 		next := 0
-		for _, part := range strings.Split(resp.Header.Get("Link"), ",") {
-			if !strings.Contains(part, `rel="next"`) {
-				continue
+		for _, value := range resp.Header.Values("Link") {
+			for _, part := range strings.Split(value, ",") {
+				if !strings.Contains(part, `rel="next"`) {
+					continue
+				}
+				left, right := strings.Index(part, "<"), strings.Index(part, ">")
+				if left < 0 || right <= left {
+					return nil, fmt.Errorf("invalid release pagination")
+				}
+				u, err := url.Parse(part[left+1 : right])
+				if err != nil {
+					return nil, fmt.Errorf("invalid release pagination")
+				}
+				u = origin.ResolveReference(u)
+				n, err := strconv.Atoi(u.Query().Get("page"))
+				if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || !isReleasePaginationPath(u.EscapedPath()) || u.User != nil || next != 0 {
+					return nil, fmt.Errorf("invalid release pagination")
+				}
+				next = n
 			}
-			left, right := strings.Index(part, "<"), strings.Index(part, ">")
-			if left < 0 || right <= left {
-				return nil, fmt.Errorf("invalid release pagination")
-			}
-			u, err := url.Parse(part[left+1 : right])
-			if err != nil {
-				return nil, fmt.Errorf("invalid release pagination")
-			}
-			u = origin.ResolveReference(u)
-			n, err := strconv.Atoi(u.Query().Get("page"))
-			if err != nil || n <= page || u.Scheme != origin.Scheme || u.Host != origin.Host || u.Path != "/repos/"+Repository+"/releases" || u.User != nil || next != 0 {
-				return nil, fmt.Errorf("invalid release pagination")
-			}
-			next = n
 		}
 		if next == 0 {
 			return releases, nil
@@ -90,6 +92,23 @@ func (g GitHub) Releases(ctx context.Context) ([]Release, error) {
 	}
 	return nil, fmt.Errorf("GitHub release pagination limit exceeded")
 }
+
+func isReleasePaginationPath(path string) bool {
+	if path == "/repos/"+Repository+"/releases" {
+		return true
+	}
+	repositoryID, ok := strings.CutPrefix(path, "/repositories/")
+	if !ok {
+		return false
+	}
+	repositoryID, ok = strings.CutSuffix(repositoryID, "/releases")
+	if !ok {
+		return false
+	}
+	id, err := strconv.ParseUint(repositoryID, 10, 64)
+	return err == nil && id > 0
+}
+
 func readBounded(r io.Reader, max int64) ([]byte, error) {
 	b, err := io.ReadAll(io.LimitReader(r, max+1))
 	if err != nil {

internal/update/release_test.go

@@ -87,33 +87,73 @@ func TestSelectRejectsIncompleteAndInconsistentReleases(t *testing.T) {
 	}
 }
 func TestGitHubPagination(t *testing.T) {
+	for _, tc := range []struct {
+		name, nextPath string
+	}{{"named repository", "/repos/Automattic/vip-cli/releases"}, {"numeric repository ID", "/repositories/116313791/releases"}} {
+		t.Run(tc.name, func(t *testing.T) {
+			calls := 0
+			s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				calls++
+				if r.Header.Get("Authorization") != "" {
+					t.Error("credentials")
+				}
+				if r.URL.Path != "/repos/Automattic/vip-cli/releases" {
+					t.Error(r.URL.Path)
+				}
+				if r.URL.Query().Get("page") == "1" {
+					w.Header().Set("Link", "<http://"+r.Host+tc.nextPath+"?per_page=100&page=2>; rel=\"next\"")
+					fmt.Fprint(w, `[{"tag_name":"4.9.0"}]`)
+				} else {
+					fmt.Fprint(w, `[{"tag_name":"5.0.0"}]`)
+				}
+			}))
+			defer s.Close()
+			rs, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background())
+			if err != nil || len(rs) != 2 || calls != 2 {
+				t.Fatal(rs, err, calls)
+			}
+		})
+	}
+}
+
+func TestGitHubRejectsDuplicateNextLinkHeaders(t *testing.T) {
 	calls := 0
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		calls++
-		if r.Header.Get("Authorization") != "" {
-			t.Error("credentials")
-		}
-		if r.URL.Path != "/repos/Automattic/vip-cli/releases" {
-			t.Error(r.URL.Path)
+		if r.URL.Query().Get("page") == "1" {
+			w.Header().Add("Link", "<http://"+r.Host+"/repositories/116313791/releases?page=2>; rel=\"next\"")
+			w.Header().Add("Link", "<http://"+r.Host+"/repositories/116313791/releases?page=3>; rel=\"next\"")
 		}
+		fmt.Fprint(w, `[]`)
+	}))
+	defer s.Close()
+	_, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background())
+	if err == nil || calls != 1 {
+		t.Fatal(err, calls)
+	}
+}
+
+func TestGitHubRejectsEncodedReleasePaginationSeparator(t *testing.T) {
+	calls := 0
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
 		if r.URL.Query().Get("page") == "1" {
-			w.Header().Set("Link", "<http://"+r.Host+"/repos/Automattic/vip-cli/releases?per_page=100&page=2>; rel=\"next\"")
-			fmt.Fprint(w, `[{"tag_name":"4.9.0"}]`)
-		} else {
-			fmt.Fprint(w, `[{"tag_name":"5.0.0"}]`)
+			w.Header().Set("Link", "<http://"+r.Host+"/repositories/116313791%2Freleases?page=2>; rel=\"next\"")
 		}
+		fmt.Fprint(w, `[]`)
 	}))
 	defer s.Close()
-	rs, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background())
-	if err != nil || len(rs) != 2 || calls != 2 {
-		t.Fatal(rs, err, calls)
+	_, err := (GitHub{Client: s.Client(), BaseURL: s.URL}).Releases(context.Background())
+	if err == nil || calls != 1 {
+		t.Fatal(err, calls)
 	}
 }
+
 func TestGitHubErrors(t *testing.T) {
 	for _, tc := range []struct {
 		status     int
 		body, link string
-	}{{429, `{}`, ""}, {200, `broken`, ""}, {200, `[]`, `<https://evil.invalid/next>; rel="next"`}, {200, `[]`, `</repos/Automattic/vip-cli/releases?page=1>; rel="next"`}} {
+	}{{429, `{}`, ""}, {200, `broken`, ""}, {200, `[]`, `<https://evil.invalid/next>; rel="next"`}, {200, `[]`, `</repositories/not-a-number/releases?page=2>; rel="next"`}, {200, `[]`, `</repos/Automattic/vip-cli/releases?page=1>; rel="next"`}} {
 		s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			w.Header().Set("Link", tc.link)
 			w.WriteHeader(tc.status)