Validate archive length (#785)

* Validate archive length

internal/archive/zip/zip.go

@@ -39,59 +39,62 @@ func New(f io.Writer, matcher sauceignore.Matcher) (Writer, error) {
 }
 
 // Add adds the file at src to the destination dst in the archive and returns a count of
-// the files added to the archive.
-func (w *Writer) Add(src, dst string) (int, error) {
+// the files added to the archive, as well the length of the longest path.
+func (w *Writer) Add(src, dst string) (count int, length int, err error) {
 	finfo, err := os.Stat(src)
 	if err != nil {
-		return 0, err
+		return 0, 0, err
 	}
 
 	// Only will be applied if we have .sauceignore file and have patterns to exclude files and folders
 	if w.M.Match(strings.Split(src, string(os.PathSeparator)), finfo.IsDir()) {
-		return 0, nil
+		return 0, 0, nil
 	}
 
 	if !finfo.IsDir() {
 		log.Debug().Str("name", src).Msg("Adding to archive")
-		w, err := w.W.Create(path.Join(dst, finfo.Name()))
+		target := path.Join(dst, finfo.Name())
+		w, err := w.W.Create(target)
 		if err != nil {
-			return 0, err
+			return 0, 0, err
 		}
 		f, err := os.Open(src)
 		if err != nil {
-			return 0, err
+			return 0, 0, err
 		}
 
 		if _, err := io.Copy(w, f); err != nil {
-			return 0, err
+			return 0, 0, err
 		}
 
 		if err := f.Close(); err != nil {
-			return 0, err
+			return 0, 0, err
 		}
 
-		return 1, err
+		return 1, len(target), err
 	}
 
 	files, err := os.ReadDir(src)
 	if err != nil {
-		return 0, err
+		return 0, 0, err
 	}
 
-	totalFileCount := 0
 	for _, f := range files {
 		base := filepath.Base(src)
 		rebase := path.Join(dst, base)
 		fpath := filepath.Join(src, f.Name())
-		fileCount, err := w.Add(fpath, rebase)
+		fileCount, pathLength, err := w.Add(fpath, rebase)
 		if err != nil {
-			return 0, err
+			return 0, 0, err
 		}
 
-		totalFileCount += fileCount
+		count += fileCount
+		if pathLength > length {
+			length = pathLength
+		}
 	}
 
-	return totalFileCount, nil
+	return count, length, nil
 }
 
 // Close closes the archive. Adding more files to the archive is not possible after this.

internal/archive/zip/zip_test.go

@@ -3,6 +3,7 @@ package zip
 import (
 	"archive/zip"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -18,6 +19,8 @@ func TestZipper_Add(t *testing.T) {
 		fs.WithFile("some.other.bar.js", "bar", fs.WithMode(0755)))
 	defer dir.Remove()
 
+	dirBase := filepath.Base(dir.Path())
+
 	out, err := os.CreateTemp("", "add_test.*.zip")
 	if err != nil {
 		t.Errorf("failed to create temp file for storing the zip: %v", err)
@@ -41,20 +44,22 @@ func TestZipper_Add(t *testing.T) {
 		outName string
 	}
 	tests := []struct {
-		name      string
-		fields    fields
-		args      args
-		wantErr   bool
-		wantFiles []string
-		wantCount int
+		name       string
+		fields     fields
+		args       args
+		wantErr    bool
+		wantFiles  []string
+		wantCount  int
+		wantLength int
 	}{
 		{
-			name:      "zip it up",
-			fields:    fields{W: zip.NewWriter(out), M: sauceignore.NewMatcher([]sauceignore.Pattern{}), ZipFile: out},
-			args:      args{dir.Path(), "", out.Name()},
-			wantErr:   false,
-			wantFiles: []string{"/screenshot1.png", "/some.foo.js", "/some.other.bar.js"},
-			wantCount: 3,
+			name:       "zip it up",
+			fields:     fields{W: zip.NewWriter(out), M: sauceignore.NewMatcher([]sauceignore.Pattern{}), ZipFile: out},
+			args:       args{dir.Path(), "", out.Name()},
+			wantErr:    false,
+			wantFiles:  []string{"/screenshot1.png", "/some.foo.js", "/some.other.bar.js"},
+			wantCount:  3,
+			wantLength: len(dirBase) + len("/screenshots/screenshot1.png"),
 		},
 		{
 			name: "zip some.other.bar.js and skip some.foo.js file and screenshots folder",
@@ -66,10 +71,11 @@ func TestZipper_Add(t *testing.T) {
 				}),
 				ZipFile: sauceignoreOut,
 			},
-			args:      args{dir.Path(), "", sauceignoreOut.Name()},
-			wantErr:   false,
-			wantFiles: []string{"/some.other.bar.js"},
-			wantCount: 1,
+			args:       args{dir.Path(), "", sauceignoreOut.Name()},
+			wantErr:    false,
+			wantFiles:  []string{"/some.other.bar.js"},
+			wantCount:  1,
+			wantLength: len(dirBase) + len("/some.other.bar.js"),
 		},
 	}
 	for _, tt := range tests {
@@ -79,7 +85,7 @@ func TestZipper_Add(t *testing.T) {
 				M:       tt.fields.M,
 				ZipFile: tt.fields.ZipFile,
 			}
-			fileCount, err := z.Add(tt.args.src, tt.args.dst)
+			fileCount, pathLength, err := z.Add(tt.args.src, tt.args.dst)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Add() error = %v, wantErr %v", err, tt.wantErr)
 			}
@@ -100,7 +106,9 @@ func TestZipper_Add(t *testing.T) {
 			if tt.wantCount != fileCount {
 				t.Errorf("got %v, want %v", fileCount, tt.wantCount)
 			}
-
+			if tt.wantLength != pathLength {
+				t.Errorf("got %v, want %v", pathLength, tt.wantLength)
+			}
 		})
 	}
 }

