Revert "fix: remove api double parameter unescaping (#740)" (#788)

This reverts commit f744acc.
kubeshop · Jan 17, 2022 · 1f1ab69 · 1f1ab69
1 parent 08d5f8a
commit 1f1ab69
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 10 deletions.
diff --git a/internal/app/api/v1/executions.go b/internal/app/api/v1/executions.go
@@ -140,7 +140,6 @@ func (s TestKubeAPI) ListExecutionsHandler() fiber.Handler {
 	}
 }
 
-// ExecutionLogsHandler returns execution logs for given execution id
 func (s TestKubeAPI) ExecutionLogsHandler() fiber.Handler {
 	return func(c *fiber.Ctx) error {
 		executionID := c.Params("executionID")
@@ -224,26 +223,31 @@ func (s TestKubeAPI) GetExecutionHandler() fiber.Handler {
 	}
 }
 
-// AbortExecutionHandler aborts script execution for given executor id
 func (s TestKubeAPI) AbortExecutionHandler() fiber.Handler {
 	return func(c *fiber.Ctx) error {
 		id := c.Params("id")
 		return s.Executor.Abort(id)
 	}
 }
 
-// GetArtifactHandler returns execution result file for given execution id and filename
 func (s TestKubeAPI) GetArtifactHandler() fiber.Handler {
 	return func(c *fiber.Ctx) error {
 		executionID := c.Params("executionID")
 		fileName := c.Params("filename")
 
+		// TODO fix this someday :) we don't know 15 mins before release why it's working this way
 		unescaped, err := url.QueryUnescape(fileName)
-		if err != nil {
-			return s.Error(c, http.StatusBadRequest, fmt.Errorf("can't unescape filename %s for execution %s with error %w", fileName, executionID, err))
+		if err == nil {
+			fileName = unescaped
+		}
+
+		unescaped, err = url.QueryUnescape(fileName)
+		if err == nil {
+			fileName = unescaped
 		}
 
-		fileName = unescaped
+		//// quickfix end
+
 		file, err := s.Storage.DownloadFile(executionID, fileName)
 		if err != nil {
 			return s.Error(c, http.StatusInternalServerError, err)
@@ -254,7 +258,7 @@ func (s TestKubeAPI) GetArtifactHandler() fiber.Handler {
 	}
 }
 
-// ListArtifactsHandler returns list of files for the given execution id
+// GetArtifacts returns list of files in the given bucket
 func (s TestKubeAPI) ListArtifactsHandler() fiber.Handler {
 	return func(c *fiber.Ctx) error {
 
@@ -268,7 +272,6 @@ func (s TestKubeAPI) ListArtifactsHandler() fiber.Handler {
 	}
 }
 
-// GetExecuteOptions returns execute options for given namespace, script id and request
 func (s TestKubeAPI) GetExecuteOptions(namespace, scriptID string, request testkube.ExecutionRequest) (options client.ExecuteOptions, err error) {
 	// get script content from kubernetes CRs
 	scriptCR, err := s.ScriptsClient.Get(namespace, scriptID)

diff --git a/pkg/api/v1/client/direct.go b/pkg/api/v1/client/direct.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -461,7 +462,7 @@ func (c DirectScriptsAPI) GetExecutionArtifacts(executionID string) (artifacts t
 }
 
 func (c DirectScriptsAPI) DownloadFile(executionID, fileName, destination string) (artifact string, err error) {
-	uri := c.getURI("/executions/%s/artifacts/%s", executionID, fileName)
+	uri := c.getURI("/executions/%s/artifacts/%s", executionID, url.QueryEscape(fileName))
 	resp, err := c.client.Get(uri)
 	if err != nil {
 		return artifact, err

diff --git a/pkg/api/v1/client/proxy.go b/pkg/api/v1/client/proxy.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -478,7 +479,7 @@ func (c ProxyScriptsAPI) GetExecutionArtifacts(executionID string) (artifacts te
 }
 
 func (c ProxyScriptsAPI) DownloadFile(executionID, fileName, destination string) (artifact string, err error) {
-	uri := c.getURI("/executions/%s/artifacts/%s", executionID, fileName)
+	uri := c.getURI("/executions/%s/artifacts/%s", executionID, url.QueryEscape(fileName))
 	req, err := c.GetProxy("GET").
 		Suffix(uri).
 		SetHeader("Accept", "text/event-stream").