Skip to content

Encode artifact path to fix wrongly generated URLs

Catalin Irimie requested to merge cat-fix-artifacts-urlencoded-361 into master

This is linked to #361 (closed) and the bug is actually related to a golang issue where %2F is "incorrectly" (in the eyes of the user.. but documented) decoded to "/", but we need it for the group%2Fproject construct.

Related to #361 (closed).

The golang interaction explained in a short snippet: playground link

url1 := "domain.test/A/B%2fC/D"	
url2 := "domain.test/A/B%2fC/D{"	

// %2F not changed, because the entire URL is "URL-encoded" valid, meaning
// no changes needed, so the path is left as-is, which is the normal working	
// scenario that we see with normal paths at the moment
fmt.Println(url.Parse(url1)) // => "domain.test/A/B/%2fC/D"
	
// %2F is "parsed" to "/" because the "{" character needs URL encoding
// so the entire path is getting "parsed". This is our current bug	
// when we get artifact paths with characters needing URL encoding
fmt.Println(url.Parse(url2)) // => "domain.test/A/B/C/D%7B"

The actual solution proposed in this MR is to URL encode each path segment of the artifact path (as there could be folders, files with special characters), a short example (with working playground link) to demonstrate why this works and encoding the entire path doesn't:

generated := fmt.Sprintf(apiURLTemplate, "gitlab.com/api/v4", projectID, jobID, artifactPath)

fmt.Println(url.Parse(generated)) // current scenario, %2F wrongly converted to "/"
	
generated = fmt.Sprintf(apiURLTemplate, "gitlab.com/api/v4", projectID, jobID, url.PathEscape(artifactPath))

fmt.Println(url.Parse(generated)) // doesn't work, as the actual "/"s in the artifactPath will get encoded as well

generated = fmt.Sprintf(apiURLTemplate, "gitlab.com/api/v4", projectID, jobID, encodePathSegments(artifactPath))
		
fmt.Println(url.Parse(generated)) // final _working_ solution, which encodes each path segment
Edited by Catalin Irimie

Merge request reports