From ef2838440043438f291c9920924b8aff57f2a9f1 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 6 Jun 2026 00:28:22 -0400 Subject: [PATCH 1/2] fix(wfctl): address download review feedback --- cmd/wfctl/download_progress.go | 5 +++-- cmd/wfctl/plugin_install.go | 5 ++++- cmd/wfctl/plugin_install_test.go | 7 ++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/wfctl/download_progress.go b/cmd/wfctl/download_progress.go index 0dd04b3f..266fa7a4 100644 --- a/cmd/wfctl/download_progress.go +++ b/cmd/wfctl/download_progress.go @@ -62,13 +62,14 @@ func (p *downloadProgress) emit(prefix string) { func formatDownloadBytes(n int64) string { const unit = 1024 + const prefixes = "KMGTPE" if n < unit { return fmt.Sprintf("%d B", n) } div, exp := int64(unit), 0 - for next := n / unit; next >= unit && exp < 4; next /= unit { + for next := n / unit; next >= unit && exp < len(prefixes)-1; next /= unit { div *= unit exp++ } - return fmt.Sprintf("%.1f %ciB", float64(n)/float64(div), "KMGTPE"[exp]) + return fmt.Sprintf("%.1f %ciB", float64(n)/float64(div), prefixes[exp]) } diff --git a/cmd/wfctl/plugin_install.go b/cmd/wfctl/plugin_install.go index d5d5f3a1..c461d89c 100644 --- a/cmd/wfctl/plugin_install.go +++ b/cmd/wfctl/plugin_install.go @@ -1122,9 +1122,9 @@ func downloadGitHubReleaseAsset(owner, repo, tag, filename, token string) ([]byt neturl.PathEscape(tag), ) metadataCtx, metadataCancel := context.WithTimeout(context.Background(), gitHubReleaseMetadataTimeout) - defer metadataCancel() req, err := http.NewRequestWithContext(metadataCtx, http.MethodGet, releaseURL, nil) if err != nil { + metadataCancel() return nil, err } req.Header.Set("Authorization", "Bearer "+token) @@ -1134,10 +1134,12 @@ func downloadGitHubReleaseAsset(owner, repo, tag, filename, token string) ([]byt resp, err := gitHubAPIClient.Do(req) if err != nil { + metadataCancel() return nil, fmt.Errorf("GitHub releases API: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + metadataCancel() return nil, fmt.Errorf("GitHub releases API: HTTP %d for %s/%s@%s", resp.StatusCode, owner, repo, tag) } @@ -1148,6 +1150,7 @@ func downloadGitHubReleaseAsset(owner, repo, tag, filename, token string) ([]byt } `json:"assets"` } if err := json.NewDecoder(resp.Body).Decode(&release); err != nil { + metadataCancel() return nil, fmt.Errorf("decode GitHub release response: %w", err) } metadataCancel() diff --git a/cmd/wfctl/plugin_install_test.go b/cmd/wfctl/plugin_install_test.go index 21e72af9..75e68bbb 100644 --- a/cmd/wfctl/plugin_install_test.go +++ b/cmd/wfctl/plugin_install_test.go @@ -134,7 +134,12 @@ func TestDownloadURL_DirectGetUsesBoundedRequestContext(t *testing.T) { } t.Cleanup(func() { http.DefaultClient = orig }) - got, err := downloadURL("https://example.com/plugin.tar.gz") + var got []byte + _, err := captureStderr(t, func() error { + var err error + got, err = downloadURL("https://example.com/plugin.tar.gz") + return err + }) if err != nil { t.Fatalf("downloadURL: %v", err) } From 8e6988a8080c1e0fac635e00f939c7af7fcdb8dc Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 6 Jun 2026 01:02:48 -0400 Subject: [PATCH 2/2] fix(wfctl): close download responses on error --- cmd/wfctl/plugin_install.go | 9 +++++++++ cmd/wfctl/plugin_install_test.go | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/cmd/wfctl/plugin_install.go b/cmd/wfctl/plugin_install.go index c461d89c..68135420 100644 --- a/cmd/wfctl/plugin_install.go +++ b/cmd/wfctl/plugin_install.go @@ -1134,6 +1134,7 @@ func downloadGitHubReleaseAsset(owner, repo, tag, filename, token string) ([]byt resp, err := gitHubAPIClient.Do(req) if err != nil { + closeResponseBody(resp) metadataCancel() return nil, fmt.Errorf("GitHub releases API: %w", err) } @@ -1186,6 +1187,7 @@ func downloadGitHubReleaseAsset(owner, repo, tag, filename, token string) ([]byt resp2, err := gitHubAPIClient.Do(req2) if err != nil { + closeResponseBody(resp2) return nil, fmt.Errorf("GitHub asset download API: %w", err) } defer resp2.Body.Close() @@ -1227,6 +1229,7 @@ func downloadURL(rawURL string) ([]byte, error) { } resp, err := http.DefaultClient.Do(req) if err != nil { + closeResponseBody(resp) return nil, err } defer resp.Body.Close() @@ -1236,6 +1239,12 @@ func downloadURL(rawURL string) ([]byte, error) { return readDownloadBodyWithProgress(resp.Body, resp.ContentLength) } +func closeResponseBody(resp *http.Response) { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } +} + // verifyChecksum checks that data matches the expected SHA256 hex string. func verifyChecksum(data []byte, expected string) error { h := sha256.Sum256(data) diff --git a/cmd/wfctl/plugin_install_test.go b/cmd/wfctl/plugin_install_test.go index 75e68bbb..4574119b 100644 --- a/cmd/wfctl/plugin_install_test.go +++ b/cmd/wfctl/plugin_install_test.go @@ -175,6 +175,14 @@ func TestDownloadURL_LargeDirectDownloadEmitsProgress(t *testing.T) { } } +func TestCloseResponseBodyClosesNonNilBody(t *testing.T) { + body := &trackingReadCloser{Reader: strings.NewReader("metadata")} + closeResponseBody(&http.Response{Body: body}) + if !body.closed { + t.Fatal("response body was not closed") + } +} + // TestDownloadURL_GitHubAuthHeader verifies that downloadURL injects a Bearer // Authorization header for non-release github.com URLs (direct-download path) // using the first non-empty token env var (RELEASES_TOKEN > GH_TOKEN > @@ -519,6 +527,16 @@ func TestDownloadURL_PrivateReleaseAssetUsesFreshAssetDownloadDeadline(t *testin } } +type trackingReadCloser struct { + *strings.Reader + closed bool +} + +func (r *trackingReadCloser) Close() error { + r.closed = true + return nil +} + // TestDownloadURL_PublicReleaseNoToken verifies that when no token is set, // downloadURL falls back to a plain GET for release download URLs (public repos). func TestDownloadURL_PublicReleaseNoToken(t *testing.T) {