internal/msg/msg.go

@@ -38,6 +38,9 @@ or peruse some of our example repositories:
 // ArchiveFileCountWarning is a warning to the user that their project archive may be unintentionally large.
 const ArchiveFileCountWarning = "The project archive is unusually large which can cause delays in your test execution."
 
+// ArchivePathLengthWarning is a warning to the user that their project archive may unintentionally contain long file paths.
+const ArchivePathLengthWarning = "The project archive contains paths that exceed the limit of %d characters. This archive may not be usable on Windows."
+
 // WarningLine is a line of starts highlighting the WARNING word.
 const WarningLine = "************************************* WARNING *************************************"
 
@@ -51,6 +54,13 @@ func LogArchiveSizeWarning() {
 	fmt.Printf("\n%s\n\n%s\n\n", warning.Sprint(ArchiveFileCountWarning), SauceIgnoreSuggestion)
 }
 
+// LogArchivePathLengthWarning prints out a warning about the project archive
+// path length along with suggestions on how to fix it.
+func LogArchivePathLengthWarning(limit int) {
+	warning := color.New(color.FgYellow)
+	fmt.Printf("\n%s\n\n%s\n\n", warning.Sprintf(ArchivePathLengthWarning, limit), SauceIgnoreSuggestion)
+}
+
 // LogSauceIgnoreNotExist prints out a formatted and color coded version of SauceIgnoreNotExist.
 func LogSauceIgnoreNotExist() {
 	red := color.New(color.FgRed).SprintFunc()

internal/saucecloud/cloud.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"os"
 	"os/signal"
 	"path"
@@ -85,13 +84,6 @@ type result struct {
 // ConsoleLogAsset represents job asset log file name.
 const ConsoleLogAsset = "console.log"
 
-// BaseFilepathLength represents the path length where project will be unpacked.
-// Example: "D:\sauce-playwright-runner\1.12.0\bundle\__project__\"
-const BaseFilepathLength = 53
-
-// MaxFilepathLength represents the maximum path length acceptable.
-const MaxFilepathLength = 255
-
 func (r *CloudRunner) createWorkerPool(ccy int, maxRetries int) (chan job.StartOptions, chan result, error) {
 	jobOpts := make(chan job.StartOptions, maxRetries+1)
 	results := make(chan result, ccy)
@@ -497,33 +489,6 @@ func (r *CloudRunner) remoteArchiveFiles(project interface{}, files []string, sa
 	return strings.Join(uris, ","), nil
 }
 
-func checkPathLength(projectFolder string, matcher sauceignore.Matcher) (string, error) {
-	exampleName := ""
-	maxLength := 0
-	if err := filepath.Walk(projectFolder, func(file string, info fs.FileInfo, err error) error {
-		if matcher.Match(strings.Split(file, string(os.PathSeparator)), info.IsDir()) {
-			if info.IsDir() {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-
-		if maxLength < len(file) {
-			exampleName = file
-			maxLength = len(file)
-		}
-		return nil
-	}); err != nil {
-		// When walk fails, we may not want to fail saucectl execution.
-		return "", nil
-	}
-
-	if BaseFilepathLength+maxLength > MaxFilepathLength {
-		return exampleName, errors.New("path too long")
-	}
-	return "", nil
-}
-
 type uploadType string
 
 var (

internal/saucecloud/cloud_test.go

@@ -270,94 +270,6 @@ func TestRunJobTimeoutRDC(t *testing.T) {
 	assert.True(t, res.job.TimedOut)
 }
 
-func TestCheckPathLength(t *testing.T) {
-	dir := fs.NewDir(t, "passing",
-		fs.WithDir("failing-test",
-			fs.WithMode(0755),
-			fs.WithDir("bqRamRa7aqyg3mDeaP8zvx7fUs5m5vr74g9ecPyAUkk93MyeETA6hWjyhgsPGtNQS9WEwJpmswcCADYJs7y8t55FsP79TZw7Fy7x",
-				fs.WithMode(0755),
-				fs.WithDir("dR6y58AjgHCunQ6VtrbbsWyhdMXLtf7xUAvuwmx67sqDpDW2Ln6bYFX6tzK8xufHM9UJWT9KLENTF4UtYehwxbZev59rUtWNbW2k",
-					fs.WithMode(0755),
-					fs.WithFile("test.spec.js", "dummy-content", fs.WithMode(0644)),
-				),
-			),
-		),
-		fs.WithDir("passing-test",
-			fs.WithMode(0755),
-			fs.WithDir("dir1",
-				fs.WithMode(0755),
-				fs.WithDir("dir2",
-					fs.WithMode(0755),
-					fs.WithFile("test.spec.js", "dummy-content", fs.WithMode(0644)),
-				),
-			),
-		),
-	)
-	defer dir.Remove()
-
-	// Use created dir as referential
-	wd, _ := os.Getwd()
-	defer func() {
-		if err := os.Chdir(wd); err != nil {
-			t.Errorf("failed to change directory to %s: %v", wd, err)
-		}
-	}()
-	if err := os.Chdir(dir.Path()); err != nil {
-		t.Errorf("failed to change directory to %s: %v", dir.Path(), err)
-	}
-
-	type args struct {
-		projectFolder string
-		matcher       sauceignore.Matcher
-	}
-	tests := []struct {
-		name    string
-		args    args
-		want    string
-		wantErr error
-	}{
-		{
-			name: "Passing filepath",
-			args: args{
-				projectFolder: "passing-test",
-				matcher:       sauceignore.NewMatcher([]sauceignore.Pattern{}),
-			},
-			want:    "",
-			wantErr: nil,
-		},
-		{
-			name: "Failing filepath",
-			args: args{
-				projectFolder: "failing-test",
-				matcher:       sauceignore.NewMatcher([]sauceignore.Pattern{}),
-			},
-			want:    "failing-test/bqRamRa7aqyg3mDeaP8zvx7fUs5m5vr74g9ecPyAUkk93MyeETA6hWjyhgsPGtNQS9WEwJpmswcCADYJs7y8t55FsP79TZw7Fy7x/dR6y58AjgHCunQ6VtrbbsWyhdMXLtf7xUAvuwmx67sqDpDW2Ln6bYFX6tzK8xufHM9UJWT9KLENTF4UtYehwxbZev59rUtWNbW2k/test.spec.js",
-			wantErr: errors.New("path too long"),
-		},
-		{
-			name: "Excluding filepath",
-			args: args{
-				projectFolder: "failing-test",
-				matcher: sauceignore.NewMatcher([]sauceignore.Pattern{
-					{
-						P: "bqRamRa7aqyg3mDeaP8zvx7fUs5m5vr74g9ecPyAUkk93MyeETA6hWjyhgsPGtNQS9WEwJpmswcCADYJs7y8t55FsP79TZw7Fy7x",
-					},
-				}),
-			},
-			want:    "",
-			wantErr: nil,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := checkPathLength(tt.args.projectFolder, tt.args.matcher)
-
-			assert.Equalf(t, tt.want, got, "checkPathLength(%v, %v)", tt.args.projectFolder, tt.args.matcher)
-			assert.Equalf(t, tt.wantErr, err, "checkPathLength(%v, %v)", tt.args.projectFolder, tt.args.matcher)
-		})
-	}
-}
-
 func TestCloudRunner_archiveNodeModules(t *testing.T) {
 	tempDir, err := os.MkdirTemp(os.TempDir(), "saucectl-app-payload-")
 	if err != nil {

internal/saucecloud/xcuitest.go

@@ -219,7 +219,7 @@ func archiveAppToIpa(appPath string) (string, error) {
 	}
 	arch, _ := zip.New(tmpFile, sauceignore.NewMatcher([]sauceignore.Pattern{}))
 	defer arch.Close()
-	_, err = arch.Add(appPath, "Payload/")
+	_, _, err = arch.Add(appPath, "Payload/")
 	if err != nil {
 		return "", err
 	}