Multipart upload - Repo key and repo path encoding (#993)

jfrog · Aug 5, 2024 · 901f0eb · 901f0eb
1 parent 32ba23f
commit 901f0eb
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 25 deletions.
diff --git a/artifactory/services/utils/multipartupload.go b/artifactory/services/utils/multipartupload.go
@@ -8,6 +8,7 @@ import (
 	"github.com/jfrog/gofrog/crypto"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"strings"
 	"sync"
@@ -262,8 +263,8 @@ func (mu *MultipartUpload) uploadPart(logMsgPrefix, localPath string, fileSize,
 }
 
 func (mu *MultipartUpload) createMultipartUpload(repoKey, repoPath string, partSize int64) (token string, err error) {
-	url := fmt.Sprintf("%s%screate?repoKey=%s&repoPath=%s&partSizeMB=%d", mu.artifactoryUrl, uploadsApi, repoKey, repoPath, partSize/SizeMiB)
-	resp, body, err := mu.client.SendPost(url, []byte{}, mu.httpClientsDetails)
+	requestUrl := fmt.Sprintf("%s%screate?repoKey=%s&repoPath=%s&partSizeMB=%d", mu.artifactoryUrl, uploadsApi, url.QueryEscape(repoKey), url.QueryEscape(repoPath), partSize/SizeMiB)
+	resp, body, err := mu.client.SendPost(requestUrl, []byte{}, mu.httpClientsDetails)
 	if err != nil {
 		return
 	}

diff --git a/artifactory/services/utils/multipartupload_test.go b/artifactory/services/utils/multipartupload_test.go
@@ -78,30 +78,48 @@ func TestUnsupportedVersion(t *testing.T) {
 }
 
 func TestCreateMultipartUpload(t *testing.T) {
-	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Check method
-		assert.Equal(t, http.MethodPost, r.Method)
-
-		// Check URL
-		assert.Equal(t, "/api/v1/uploads/create", r.URL.Path)
-		assert.Equal(t, fmt.Sprintf("repoKey=%s&repoPath=%s&partSizeMB=%d", repoKey, repoPath, partSizeMB), r.URL.RawQuery)
-
-		// Send response 200 OK
-		w.WriteHeader(http.StatusOK)
-		response, err := json.Marshal(createMultipartUploadResponse{Token: token})
-		assert.NoError(t, err)
-		_, err = w.Write(response)
-		assert.NoError(t, err)
-	})
-
-	// Create mock multipart upload with server
-	multipartUpload, cleanUp := createMockMultipartUpload(t, handler)
-	defer cleanUp()
+	testCases := []struct {
+		name                string
+		repoKey             string
+		repoPath            string
+		urlExpectedRepoKey  string
+		urlExpectedRepoPath string
+	}{
+		{"Single word names", repoKey, repoPath, repoKey, repoPath},
+		{"Spaced names", "repo with space", "path with space", "repo+with+space", "path+with+space"},
+		{"Names contains _", "repo_name", "path_name", "repo_name", "path_name"},
+		{"Names contains %", "repo%name", "path%name", "repo%25name", "path%25name"},
+		{"Names contains &", "repo&name", "path&name", "repo%26name", "path%26name"},
+	}
 
-	// Execute CreateMultipartUpload
-	actualToken, err := multipartUpload.createMultipartUpload(repoKey, repoPath, partSize)
-	assert.NoError(t, err)
-	assert.Equal(t, token, actualToken)
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				// Check method
+				assert.Equal(t, http.MethodPost, r.Method)
+
+				// Check URL
+				assert.Equal(t, "/api/v1/uploads/create", r.URL.Path)
+				assert.Equal(t, fmt.Sprintf("repoKey=%s&repoPath=%s&partSizeMB=%d", testCase.urlExpectedRepoKey, testCase.urlExpectedRepoPath, partSizeMB), r.URL.RawQuery)
+
+				// Send response 200 OK
+				w.WriteHeader(http.StatusOK)
+				response, err := json.Marshal(createMultipartUploadResponse{Token: token})
+				assert.NoError(t, err)
+				_, err = w.Write(response)
+				assert.NoError(t, err)
+			})
+
+			// Create mock multipart upload with server
+			multipartUpload, cleanUp := createMockMultipartUpload(t, handler)
+			defer cleanUp()
+
+			// Execute CreateMultipartUpload
+			actualToken, err := multipartUpload.createMultipartUpload(testCase.repoKey, testCase.repoPath, partSize)
+			assert.NoError(t, err)
+			assert.Equal(t, token, actualToken)
+		})
+	}
 }
 
 func TestUploadPartsConcurrentlyTooManyAttempts(t *testing.T) {