Merge pull request #153 from Pix4D/pci-3688-cogito-encode-url

cogito: properly encode url
Pix4D · Mar 26, 2024 · fd7191d · fd7191d
2 parents b1b895f + 35d30ac
commit fd7191d
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 2 deletions.
diff --git a/cogito/putter.go b/cogito/putter.go
@@ -333,6 +333,12 @@ func concourseBuildURL(env Environment) string {
 	// https://ci.example.com/teams/main/pipelines/cogito/jobs/autocat/builds/3?vars=%7B%22branch%22%3A%22stable%22%7D
 	if env.BuildPipelineInstanceVars != "" {
 		buildURL += fmt.Sprintf("?vars=%s", url.QueryEscape(env.BuildPipelineInstanceVars))
+		// Concourse requires that the space characters
+		// are encoded as %20 instead of +. On the other hand
+		// golangs url.QueryEscape as well as url.Values.Encode()
+		// both encode the space as a + character.
+		// See: https://github.com/golang/go/issues/4013
+		buildURL = strings.ReplaceAll(buildURL, "+", "%20")
 	}
 
 	return buildURL

diff --git a/cogito/putter_private_test.go b/cogito/putter_private_test.go
@@ -342,17 +342,29 @@ func TestConcourseBuildURL(t *testing.T) {
 			want: "https://ci.example.com/teams/devs/pipelines/magritte/jobs/paint/builds/42",
 		},
 		{
-			name: "instanced vars 1",
+			name: "single instance variable",
 			env: testhelp.MergeStructs(baseEnv,
 				Environment{BuildPipelineInstanceVars: `{"branch":"stable"}`}),
 			want: "https://ci.example.com/teams/devs/pipelines/magritte/jobs/paint/builds/42?vars=%7B%22branch%22%3A%22stable%22%7D",
 		},
 		{
-			name: "instanced vars 2",
+			name: "single instance variable with spaces",
+			env: testhelp.MergeStructs(baseEnv,
+				Environment{BuildPipelineInstanceVars: `{"branch":"foo bar"}`}),
+			want: "https://ci.example.com/teams/devs/pipelines/magritte/jobs/paint/builds/42?vars=%7B%22branch%22%3A%22foo%20bar%22%7D",
+		},
+		{
+			name: "multiple instance variables",
 			env: testhelp.MergeStructs(baseEnv,
 				Environment{BuildPipelineInstanceVars: `{"branch":"stable","foo":"bar"}`}),
 			want: "https://ci.example.com/teams/devs/pipelines/magritte/jobs/paint/builds/42?vars=%7B%22branch%22%3A%22stable%22%2C%22foo%22%3A%22bar%22%7D",
 		},
+		{
+			name: "multiple instance variables: nested json with spaces",
+			env: testhelp.MergeStructs(baseEnv,
+				Environment{BuildPipelineInstanceVars: `{"branch":"foo bar","version":{"from":1.0,"to":2.0}}`}),
+			want: "https://ci.example.com/teams/devs/pipelines/magritte/jobs/paint/builds/42?vars=%7B%22branch%22%3A%22foo%20bar%22%2C%22version%22%3A%7B%22from%22%3A1.0%2C%22to%22%3A2.0%7D%7D",
+		},
 	}
 
 	for _, tc := range testCases {