From 8fe3a9684c4a9eb383f1d714d8a5e07d81707227 Mon Sep 17 00:00:00 2001
From: Rafa Castelblanque <rcastelblanq@vmware.com>
Date: Tue, 30 Nov 2021 15:05:27 +0100
Subject: [PATCH] Allow to sync and store charts with spaces in name (#3758)

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
---
 .../server/testdata/helm-index-spaces.yaml    |  7 ++
 cmd/asset-syncer/server/utils.go              | 23 ++++-
 cmd/asset-syncer/server/utils_test.go         | 95 +++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 cmd/asset-syncer/server/testdata/helm-index-spaces.yaml

diff --git a/cmd/asset-syncer/server/testdata/helm-index-spaces.yaml b/cmd/asset-syncer/server/testdata/helm-index-spaces.yaml
new file mode 100644
index 00000000000..1a63cd1139b
--- /dev/null
+++ b/cmd/asset-syncer/server/testdata/helm-index-spaces.yaml
@@ -0,0 +1,7 @@
+entries:
+  chart-with-spaces:
+    - appVersion: v1
+      name: "chart\x20with\x20spaces"
+  chart-without-spaces:
+    - appVersion: v2
+      name: "chart-without-spaces"
diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go
index 97c84b83d79..8604e14920c 100644
--- a/cmd/asset-syncer/server/utils.go
+++ b/cmd/asset-syncer/server/utils.go
@@ -198,6 +198,27 @@ func satisfy(chartInput map[string]interface{}, code *gojq.Code, vars []interfac
 	return satisfied, nil
 }
 
+// Make sure charts are treated without escaped data
+func unescapeChartsData(charts []models.Chart) []models.Chart {
+	result := []models.Chart{}
+	for _, chart := range charts {
+		chart.Name = unescapeOrDefaultValue(chart.Name)
+		chart.ID = unescapeOrDefaultValue(chart.ID)
+		result = append(result, chart)
+	}
+	return result
+}
+
+// Unescape string or return value itself if error
+func unescapeOrDefaultValue(value string) string {
+	unescapedValue, err := url.PathUnescape(value)
+	if err != nil {
+		return value
+	} else {
+		return unescapedValue
+	}
+}
+
 func filterCharts(charts []models.Chart, filterRule *apprepov1alpha1.FilterRuleSpec) ([]models.Chart, error) {
 	if filterRule == nil || filterRule.JQ == "" {
 		// No filter
@@ -248,7 +269,7 @@ func (r *HelmRepo) Charts(fetchLatestOnly bool) ([]models.Chart, error) {
 		return []models.Chart{}, fmt.Errorf("no charts in repository index")
 	}
 
-	return filterCharts(charts, r.filter)
+	return filterCharts(unescapeChartsData(charts), r.filter)
 }
 
 // FetchFiles retrieves the important files of a chart and version from the repo
diff --git a/cmd/asset-syncer/server/utils_test.go b/cmd/asset-syncer/server/utils_test.go
index f92dc867b39..87fee323c6b 100644
--- a/cmd/asset-syncer/server/utils_test.go
+++ b/cmd/asset-syncer/server/utils_test.go
@@ -1296,6 +1296,101 @@ func Test_filterCharts(t *testing.T) {
 	}
 }
 
+func TestUnescapeCharsData(t *testing.T) {
+	tests := []struct {
+		description string
+		input       []models.Chart
+		expected    []models.Chart
+	}{
+		{
+			"chart with encoded spaces in id",
+			[]models.Chart{
+				{ID: "foo%20bar"},
+			},
+			[]models.Chart{
+				{ID: "foo bar"},
+			},
+		},
+		{
+			"chart with encoded spaces in name",
+			[]models.Chart{
+				{Name: "foo%20bar"},
+			},
+			[]models.Chart{
+				{Name: "foo bar"},
+			},
+		},
+		{
+			"chart with mixed encoding in name",
+			[]models.Chart{
+				{Name: "test/foo%20bar"},
+			},
+			[]models.Chart{
+				{Name: "test/foo bar"},
+			},
+		},
+		{
+			"chart with no encoding nor spaces",
+			[]models.Chart{
+				{Name: "test/foobar"},
+			},
+			[]models.Chart{
+				{Name: "test/foobar"},
+			},
+		},
+		{
+			"chart with unencoded spaces",
+			[]models.Chart{
+				{Name: "test/foo bar"},
+			},
+			[]models.Chart{
+				{Name: "test/foo bar"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			res := unescapeChartsData(tt.input)
+			if !cmp.Equal(res, tt.expected) {
+				t.Errorf("Unexpected result: %v", cmp.Diff(res, tt.expected))
+			}
+		})
+	}
+}
+
+func TestHelmRepoAppliesUnescape(t *testing.T) {
+	repo := &models.RepoInternal{Name: "test", Namespace: "repo-namespace", URL: "http://testrepo.com"}
+	expectedRepo := &models.Repo{Name: repo.Name, Namespace: repo.Namespace, URL: repo.URL}
+	repoIndexYAMLBytes, _ := ioutil.ReadFile("testdata/helm-index-spaces.yaml")
+	repoIndexYAML := string(repoIndexYAMLBytes)
+	expectedCharts := []models.Chart{
+		{
+			ID:            "test/chart with spaces",
+			Name:          "chart with spaces",
+			Repo:          expectedRepo,
+			Maintainers:   []chart.Maintainer{},
+			ChartVersions: []models.ChartVersion{{AppVersion: "v1"}},
+		},
+		{
+			ID:            "test/chart-without-spaces",
+			Name:          "chart-without-spaces",
+			Repo:          expectedRepo,
+			Maintainers:   []chart.Maintainer{},
+			ChartVersions: []models.ChartVersion{{AppVersion: "v2"}},
+		},
+	}
+	helmRepo := &HelmRepo{
+		content:      []byte(repoIndexYAML),
+		RepoInternal: repo,
+	}
+	t.Run("Helm repo applies unescaping to chart data", func(t *testing.T) {
+		charts, _ := helmRepo.Charts(false)
+		if !cmp.Equal(charts, expectedCharts) {
+			t.Errorf("Unexpected result: %v", cmp.Diff(charts, expectedCharts))
+		}
+	})
+}
+
 func Test_isURLDomainEqual(t *testing.T) {
 	tests := []struct {
 		name     string