Repository: helm/helm
Pull request #9763 (2021-06-02 12:30:14)

Ensure RawPath match Path when resolving reference
**What this PR does / why we need it**:

When the repo url contains URL-encoded parts, and doesn't end with `/`, relative downloads are not properly resolved.

Example (with a [patched Gitlab](https://gitlab.com/gitlab-org/gitlab/-/issues/18997#note_530270349)):

```shell
$ helm repo add foo https://gitlab.example.org/api/v4/projects/external-packages%2fargocd/packages/helm/stable
"foo" has been added to your repositories

$ helm show chart foo/argo-cd --debug
show.go:173: [debug] Original chart version: ""
Error: failed to fetch https://gitlab.example.org/api/v4/projects/external-packages/argocd/packages/helm/stable/charts/argo-cd-3.6.5.tgz : 404 Not Found
helm.go:81: [debug] failed to fetch https://gitlab.example.org/api/v4/projects/external-packages/argocd/packages/helm/stable/charts/argo-cd-3.6.5.tgz : 404 Not Found
helm.sh/helm/v3/pkg/getter.(*HTTPGetter).get
        /home/circleci/helm.sh/helm/pkg/getter/httpgetter.go:73
helm.sh/helm/v3/pkg/getter.(*HTTPGetter).Get
        /home/circleci/helm.sh/helm/pkg/getter/httpgetter.go:41
helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).DownloadTo
        /home/circleci/helm.sh/helm/pkg/downloader/chart_downloader.go:99
helm.sh/helm/v3/pkg/action.(*ChartPathOptions).LocateChart
        /home/circleci/helm.sh/helm/pkg/action/install.go:675
main.runShow
        /home/circleci/helm.sh/helm/cmd/helm/show.go:179
main.newShowCmd.func4
        /home/circleci/helm.sh/helm/cmd/helm/show.go:116
github.com/spf13/cobra.(*Command).execute
        /go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:850
github.com/spf13/cobra.(*Command).ExecuteC
        /go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:958
github.com/spf13/cobra.(*Command).Execute
        /go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:895
main.main
        /home/circleci/helm.sh/helm/cmd/helm/helm.go:80
runtime.main
        /usr/local/go/src/runtime/proc.go:204
runtime.goexit
        /usr/local/go/src/runtime/asm_amd64.s:1374
```

As you can see, `%2f` was replaced by `/` which leads to `404 Not found`.

Workaround: Add the slash when issuing `helm repo add`:

```shell
$ helm repo add foo  https://gitlab.k8s.qt.insee.test/api/v4/projects/external-packages%2fargocd/packages/helm/stable/
"foo" has been added to your repositories

$ helm show chart foo/argo-cd
apiVersion: v2
appVersion: 2.0.3
dependencies:
- condition: redis-ha.enabled
  name: redis-ha
  repository: https://dandydeveloper.github.io/charts/
  version: 4.12.14
description: A Helm chart for ArgoCD, a declarative, GitOps continuous delivery tool for Kubernetes.
home: https://github.com/argoproj/argo-helm
icon: https://raw.githubusercontent.com/argoproj/argo/master/docs/assets/argo.png
keywords:
- argoproj
- argocd
- gitops
maintainers:
- name: alexec
- name: alexmt
- name: jessesuen
- name: seanson
name: argo-cd
version: 3.6.5
```

**Special notes for your reviewer**:

Fixes: #9764.

**If applicable**:
- [ ] this PR contains documentation
- [ ] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


cndoit18: PRs must be linked to an existing issue. If none exists, may you please open one explaining the issue and expected outcome?
Please also write tests to test and verify this PR fixes your issue. Thanks.
sathieu: > 
> 
> PRs must be linked to an existing issue. If none exists, may you please open one explaining the issue and expected outcome?
> Please also write tests to test and verify this PR fixes your issue. Thanks.

Thanks @cndoit18. Issue #9764 filled.

sathieu: @cndoit18 This PR has tests and it fixes my problem.
sathieu: @cndoit18 Anything to do on this PR?
sathieu: Note: There is a similar problem for `helm dependency build` [here](https://github.com/helm/helm/blob/699ea6dcef302d7ded608bc8a1ab6c404de81685/pkg/downloader/manager.go#L780). Should I create another PR or push to this one?

Patch:
diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go
index 67ede93fd..956997cc9 100644
--- a/pkg/repo/chartrepo.go
+++ b/pkg/repo/chartrepo.go
@@ -286,7 +286,8 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName,
 // ResolveReferenceURL resolves refURL relative to baseURL.
 // If refURL is absolute, it simply returns refURL.
 func ResolveReferenceURL(baseURL, refURL string) (string, error) {
-	parsedBaseURL, err := url.Parse(baseURL)
+	// We need a trailing slash for ResolveReference to work, but make sure there isn't already one
+	parsedBaseURL, err := url.Parse(strings.TrimSuffix(baseURL, "/") + "/")
 	if err != nil {
 		return "", errors.Wrapf(err, "failed to parse %s as URL", baseURL)
 	}
@@ -296,8 +297,6 @@ func ResolveReferenceURL(baseURL, refURL string) (string, error) {
 		return "", errors.Wrapf(err, "failed to parse %s as URL", refURL)
 	}
 
-	// We need a trailing slash for ResolveReference to work, but make sure there isn't already one
-	parsedBaseURL.Path = strings.TrimSuffix(parsedBaseURL.Path, "/") + "/"
 	return parsedBaseURL.ResolveReference(parsedRefURL).String(), nil
 }
 


Test patch:
diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go
index 85401284e..0f317b2fd 100644
--- a/pkg/repo/chartrepo_test.go
+++ b/pkg/repo/chartrepo_test.go
@@ -400,4 +400,12 @@ func TestResolveReferenceURL(t *testing.T) {
 	if chartURL != "https://charts.helm.sh/stable/nginx-0.2.0.tgz" {
 		t.Errorf("%s", chartURL)
 	}
+
+	chartURL, err = ResolveReferenceURL("http://localhost:8123/charts%2fwith%2fescaped%2fslash", "nginx-0.2.0.tgz")
+	if err != nil {
+		t.Errorf("%s", err)
+	}
+	if chartURL != "http://localhost:8123/charts%2fwith%2fescaped%2fslash/nginx-0.2.0.tgz" {
+		t.Errorf("%s", chartURL)
+	}
 }