diff options
author | Catalin Irimie <cirimie@gitlab.com> | 2020-03-06 01:48:47 +0300 |
---|---|---|
committer | Catalin Irimie <cirimie@gitlab.com> | 2020-03-06 01:50:42 +0300 |
commit | 498143506760da4587b2313262ce5ba15a09579c (patch) | |
tree | 51c651714f23e55492e22343f91bfdc7dc09c21b | |
parent | 52b28999916a20b27c5caa6c399d871c22d146ca (diff) |
Encode artifact path to fix wrongly generated URLs
-rw-r--r-- | internal/artifact/artifact.go | 18 | ||||
-rw-r--r-- | internal/artifact/artifact_test.go | 9 |
2 files changed, 26 insertions, 1 deletions
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 28a595bb..91ecbcf8 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -130,6 +130,22 @@ func addCacheHeader(w http.ResponseWriter, resp *http.Response) { } } +// encodePathSegments separately encodes each segment of the path, as +// segments can have special characters in them, if the path is not valid +// and gets re-encoded by URL.Parse, %2f will get replaced with '/', +// breaking the namespace that we pass for group%2fproject. +// +// See https://github.com/golang/go/issues/6658 for more context +func encodePathSegments(path string) string { + parsed := strings.Split(path, "/") + + var encoded []string + for _, str := range parsed { + encoded = append(encoded, url.PathEscape(str)) + } + return strings.Join(encoded, "/") +} + // BuildURL returns a pointer to a url.URL for where the request should be // proxied to. The returned bool will indicate if there is some sort of issue // with the url while it is being generated. @@ -156,7 +172,7 @@ func (a *Artifact) BuildURL(host, requestPath string) (*url.URL, bool) { } jobID := parts[0][2] - artifactPath := parts[0][3] + artifactPath := encodePathSegments(parts[0][3]) projectID := url.PathEscape(path.Join(topGroup, restOfPath)) generated := fmt.Sprintf(apiURLTemplate, a.server, projectID, jobID, artifactPath) diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go index 689effb5..930e1438 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -143,6 +143,15 @@ func TestBuildURL(t *testing.T) { { "https://gitlab.com/api/v4/", "GROUP.GITLAB.IO", + "/-/SUBGROUP/PROJECT/-/JOBS/1/ARTIFACTS/PATH/TO/FILE {!#1.txt", + "https://gitlab.com/api/v4/projects/GROUP%2FSUBGROUP%2FPROJECT/jobs/1/artifacts/PATH/TO/FILE%20%7B%21%231.txt", + "gitlab.io", + true, + "Special characters in name", + }, + { + "https://gitlab.com/api/v4/", + "GROUP.GITLAB.IO", "/-/SUBGROUP/PROJECT/-/JOBS/1/ARTIFACTS/PATH/TO/FILE.txt", "https://gitlab.com/api/v4/projects/GROUP%2FSUBGROUP%2FPROJECT/jobs/1/artifacts/PATH/TO/FILE.txt", "gitlab.io", |