diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 010e374..6bff815 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -122,8 +122,19 @@ func fetchBounded(url string, limit int64) ([]byte, error) { return body, nil } +// trimVersionPrefix removes the "v" the toolchain records with, so a +// recorded version ("v1.2.3", or a pseudo-version "v0.0.0-…") compares +// against release tags, which CheckLatest already reports without it. +func trimVersionPrefix(v string) string { + return strings.TrimPrefix(v, "v") +} + // CompareVersions compares two dotted versions numerically per part. +// Either side may carry the leading "v" of a recorded version or of a +// tag; the shapes compare equal, which is what keeps an installation +// running v1.0.0 from being offered an update to 1.0.0. func CompareVersions(a, b string) int { + a, b = trimVersionPrefix(a), trimVersionPrefix(b) as, bs := strings.Split(a, "."), strings.Split(b, ".") for i := 0; i < len(as) || i < len(bs); i++ { av, bv := part(as, i), part(bs, i) diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index 40a8890..732914a 100644 --- a/internal/updater/updater_test.go +++ b/internal/updater/updater_test.go @@ -30,6 +30,13 @@ func TestCompareVersions(t *testing.T) { {"0.6", "0.6.0", 0}, {"0.10.0", "0.9.0", 1}, {"1.2.3-rc1", "1.2.3", 0}, // suffix ignored per part + {"v1.0.0", "1.0.0", 0}, // the recorded version carries the v a tag does + {"1.0.0", "v1.0.0", 0}, + {"v1.1.0", "v1.0.0", 1}, + {"1.28.0", "1.9.0", 1}, // parts compare numerically, not by character + {"1.9.0", "1.28.0", -1}, + {"1.28.0", "1.128.0", -1}, + {"v1.28.0", "1.28.0", 0}, } for _, tc := range cases { if got := CompareVersions(tc.a, tc.b); got != tc.want { @@ -60,6 +67,39 @@ func TestCheckLatestParsesTag(t *testing.T) { if got := UpdateAvailable("2.0.0"); got != "" { t.Fatalf("UpdateAvailable(newer local) = %q", got) } + // The recorded version carries the "v" the release tag does: an + // installation running v1.2.3 must not be offered 1.2.3. + if got := UpdateAvailable("v1.2.3"); got != "" { + t.Fatalf("UpdateAvailable(v-prefixed current) = %q", got) + } + if got := UpdateAvailable("v1.2.2"); got != "1.2.3" { + t.Fatalf("UpdateAvailable(v-prefixed older) = %q", got) + } +} + +// The production bug: an installation running v1.0.0 was offered an +// update to 1.0.0, because the recorded version and the tag compared +// unequal through the prefix. SelfUpdate must refuse in place, and it +// must not download anything on the way. +func TestSelfUpdateRefusesTheSameVersionThroughThePrefix(t *testing.T) { + var downloads int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/releases/download/") { + downloads++ + } + _ = json.NewEncoder(w).Encode(map[string]any{"tag_name": "v9.9.9"}) + })) + defer srv.Close() + old := ReleaseBase + ReleaseBase = srv.URL + defer func() { ReleaseBase = old }() + + if _, err := SelfUpdate("v9.9.9"); err == nil || !strings.Contains(err.Error(), "already running") { + t.Fatalf("SelfUpdate(v-prefixed current) = %v, want the latest-release refusal", err) + } + if downloads != 0 { + t.Fatalf("%d download requests fired for a same-version update", downloads) + } } func TestCheckLatestErrors(t *testing.T) {