From a1a99285a8b35c7dd040cde48a9c5ce69c709618 Mon Sep 17 00:00:00 2001 From: Dejan K Date: Wed, 4 Feb 2026 15:10:37 +0100 Subject: [PATCH 1/2] feat: Enhance GetObject method to handle plus signs in URLs and improve error handling --- pkg/api/signed_url.go | 23 ++++++++++++++++++----- pkg/api/signed_url_test.go | 14 ++++++++++++++ pkg/storage/pull.go | 7 ++++++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/pkg/api/signed_url.go b/pkg/api/signed_url.go index 523cb4f..def6da1 100644 --- a/pkg/api/signed_url.go +++ b/pkg/api/signed_url.go @@ -212,27 +212,40 @@ func (u *SignedURL) delete(client *retryablehttp.Client, artifact *Artifact) err func (u *SignedURL) GetObject() (string, error) { URL, _ := url.Parse(u.URL) + var obj string + var err error switch host := URL.Host; { case host == "storage.googleapis.com": log.Debugf("Parsing GCS URL: %s\n", u.URL) - return parseGoogleStorageURL(URL) + obj, err = parseGoogleStorageURL(URL) case strings.HasSuffix(host, "amazonaws.com"): log.Debugf("Parsing S3 URL: %s\n", u.URL) - return parseS3URL(URL) + obj, err = parseS3URL(URL) case strings.HasPrefix(host, "127.0.0.1"): log.Debugf("Parsing localhost URL: %s\n", u.URL) - return parseLocalhostURL(URL) + obj, err = parseLocalhostURL(URL) case customDomainRegex.Match([]byte(URL.String())): log.Debugf("Parsing custom domain URL: %s\n", u.URL) - return parseCustomDomainURL(URL) + obj, err = parseCustomDomainURL(URL) default: log.Warnf("Failed to parse URL '%s' - unrecognized host '%s'\n", u.URL, host) return "", fmt.Errorf("unrecognized host %s", host) } + + if err != nil { + return "", err + } + + decoded, err := url.PathUnescape(obj) + if err != nil { + return "", fmt.Errorf("failed to decode object path '%s': %v", obj, err) + } + + return decoded, nil } // GCS URLs follow the format 'https://storage.googleapis.com//' @@ -280,5 +293,5 @@ func parseCustomDomainURL(URL *url.URL) (string, error) { // Localhost URLs are used during tests func parseLocalhostURL(URL *url.URL) (string, error) { // we don't want the leading slash - return URL.Path[1:], nil + return strings.TrimPrefix(URL.EscapedPath(), "/"), nil } diff --git a/pkg/api/signed_url_test.go b/pkg/api/signed_url_test.go index 32b24c4..08a7ceb 100644 --- a/pkg/api/signed_url_test.go +++ b/pkg/api/signed_url_test.go @@ -14,6 +14,13 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/myfile.txt", obj) }) + t.Run("GCS - file with plus", func(t *testing.T) { + signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/test_art%2Bifact.txt?Expires=231256754712"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) + }) + t.Run("GCS - file inside directory", func(t *testing.T) { signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/mydir/myfile.txt?Expires=231256754712"} obj, err := signedURL.GetObject() @@ -28,6 +35,13 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/myfile.txt", obj) }) + t.Run("S3 - file with plus", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/test_art%2Bifact.txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) + }) + t.Run("S3 with region-less URL - file", func(t *testing.T) { signedURL := SignedURL{URL: "https://my-bucket1.s3.amazonaws.com/projectid/artifacts/project/projectid/myfile.txt?X-Amz-Whatever"} obj, err := signedURL.GetObject() diff --git a/pkg/storage/pull.go b/pkg/storage/pull.go index a46330e..06c1dc0 100644 --- a/pkg/storage/pull.go +++ b/pkg/storage/pull.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path" + "strings" api "github.com/semaphoreci/artifact/pkg/api" "github.com/semaphoreci/artifact/pkg/files" @@ -60,7 +61,11 @@ func buildArtifacts(signedURLs []*api.SignedURL, paths *files.ResolvedPath, forc return nil, err } - localPath := path.Join(paths.Destination, obj[len(paths.Source):]) + relative := strings.TrimPrefix(obj, paths.Source) + if relative == obj { + return nil, fmt.Errorf("failed to resolve local path: remote object '%s' does not match source '%s'", obj, paths.Source) + } + localPath := path.Join(paths.Destination, relative) if !force { if _, err := os.Stat(localPath); err == nil { From ac1ba8b332123721aaf6396b34c9e15d97d51aa8 Mon Sep 17 00:00:00 2001 From: Dejan K Date: Mon, 9 Feb 2026 09:44:11 +0100 Subject: [PATCH 2/2] fix: Improve URL parsing and add tests for special characters handling in signed URLs --- pkg/api/signed_url.go | 6 +- pkg/api/signed_url_test.go | 123 +++++++++++++++++++++++++ pkg/storage/pull.go | 17 +++- pkg/storage/pull_test.go | 181 ++++++++++++++++++++++++++++++++++++- 4 files changed, 320 insertions(+), 7 deletions(-) diff --git a/pkg/api/signed_url.go b/pkg/api/signed_url.go index def6da1..ff3264f 100644 --- a/pkg/api/signed_url.go +++ b/pkg/api/signed_url.go @@ -210,10 +210,12 @@ func (u *SignedURL) delete(client *retryablehttp.Client, artifact *Artifact) err } func (u *SignedURL) GetObject() (string, error) { - URL, _ := url.Parse(u.URL) + URL, err := url.Parse(u.URL) + if err != nil { + return "", fmt.Errorf("failed to parse URL '%s': %v", u.URL, err) + } var obj string - var err error switch host := URL.Host; { case host == "storage.googleapis.com": log.Debugf("Parsing GCS URL: %s\n", u.URL) diff --git a/pkg/api/signed_url_test.go b/pkg/api/signed_url_test.go index 08a7ceb..117e304 100644 --- a/pkg/api/signed_url_test.go +++ b/pkg/api/signed_url_test.go @@ -21,6 +21,36 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) }) + t.Run("GCS - file with space", func(t *testing.T) { + signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/my%20file.txt?Expires=231256754712"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/my file.txt", obj) + }) + + t.Run("GCS - file with multiple special chars", func(t *testing.T) { + signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/build%2B%2B/output%20(1).txt?Expires=231256754712"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/build++/output (1).txt", obj) + }) + + t.Run("GCS - file with percent literal", func(t *testing.T) { + signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/100%25done.txt?Expires=231256754712"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/100%done.txt", obj) + }) + + t.Run("GCS - file with literal %2B in name", func(t *testing.T) { + // Filename is literally "file%2Bname.txt" on disk. + // SDK double-encodes: %2B -> %252B in the signed URL. + signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/file%252Bname.txt?Expires=231256754712"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/file%2Bname.txt", obj) + }) + t.Run("GCS - file inside directory", func(t *testing.T) { signedURL := SignedURL{URL: "https://storage.googleapis.com/my-bucket1/artifacts/project/projectid/mydir/myfile.txt?Expires=231256754712"} obj, err := signedURL.GetObject() @@ -42,6 +72,43 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) }) + t.Run("S3 - file with space", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/my%20file.txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/my file.txt", obj) + }) + + t.Run("S3 - file with multiple special chars", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/build%2B%2B/output%20(1).txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/build++/output (1).txt", obj) + }) + + t.Run("S3 - file with percent literal", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/100%25done.txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/100%done.txt", obj) + }) + + t.Run("S3 - file with literal %2B in name", func(t *testing.T) { + // Filename is literally "file%2Bname.txt" on disk. + // SDK double-encodes: %2B -> %252B in the signed URL. + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/file%252Bname.txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/file%2Bname.txt", obj) + }) + + t.Run("S3 region-less - file with plus", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.amazonaws.com/projectid/artifacts/project/projectid/test_art%2Bifact.txt?X-Amz-Whatever"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) + }) + t.Run("S3 with region-less URL - file", func(t *testing.T) { signedURL := SignedURL{URL: "https://my-bucket1.s3.amazonaws.com/projectid/artifacts/project/projectid/myfile.txt?X-Amz-Whatever"} obj, err := signedURL.GetObject() @@ -77,6 +144,27 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/mydir/myfile.txt", obj) }) + t.Run("127.0.0.1 - file with plus", func(t *testing.T) { + signedURL := SignedURL{URL: "http://127.0.0.1:8080/artifacts/project/projectid/test_art%2Bifact.txt"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) + }) + + t.Run("127.0.0.1 - file with space", func(t *testing.T) { + signedURL := SignedURL{URL: "http://127.0.0.1:8080/artifacts/project/projectid/my%20file.txt"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/my file.txt", obj) + }) + + t.Run("127.0.0.1 - file with literal %2B in name", func(t *testing.T) { + signedURL := SignedURL{URL: "http://127.0.0.1:8080/artifacts/project/projectid/file%252Bname.txt"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/file%2Bname.txt", obj) + }) + t.Run("custom domain - file", func(t *testing.T) { signedURL := SignedURL{URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/project/projectid/myfile.txt?X-Amz-Algorithm"} obj, err := signedURL.GetObject() @@ -91,6 +179,41 @@ func Test__GetObject(t *testing.T) { assert.Equal(t, "artifacts/project/projectid/mydir/myfile.txt", obj) }) + t.Run("custom domain - file with plus", func(t *testing.T) { + signedURL := SignedURL{URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/project/projectid/test_art%2Bifact.txt?X-Amz-Algorithm"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/test_art+ifact.txt", obj) + }) + + t.Run("custom domain - file with space", func(t *testing.T) { + signedURL := SignedURL{URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/project/projectid/my%20file.txt?X-Amz-Algorithm"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/my file.txt", obj) + }) + + t.Run("custom domain - file with multiple special chars", func(t *testing.T) { + signedURL := SignedURL{URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/project/projectid/build%2B%2B/output%20(1).txt?X-Amz-Algorithm"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/build++/output (1).txt", obj) + }) + + t.Run("custom domain - file with literal %2B in name", func(t *testing.T) { + signedURL := SignedURL{URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/project/projectid/file%252Bname.txt?X-Amz-Algorithm"} + obj, err := signedURL.GetObject() + assert.Nil(t, err) + assert.Equal(t, "artifacts/project/projectid/file%2Bname.txt", obj) + }) + + t.Run("S3 - malformed URL escape returns parse error", func(t *testing.T) { + signedURL := SignedURL{URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/project/projectid/bad%2Xname.txt?X-Amz-Whatever"} + _, err := signedURL.GetObject() + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "failed to parse URL") + }) + t.Run("bad URL", func(t *testing.T) { signedURL := SignedURL{URL: "http://somehost.com/projectid/artifacts/project/projectid/myfile.txt"} _, err := signedURL.GetObject() diff --git a/pkg/storage/pull.go b/pkg/storage/pull.go index 06c1dc0..cead7b3 100644 --- a/pkg/storage/pull.go +++ b/pkg/storage/pull.go @@ -61,8 +61,8 @@ func buildArtifacts(signedURLs []*api.SignedURL, paths *files.ResolvedPath, forc return nil, err } - relative := strings.TrimPrefix(obj, paths.Source) - if relative == obj { + relative, ok := objectRelativeToSource(obj, paths.Source) + if !ok { return nil, fmt.Errorf("failed to resolve local path: remote object '%s' does not match source '%s'", obj, paths.Source) } localPath := path.Join(paths.Destination, relative) @@ -83,6 +83,19 @@ func buildArtifacts(signedURLs []*api.SignedURL, paths *files.ResolvedPath, forc return artifacts, nil } +func objectRelativeToSource(object, source string) (string, bool) { + if object == source { + return "", true + } + + sourcePrefix := strings.TrimSuffix(source, "/") + "/" + if strings.HasPrefix(object, sourcePrefix) { + return strings.TrimPrefix(object, sourcePrefix), true + } + + return "", false +} + func doPull(artifacts []*api.Artifact) (*PullStats, error) { client := newHTTPClient() stats := &PullStats{} diff --git a/pkg/storage/pull_test.go b/pkg/storage/pull_test.go index ce8e04a..9d4c2b3 100644 --- a/pkg/storage/pull_test.go +++ b/pkg/storage/pull_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/semaphoreci/artifact/pkg/api" + "github.com/semaphoreci/artifact/pkg/files" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -45,7 +46,7 @@ func Test__doPull_Stats(t *testing.T) { // Mock the doPull function to skip actual HTTP calls // We'll test the stats collection logic by creating a modified version stats := &PullStats{} - + // Simulate the stats collection that happens in doPull for _, artifact := range artifacts { if fileInfo, err := os.Stat(artifact.LocalPath); err == nil { @@ -62,11 +63,185 @@ func Test__doPull_Stats(t *testing.T) { func Test__PullStats_EmptyDirectory(t *testing.T) { // Test with no files stats := &PullStats{} - + assert.Equal(t, 0, stats.FileCount) assert.Equal(t, int64(0), stats.TotalSize) } +func Test__buildArtifacts_SpecialChars(t *testing.T) { + t.Run("file with plus in name - S3 encoded URL", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/test_art%2Bifact.txt?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/test_art+ifact.txt", + Destination: "test_art+ifact.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "test_art+ifact.txt", artifacts[0].LocalPath) + assert.Equal(t, "artifacts/workflows/wfid/test_art+ifact.txt", artifacts[0].RemotePath) + }) + + t.Run("file with plus in name - GCS encoded URL", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://storage.googleapis.com/my-bucket1/artifacts/workflows/wfid/test_art%2Bifact.txt?Expires=231256754712", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/test_art+ifact.txt", + Destination: "test_art+ifact.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "test_art+ifact.txt", artifacts[0].LocalPath) + }) + + t.Run("file with plus in name - custom domain URL", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://artifacts.somedomain.com/my-bucket1/projectid/artifacts/workflows/wfid/test_art%2Bifact.txt?X-Amz-Algorithm", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/test_art+ifact.txt", + Destination: "test_art+ifact.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "test_art+ifact.txt", artifacts[0].LocalPath) + }) + + t.Run("file with space in name - S3 encoded URL", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/my%20file.txt?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/my file.txt", + Destination: "my file.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "my file.txt", artifacts[0].LocalPath) + }) + + t.Run("directory pull with plus in name - multiple files", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/build%2B%2B/main.o?X-Amz-Whatever", Method: "GET"}, + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/build%2B%2B/lib.o?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/build++", + Destination: "build++", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 2) + assert.Equal(t, "build++/main.o", artifacts[0].LocalPath) + assert.Equal(t, "build++/lib.o", artifacts[1].LocalPath) + }) + + t.Run("source prefix mismatch returns error", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/other_file.txt?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/expected_file.txt", + Destination: "expected_file.txt", + } + + _, err := buildArtifacts(signedURLs, paths, true) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "does not match source") + }) + + t.Run("source prefix boundary mismatch returns error", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/build%2B%2B/main.o?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/build+", + Destination: "build+", + } + + _, err := buildArtifacts(signedURLs, paths, true) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "does not match source") + }) + + t.Run("file with literal %2B in name - S3 double-encoded URL", func(t *testing.T) { + // Filename on disk is literally "file%2Bname.txt" (not file+name.txt). + // The SDK double-encodes %2B -> %252B in the signed URL. + // GetObject should decode one level: %252B -> %2B (preserving the literal). + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/file%252Bname.txt?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/file%2Bname.txt", + Destination: "file%2Bname.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "file%2Bname.txt", artifacts[0].LocalPath) + assert.Equal(t, "artifacts/workflows/wfid/file%2Bname.txt", artifacts[0].RemotePath) + }) + + t.Run("file with literal %2B in name - GCS double-encoded URL", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://storage.googleapis.com/my-bucket1/artifacts/workflows/wfid/file%252Bname.txt?Expires=231256754712", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/file%2Bname.txt", + Destination: "file%2Bname.txt", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "file%2Bname.txt", artifacts[0].LocalPath) + }) + + t.Run("literal %2B must not be confused with actual plus", func(t *testing.T) { + // URL has %252B (literal %2B), but source path has actual + sign. + // These should NOT match - the decoded object path "file%2Bname.txt" + // does not have prefix "artifacts/workflows/wfid/file+name.txt". + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/file%252Bname.txt?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/file+name.txt", + Destination: "file+name.txt", + } + + _, err := buildArtifacts(signedURLs, paths, true) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "does not match source") + }) + + t.Run("complex docker path with plus - real world case", func(t *testing.T) { + signedURLs := []*api.SignedURL{ + {URL: "https://my-bucket1.s3.us-east-1.amazonaws.com/projectid/artifacts/workflows/wfid/lib/bild/test-build%2Bdeps-check/v1.432.1-2-ab123-build-test.gz?X-Amz-Whatever", Method: "GET"}, + } + paths := &files.ResolvedPath{ + Source: "artifacts/workflows/wfid/lib/bild/test-build+deps-check/v1.432.1-2-ab123-build-test.gz", + Destination: "lib/bild/test-build+deps-check/v1.432.1-2-ab123-build-test.gz", + } + + artifacts, err := buildArtifacts(signedURLs, paths, true) + assert.Nil(t, err) + require.Len(t, artifacts, 1) + assert.Equal(t, "lib/bild/test-build+deps-check/v1.432.1-2-ab123-build-test.gz", artifacts[0].LocalPath) + }) +} + func Test__PullStats_LargeFiles(t *testing.T) { // Create temporary directory for test files tempDir, err := ioutil.TempDir("", "pull_large_test") @@ -90,7 +265,7 @@ func Test__PullStats_LargeFiles(t *testing.T) { } stats := &PullStats{} - + // Simulate stats collection if fileInfo, err := os.Stat(artifact.LocalPath); err == nil { stats.FileCount++