From 8bef9a80937394c3713d260e9eb0ca4eba087294 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Fri, 4 Aug 2023 08:19:32 +0200 Subject: [PATCH] Always copy PHP and legacy CLI files if they have changed --- .github/workflows/ci.yml | 1 + Makefile | 13 +++-- commands/completion.go | 19 +------ commands/list.go | 25 ++------- commands/root.go | 24 ++++----- internal/file/file.go | 79 +++++++++++++++++++++++++++ internal/file/file_test.go | 63 ++++++++++++++++++++++ internal/legacy/legacy.go | 91 ++++++------------------------- internal/legacy/php_unix.go | 9 ++-- internal/legacy/php_windows.go | 97 ++++++++++++++++++++++------------ 10 files changed, 251 insertions(+), 170 deletions(-) create mode 100644 internal/file/file.go create mode 100644 internal/file/file_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4bf9f0a..2f554f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,7 @@ jobs: touch internal/legacy/archives/php_darwin_amd64 touch internal/legacy/archives/php_darwin_arm64 touch internal/config/embedded-config.yaml + touch internal/legacy/archives/php_windows.zip.sha256 - name: Run linter uses: golangci/golangci-lint-action@v3 diff --git a/Makefile b/Makefile index 213518d..34d74d1 100644 --- a/Makefile +++ b/Makefile @@ -49,14 +49,21 @@ internal/legacy/archives/php_linux_$(GOARCH): --progress=plain \ ext/static-php-cli/docker +PHP_WINDOWS_REMOTE_FILENAME := "php-$(PHP_VERSION)-nts-Win32-vs16-x64.zip" internal/legacy/archives/php_windows.zip: - mkdir -p internal/legacy/archives - wget https://windows.php.net/downloads/releases/php-$(PHP_VERSION)-nts-Win32-vs16-x64.zip -O internal/legacy/archives/php_windows.zip + ( \ + mkdir -p internal/legacy/archives ;\ + cd internal/legacy/archives ;\ + # Using curl (instead of wget) because it's built in to things like Git Bash. + curl "https://windows.php.net/downloads/releases/$(PHP_WINDOWS_REMOTE_FILENAME)" > php_windows.zip ;\ + curl https://windows.php.net/downloads/releases/sha256sum.txt | grep "$(PHP_WINDOWS_REMOTE_FILENAME)" | sed s/"$(PHP_WINDOWS_REMOTE_FILENAME)"/"php_windows.zip"/g > php_windows.zip.sha256 ;\ + sha256sum -c php_windows.zip.sha256 ;\ + ) .PHONY: internal/legacy/archives/cacert.pem internal/legacy/archives/cacert.pem: mkdir -p internal/legacy/archives - wget https://curl.se/ca/cacert.pem -O internal/legacy/archives/cacert.pem + curl https://curl.se/ca/cacert.pem > internal/legacy/archives/cacert.pem php: $(PHP_BINARY_PATH) diff --git a/commands/completion.go b/commands/completion.go index 8ab8ae2..1cd9868 100644 --- a/commands/completion.go +++ b/commands/completion.go @@ -2,14 +2,10 @@ package commands import ( "bytes" - "errors" "fmt" - "os" - "os/exec" "path" "strings" - "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -39,21 +35,8 @@ func newCompletionCommand(cnf *config.Config) *cobra.Command { Stdin: cmd.InOrStdin(), } - if err := c.Init(); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - os.Exit(1) - return - } - if err := c.Exec(cmd.Context(), completionArgs...); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - exitCode := 1 - var execErr *exec.ExitError - if errors.As(err, &execErr) { - exitCode = execErr.ExitCode() - } - os.Exit(exitCode) - return + handleLegacyError(err) } completions := strings.ReplaceAll( diff --git a/commands/list.go b/commands/list.go index b3446ff..87ce402 100644 --- a/commands/list.go +++ b/commands/list.go @@ -3,10 +3,8 @@ package commands import ( "bytes" "encoding/json" - "errors" "fmt" "os" - "os/exec" "github.com/fatih/color" "github.com/spf13/cobra" @@ -33,11 +31,6 @@ func newListCommand(cnf *config.Config) *cobra.Command { Stderr: cmd.ErrOrStderr(), Stdin: cmd.InOrStdin(), } - if err := c.Init(); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - os.Exit(1) - return - } arguments := []string{"list", "--format=json"} if viper.GetBool("all") { @@ -46,15 +39,9 @@ func newListCommand(cnf *config.Config) *cobra.Command { if len(args) > 0 { arguments = append(arguments, args[0]) } + if err := c.Exec(cmd.Context(), arguments...); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - exitCode := 1 - var execErr *exec.ExitError - if errors.As(err, &execErr) { - exitCode = execErr.ExitCode() - } - os.Exit(exitCode) - return + handleLegacyError(err) } var list List @@ -99,13 +86,7 @@ func newListCommand(cnf *config.Config) *cobra.Command { c.Stdout = cmd.OutOrStdout() arguments := []string{"list", "--format=" + format} if err := c.Exec(cmd.Context(), arguments...); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - exitCode := 1 - var execErr *exec.ExitError - if errors.As(err, &execErr) { - exitCode = execErr.ExitCode() - } - os.Exit(exitCode) + handleLegacyError(err) } return } diff --git a/commands/root.go b/commands/root.go index 3d4a7b2..b0836d0 100644 --- a/commands/root.go +++ b/commands/root.go @@ -79,20 +79,9 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob Stderr: cmd.ErrOrStderr(), Stdin: cmd.InOrStdin(), } - if err := c.Init(); err != nil { - log.Println(color.RedString(err.Error())) - os.Exit(1) - return - } if err := c.Exec(cmd.Context(), os.Args[1:]...); err != nil { - debugLog("%s\n", color.RedString(err.Error())) - exitCode := 1 - var execErr *exec.ExitError - if errors.As(err, &execErr) { - exitCode = execErr.ExitCode() - } - os.Exit(exitCode) + handleLegacyError(err) } }, PersistentPostRun: func(_ *cobra.Command, _ []string) { @@ -248,3 +237,14 @@ func debugLog(format string, v ...any) { log.Printf(format, v...) } + +func handleLegacyError(err error) { + var execErr *exec.ExitError + if errors.As(err, &execErr) { + exitCode := execErr.ExitCode() + debugLog("%s\n", err) + os.Exit(exitCode) + } + log.Println(color.RedString(err.Error())) + os.Exit(1) +} diff --git a/internal/file/file.go b/internal/file/file.go new file mode 100644 index 0000000..c5e1aa3 --- /dev/null +++ b/internal/file/file.go @@ -0,0 +1,79 @@ +package file + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "io" + "os" +) + +// CopyIfChanged copies source data to a destination filename if it has changed. +// It is considered changed if its length or contents are different. +func CopyIfChanged(destFilename string, source []byte, perm os.FileMode) error { + matches, err := compare(destFilename, source) + if (err != nil && !os.IsNotExist(err)) || matches { + return err + } + return os.WriteFile(destFilename, source, perm) +} + +// compare checks if a file matches the given source. +func compare(filename string, data []byte) (bool, error) { + f, err := os.Open(filename) + if err != nil { + return false, err + } + defer f.Close() + + fi, err := f.Stat() + if err != nil { + return false, err + } + if fi.Size() != int64(len(data)) { + return false, nil + } + + var ( + buf = make([]byte, 64*1024) + offset = 0 + ) + for { + n, err := f.Read(buf) + if err != nil { + if err == io.EOF { + break + } + return false, err + } + if offset+n > len(data) || !bytes.Equal(data[offset:offset+n], buf[:n]) { + return false, nil + } + offset += n + } + + return offset == len(data), nil +} + +// CheckHash checks if a file has the given SHA256 hash. +func CheckHash(filename, hash string) (bool, error) { + fh, err := sha256File(filename) + if err != nil { + return false, err + } + return fh == hash, nil +} + +// sha256File calculates the SHA256 hash of a file. +func sha256File(filename string) (string, error) { + f, err := os.Open(filename) + if err != nil { + return "", err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "", err + } + return hex.EncodeToString(h.Sum(nil)), nil +} diff --git a/internal/file/file_test.go b/internal/file/file_test.go new file mode 100644 index 0000000..cf37fa9 --- /dev/null +++ b/internal/file/file_test.go @@ -0,0 +1,63 @@ +package file + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCopyIfChanged(t *testing.T) { + cases := []struct { + name string + initialData []byte + sourceData []byte + expectWrite bool + }{ + {"File does not exist", nil, []byte("new data"), true}, + {"File matches source", []byte("same data"), []byte("same data"), false}, + {"File content differs", []byte("old data"), []byte("new data"), true}, + {"File size differs", []byte("short"), []byte("much longer data"), true}, + {"Empty source", []byte("existing data"), []byte{}, true}, + } + + tmpDir := t.TempDir() + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + destFile := filepath.Join(tmpDir, "testfile") + + if c.initialData != nil { + require.NoError(t, os.WriteFile(destFile, c.initialData, 0644)) + time.Sleep(time.Millisecond * 5) + } + + var modTimeBeforeCopy time.Time + stat, err := os.Stat(destFile) + if c.initialData == nil { + require.True(t, os.IsNotExist(err)) + } else { + require.NoError(t, err) + modTimeBeforeCopy = stat.ModTime() + } + + err = CopyIfChanged(destFile, c.sourceData, 0644) + require.NoError(t, err) + + statAfterCopy, err := os.Stat(destFile) + require.NoError(t, err) + if c.expectWrite { + assert.Greater(t, statAfterCopy.ModTime().Truncate(time.Millisecond), modTimeBeforeCopy.Truncate(time.Millisecond)) + } else { + assert.Equal(t, modTimeBeforeCopy.Truncate(time.Millisecond), statAfterCopy.ModTime().Truncate(time.Millisecond)) + } + + data, err := os.ReadFile(destFile) + require.NoError(t, err) + + assert.Equal(t, data, c.sourceData) + }) + } +} diff --git a/internal/legacy/legacy.go b/internal/legacy/legacy.go index 3664cc9..4f00246 100644 --- a/internal/legacy/legacy.go +++ b/internal/legacy/legacy.go @@ -1,7 +1,6 @@ package legacy import ( - "bytes" "context" _ "embed" "fmt" @@ -14,6 +13,7 @@ import ( "github.com/gofrs/flock" "github.com/platformsh/cli/internal/config" + "github.com/platformsh/cli/internal/file" ) //go:embed archives/platform.phar @@ -24,48 +24,8 @@ var ( PHPVersion = "0.0.0" ) -var phpPath = fmt.Sprintf("php-%s", PHPVersion) -var pharPath = fmt.Sprintf("phar-%s", LegacyCLIVersion) - -// copyFile from the given bytes to destination -func copyFile(destination string, fin []byte) error { - if _, err := os.Stat(destination); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("could not stat file: %w", err) - } - - fout, err := os.Create(destination) - if err != nil { - return fmt.Errorf("could not create file: %w", err) - } - defer fout.Close() - - r := bytes.NewReader(fin) - - if _, err := io.Copy(fout, r); err != nil { - return fmt.Errorf("could copy file: %w", err) - } - - return nil -} - -// fileChanged checks if a file's content differs from the provided bytes. -func fileChanged(filename string, content []byte) (bool, error) { - stat, err := os.Stat(filename) - if err != nil { - if os.IsNotExist(err) { - return true, nil - } - return false, fmt.Errorf("could not stat file: %w", err) - } - if int(stat.Size()) != len(content) { - return true, nil - } - current, err := os.ReadFile(filename) - if err != nil { - return false, err - } - return !bytes.Equal(current, content), nil -} +var phpPath = "php" +var pharPath = "legacy-cli.phar" // CLIWrapper wraps the legacy CLI type CLIWrapper struct { @@ -80,11 +40,11 @@ type CLIWrapper struct { } func (c *CLIWrapper) cacheDir() string { - return path.Join(os.TempDir(), fmt.Sprintf("%s-%s-%s", c.Config.Application.Slug, PHPVersion, LegacyCLIVersion)) + return path.Join(os.TempDir(), c.Config.Application.Slug) } -// Init the CLI wrapper, creating a temporary directory and copying over files -func (c *CLIWrapper) Init() error { +// Initialize the CLI wrapper, creating a temporary directory and copying over files. +func (c *CLIWrapper) init() error { if _, err := os.Stat(c.cacheDir()); os.IsNotExist(err) { c.debugLog("cache directory does not exist, creating: %s", c.cacheDir()) if err := os.Mkdir(c.cacheDir(), 0o700); err != nil { @@ -96,18 +56,10 @@ func (c *CLIWrapper) Init() error { return fmt.Errorf("could not acquire lock: %w", err) } c.debugLog("lock acquired: %s", fileLock.Path()) - //nolint:errcheck - defer fileLock.Unlock() - - if _, err := os.Stat(c.PharPath()); os.IsNotExist(err) { - if c.CustomPharPath != "" { - return fmt.Errorf("legacy CLI phar file not found: %w", err) - } + defer fileLock.Unlock() //nolint:errcheck - c.debugLog("phar file does not exist, copying: %s", c.PharPath()) - if err := copyFile(c.PharPath(), phar); err != nil { - return fmt.Errorf("could not copy phar file: %w", err) - } + if err := file.CopyIfChanged(c.PharPath(), phar, 0o644); err != nil { + return fmt.Errorf("could not copy phar file: %w", err) } // Always write the config.yaml file if it changed. @@ -115,31 +67,18 @@ func (c *CLIWrapper) Init() error { if err != nil { return fmt.Errorf("could not load config for checking: %w", err) } - changed, err := fileChanged(c.ConfigPath(), configContent) - if err != nil { - return fmt.Errorf("could not check config file: %w", err) - } - if changed { - if err := copyFile(c.ConfigPath(), configContent); err != nil { - return fmt.Errorf("could not copy config: %w", err) - } + if err := file.CopyIfChanged(c.ConfigPath(), configContent, 0o644); err != nil { + return fmt.Errorf("could not write config: %w", err) } - if _, err := os.Stat(c.PHPPath()); os.IsNotExist(err) { - c.debugLog("PHP binary does not exist, copying: %s", c.PHPPath()) - if err := c.copyPHP(); err != nil { - return fmt.Errorf("could not copy files: %w", err) - } - if err := os.Chmod(c.PHPPath(), 0o700); err != nil { - return fmt.Errorf("could not make PHP executable: %w", err) - } - } - - return nil + return c.copyPHP() } // Exec a legacy CLI command with the given arguments func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error { + if err := c.init(); err != nil { + return fmt.Errorf("failed to initialize CLI: %w", err) + } args = append([]string{c.PharPath()}, args...) cmd := exec.CommandContext(ctx, c.PHPPath(), args...) //nolint:gosec if c.Stdin != nil { diff --git a/internal/legacy/php_unix.go b/internal/legacy/php_unix.go index c85a6a1..85e2134 100644 --- a/internal/legacy/php_unix.go +++ b/internal/legacy/php_unix.go @@ -4,17 +4,14 @@ package legacy import ( - "fmt" "path" + + "github.com/platformsh/cli/internal/file" ) // copyPHP to destination, if it does not exist func (c *CLIWrapper) copyPHP() error { - if err := copyFile(c.PHPPath(), phpCLI); err != nil { - return fmt.Errorf("could not copy PHP CLI: %w", err) - } - - return nil + return file.CopyIfChanged(c.PHPPath(), phpCLI, 0o755) } // PHPPath returns the path that the PHP CLI will reside diff --git a/internal/legacy/php_windows.go b/internal/legacy/php_windows.go index 2bf0b9f..46986e5 100644 --- a/internal/legacy/php_windows.go +++ b/internal/legacy/php_windows.go @@ -7,15 +7,19 @@ import ( "fmt" "io" "os" - "path" "path/filepath" "strings" "text/template" + + "github.com/platformsh/cli/internal/file" ) //go:embed archives/php_windows.zip var phpCLI []byte +//go:embed archives/php_windows.zip.sha256 +var phpCLIHash string + //go:embed archives/windows_php.ini.tpl var phpIniTemplate string @@ -24,7 +28,15 @@ var caCert []byte // copyPHP to destination, if it does not exist func (c *CLIWrapper) copyPHP() error { - dest := path.Join(c.cacheDir(), "php") + destDir := filepath.Join(c.cacheDir(), "php") + hashPath := filepath.Join(destDir, "hash") + hashOK, err := file.CheckHash(hashPath, phpCLIHash) + if err != nil && !os.IsNotExist(err) { + return err + } + if hashOK { + return nil + } br := bytes.NewReader(phpCLI) r, err := zip.NewReader(br, int64(len(phpCLI))) if err != nil { @@ -32,47 +44,66 @@ func (c *CLIWrapper) copyPHP() error { } for _, f := range r.File { - rc, err := f.Open() - if err != nil { - return fmt.Errorf("could not open zipped file %s: %w", f.Name, err) - } - defer rc.Close() - - fpath := filepath.Join(dest, f.Name[strings.Index(f.Name, string(os.PathSeparator))+1:]) - if f.FileInfo().IsDir() { - continue - } - - if lastIndex := strings.LastIndex(fpath, string(os.PathSeparator)); lastIndex > -1 { - fdir := fpath[:lastIndex] - if err := os.MkdirAll(fdir, 0755); err != nil { - return fmt.Errorf("could create parent directory %s: %w", fdir, err) - } - } - - f, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode()) - if err != nil { - return fmt.Errorf("could not open file to unzip %s: %w", fpath, err) - } - defer f.Close() - - if _, err := io.Copy(f, rc); err != nil { - return fmt.Errorf("could not write zipped file %s: %w", fpath, err) + if err := copyZipFile(f, destDir); err != nil { + return err } } - w, err := os.Create(path.Join(c.cacheDir(), "php", "php.ini")) + w, err := os.Create(filepath.Join(c.cacheDir(), "php", "php.ini")) if err != nil { return fmt.Errorf("could not open php.ini file for writing: %w", err) } defer w.Close() - template.Must(template.New("php.ini").Parse(phpIniTemplate)).Execute(w, map[string]string{"CLIDir": c.cacheDir()}) - copyFile(path.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert) - return nil + tpl := template.Must(template.New("php.ini").Parse(phpIniTemplate)) + if err := tpl.Execute(w, map[string]string{"CLIDir": c.cacheDir()}); err != nil { + return fmt.Errorf("could not write php.ini file: %w", err) + } + + if err := os.WriteFile(filepath.Join(c.cacheDir(), "php", "extras", "cacert.pem"), caCert, 0o644); err != nil { + return err + } + + return file.CopyIfChanged(hashPath, []byte(phpCLIHash), 0o644) } // PHPPath returns the path that the PHP CLI will reside func (c *CLIWrapper) PHPPath() string { - return path.Join(c.cacheDir(), "php", "php.exe") + return filepath.Join(c.cacheDir(), "php", "php.exe") +} + +func copyZipFile(f *zip.File, destDir string) error { + absPath := filepath.Join(destDir, f.Name) + if !strings.HasPrefix(absPath, filepath.Clean(destDir)+string(os.PathSeparator)) { + return fmt.Errorf("invalid file path: %s", absPath) + } + + if f.FileInfo().IsDir() { + if err := os.MkdirAll(absPath, 0755); err != nil { + return fmt.Errorf("could create extracted directory %s: %w", absPath, err) + } + return nil + } + + if err := os.MkdirAll(filepath.Dir(absPath), 0755); err != nil { + return fmt.Errorf("could create parent directory for extracted file %s: %w", absPath, err) + } + + rc, err := f.Open() + if err != nil { + return fmt.Errorf("could not open file in zip archive %s: %w", f.Name, err) + } + defer rc.Close() + + destFile, err := os.OpenFile(absPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, f.Mode()) + if err != nil { + return fmt.Errorf("could not open destination for extracted file %s: %w", absPath, err) + } + defer destFile.Close() + + if _, err := io.Copy(destFile, rc); err != nil { + return fmt.Errorf("could not write extracted file %s: %w", absPath, err) + } + + return nil }