From b0c0bf7da63b74bfde7108ff449f39fb82ef4f42 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Fri, 21 Nov 2025 14:07:34 +0100 Subject: [PATCH] fix: decode URL-encoded characters in asset filenames Fixes #281 GitHub encodes special characters in asset URLs (e.g., `+` becomes `%2B`). When processing charts with SemVer build metadata like `1.0.0+build.1`, chart-releaser would fail because it tried to access files using the URL-encoded filename instead of the actual filename on disk. This change adds URL decoding in two critical locations: - UpdateIndexFile(): when extracting filenames from asset URLs - addToIndexFile(): when constructing local file paths Both the filename used for filesystem operations and the name stored in the index file are now properly decoded, ensuring consistency. Added comprehensive test coverage including a test chart package with build metadata to verify the fix works end-to-end. Signed-off-by: Matthias Bertschy --- pkg/releaser/releaser.go | 21 ++++-- pkg/releaser/releaser_test.go | 68 +++++++++++++++++- .../test-chart-0.1.0+build.1.tgz | Bin 0 -> 745 bytes 3 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 pkg/releaser/testdata/release-packages-plus/test-chart-0.1.0+build.1.tgz diff --git a/pkg/releaser/releaser.go b/pkg/releaser/releaser.go index 03e28ae2..85df9a96 100644 --- a/pkg/releaser/releaser.go +++ b/pkg/releaser/releaser.go @@ -164,6 +164,11 @@ func (r *Releaser) UpdateIndexFile() (bool, error) { for _, asset := range release.Assets { downloadURL, _ := url.Parse(asset.URL) name := filepath.Base(downloadURL.Path) + // Decode URL-encoded characters in the filename (e.g., %2B -> +) + name, err = url.PathUnescape(name) + if err != nil { + return false, errors.Wrapf(err, "error decoding filename from URL: %s", asset.URL) + } // Ignore any other files added in the release by the users. if filepath.Ext(name) != chartAssetFileExtension { continue @@ -255,8 +260,14 @@ func (r *Releaser) splitPackageNameAndVersion(pkg string) []string { return []string{pkg[0:delimIndex], pkg[delimIndex+1:]} } -func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, url string) error { - arch := filepath.Join(r.config.PackagePath, filepath.Base(url)) +func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, urlStr string) error { + // Decode URL-encoded characters in the filename (e.g., %2B -> +) + filename := filepath.Base(urlStr) + decodedFilename, err := url.PathUnescape(filename) + if err != nil { + return errors.Wrapf(err, "error decoding filename from URL: %s", urlStr) + } + arch := filepath.Join(r.config.PackagePath, decodedFilename) // extract chart metadata fmt.Printf("Extracting chart metadata from %s\n", arch) @@ -274,7 +285,7 @@ func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, url string) error { // remove url name from url as helm's index library // adds it in during .Add // there should be a better way to handle this :( - s := strings.Split(url, "/") + s := strings.Split(urlStr, "/") s = s[:len(s)-1] if r.config.PackagesWithIndex { @@ -283,8 +294,8 @@ func (r *Releaser) addToIndexFile(indexFile *repo.IndexFile, url string) error { s = s[:0] } - // Add to index - return indexFile.MustAdd(c.Metadata, filepath.Base(arch), strings.Join(s, "/"), hash) + // Add to index (using the decoded filename) + return indexFile.MustAdd(c.Metadata, decodedFilename, strings.Join(s, "/"), hash) } // CreateReleases finds and uploads Helm chart packages to GitHub diff --git a/pkg/releaser/releaser_test.go b/pkg/releaser/releaser_test.go index dde79340..6f87c781 100644 --- a/pkg/releaser/releaser_test.go +++ b/pkg/releaser/releaser_test.go @@ -116,9 +116,33 @@ func (f *FakeGitHub) CreatePullRequest(owner string, repo string, message string } func TestReleaser_UpdateIndexFile(t *testing.T) { - indexDir, _ := os.MkdirTemp(".", "index") + indexDir, err := os.MkdirTemp(".", "index") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } defer os.RemoveAll(indexDir) + // Create a separate index directory for the URL encoding test + indexDirWithPlus, err := os.MkdirTemp(".", "index-plus") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(indexDirWithPlus) + + // FakeGitHub that returns URL-encoded asset URLs (e.g., + encoded as %2B) + fakeGitHubURLEncoded := &FakeGitHubWithURLEncoding{ + release: &github.Release{ + Name: "test-chart-0.1.0+build.1", + Description: "A Helm chart with build metadata", + Assets: []*github.Asset{ + { + Path: "testdata/release-packages-plus/test-chart-0.1.0+build.1.tgz", + URL: "https://myrepo/charts/test-chart-0.1.0%2Bbuild.1.tgz", + }, + }, + }, + } + fakeGitHub := new(FakeGitHub) tests := []struct { @@ -177,6 +201,18 @@ func TestReleaser_UpdateIndexFile(t *testing.T) { }, indexFile: "", }, + { + name: "index-file-with-url-encoded-plus-sign", + exists: false, + releaser: &Releaser{ + config: &config.Options{ + IndexPath: filepath.Join(indexDirWithPlus, "index.yaml"), + PackagePath: "testdata/release-packages-plus", + }, + github: fakeGitHubURLEncoded, + }, + indexFile: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -198,6 +234,20 @@ func TestReleaser_UpdateIndexFile(t *testing.T) { } else { _, err := os.Stat(tt.releaser.config.IndexPath) assert.NoError(t, err) + + // Additional verification for URL-encoded case + if tt.name == "index-file-with-url-encoded-plus-sign" { + indexFile, err := repo.LoadIndexFile(tt.releaser.config.IndexPath) + assert.NoError(t, err, "should load index file") + assert.True(t, indexFile.Has("test-chart", "0.1.0+build.1"), + "index should contain chart version with + sign (decoded from %2B)") + + // Verify the chart entry has the correct version with build metadata + chartVersion, err := indexFile.Get("test-chart", "0.1.0+build.1") + assert.NoError(t, err, "should retrieve chart version") + assert.NotNil(t, chartVersion, "chart version should not be nil") + assert.Equal(t, "0.1.0+build.1", chartVersion.Version, "version should preserve + sign") + } } }) } @@ -535,3 +585,19 @@ func TestReleaser_ReleaseNotes(t *testing.T) { }) } } + +type FakeGitHubWithURLEncoding struct { + release *github.Release +} + +func (f *FakeGitHubWithURLEncoding) CreateRelease(ctx context.Context, input *github.Release) error { + return nil +} + +func (f *FakeGitHubWithURLEncoding) GetRelease(ctx context.Context, tag string) (*github.Release, error) { + return f.release, nil +} + +func (f *FakeGitHubWithURLEncoding) CreatePullRequest(owner string, repo string, message string, head string, base string) (string, error) { + return "https://github.com/owner/repo/pull/42", nil +} diff --git a/pkg/releaser/testdata/release-packages-plus/test-chart-0.1.0+build.1.tgz b/pkg/releaser/testdata/release-packages-plus/test-chart-0.1.0+build.1.tgz new file mode 100644 index 0000000000000000000000000000000000000000..db1466b1b1969ba7286b1771fba310634fdb773c GIT binary patch literal 745 zcmVQ54fLH~1! zLqbA=AR@!@*a^reCV>Oti&@m87Lrx~PCms-ie$FfAh+%PKZ;z%|6D))1SCU8SsT9(~ckNAP-rYMq%mC{VThfm0JF!=)6 zyn-*aa!Y|Ki`;_dIDu6@xi?Eifv+^3Ol!tdpSZ8RV@sdj`InfNoT`a^cY>YjzdHEp z{M-6}ARdkX^$dBbPOmS^ee{AY@gD}G$RVTgI3ABl91k5eGYoD0KL~esr^j%^@(Bo3 zC7p4UixSUr?UrPw3i|Qs{k^ND+>f^V^V6yXoZsxA_UMrO$J%R0;0EqH^KGE^4nabx0n zg(`u8c57rc=L`5%Q_eDmDX9KtjQ8Ci*l0KMQm>(H6)?qw)&fDym07Fy)@)hn;Hdyw zA#j#6sWi&D@&Xg^cDgX`_;7KlWm)kPM2X2e-~F6Hg&e79{g%uNNm(JmFuV=g{TL&Z$e8jRMc|Iu|QVZ_ExRXU`98&(6*njTZk~ z_|^`kYZ*EZW?u~;k? bi^XEGSS%Kc#bU8oEL->sNB4jw04M+e0cCd1 literal 0 HcmV?d00001