From f745670a6c049976f1e62bdbb688553043287b4c Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Wed, 1 Jul 2020 15:37:47 -0400 Subject: [PATCH 1/7] Verify rsync protocol version match prior to proceeding. `sync.Version()` proceeds with a warning that there may be a version mismatch if it timesout. `syncCmd.version` assumes rsync is present in the path. `wsep.RemoteExecer` isn't available publicly on GitHub, so I took a best guess with it and `wsep.ExitError`. --- cmd/coder/sync.go | 41 +++++++++++++++++++++++++++++++++++++++++ internal/sync/sync.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/cmd/coder/sync.go b/cmd/coder/sync.go index 0e26af96..4db032eb 100644 --- a/cmd/coder/sync.go +++ b/cmd/coder/sync.go @@ -1,7 +1,12 @@ package main import ( + "bufio" + "errors" + "fmt" + "log" "os" + "os/exec" "path/filepath" "strings" @@ -29,6 +34,32 @@ func (cmd *syncCmd) RegisterFlags(fl *pflag.FlagSet) { fl.BoolVarP(&cmd.init, "init", "i", false, "do initial transfer and exit") } +// See https://lxadm.com/Rsync_exit_codes#List_of_standard_rsync_exit_codes. +var IncompatRsync = errors.New("rsync: exit status 2") +var StreamErrRsync = errors.New("rsync: exit status 12") + +// Returns local rsync protocol version as a string. +func (s *syncCmd) version() string { + cmd := exec.Command("rsync", "--version") + stdout, err := cmd.StdoutPipe() + if err != nil { + log.Fatal(err) + } + + if err := cmd.Start(); err != nil { + log.Fatal(err) + } + + r := bufio.NewReader(stdout) + if err := cmd.Wait(); err != nil { + log.Fatal(err) + } + + versionString := strings.Split(r.ReadLine(), "protocol version ") + + return versionString[1] +} + func (cmd *syncCmd) Run(fl *pflag.FlagSet) { var ( local = fl.Arg(0) @@ -71,6 +102,16 @@ func (cmd *syncCmd) Run(fl *pflag.FlagSet) { LocalDir: absLocal, Client: entClient, } + + localVersion := s.version() + remoteVersion, rsyncErr := sync.Version() + + if rsyncErr != nil { + flog.Info("Unable to determine remote rsync version. Proceeding cautiously.") + } else if localVersion != remoteVersion { + flog.Fatal(fmt.Sprintf("rsync protocol mismatch. local is %s; remote is %s.", localVersion, remoteVersion)) + } + for err == nil || err == sync.ErrRestartSync { err = s.Run() } diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 32086896..8a5a0f65 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -1,6 +1,7 @@ package sync import ( + "bufio" "context" "errors" "fmt" @@ -10,6 +11,7 @@ import ( "os/exec" "path" "path/filepath" + "strings" "sync" "sync/atomic" "time" @@ -261,6 +263,41 @@ const ( maxAcceptableDispatch = time.Millisecond * 50 ) +// Returns remote protocol version as a string. +// Or, an error if one exists. +func (s Sync) Version() (string, error) { + conn, err := s.Client.DialWsep(ctx, s.Env) + if err != nil { + return "", err + } + defer conn.Close(websocket.CloseNormalClosure, "") + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + execer := wsep.RemoteExecer(conn) + process, err := execer.Start(ctx, wsep.Command{ + Command: "rsync", + Args: []string{"--version"}, + }) + if err != nil { + return "", err + } + r := bufio.NewReader(process.Stdout()) + + err = process.Wait() + if code, ok := err.(wsep.ExitError); ok { + return "", err + } + if err != nil { + return "", err + } + + versionString := strings.Split(r.ReadLine(), "protocol version ") + + return versionString[1], nil +} + // Run starts the sync synchronously. // Use this command to debug what wasn't sync'd correctly: // rsync -e "coder sh" -nicr ~/Projects/cdr/coder-cli/. ammar:/home/coder/coder-cli/ From 5538af614d7bf1ca7277c94f074bfd8b1a7ec6cd Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Wed, 1 Jul 2020 19:56:50 -0400 Subject: [PATCH 2/7] Add context to sync.Version. --- internal/sync/sync.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 8a5a0f65..c8158578 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -266,15 +266,15 @@ const ( // Returns remote protocol version as a string. // Or, an error if one exists. func (s Sync) Version() (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + conn, err := s.Client.DialWsep(ctx, s.Env) if err != nil { return "", err } defer conn.Close(websocket.CloseNormalClosure, "") - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer cancel() - execer := wsep.RemoteExecer(conn) process, err := execer.Start(ctx, wsep.Command{ Command: "rsync", From 54cecb24a578b30de0ce04fb6d906723bd01ef62 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Wed, 1 Jul 2020 20:19:08 -0400 Subject: [PATCH 3/7] Correct syncCmd.version, sync.Version. Passing `s.ReadLine` to `strings.Split` was a mistake since it returns three values. Having access to `wsep` helped the compiler find the bugs. --- cmd/coder/sync.go | 11 ++++++++--- internal/sync/sync.go | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/coder/sync.go b/cmd/coder/sync.go index 4db032eb..5d7f0fa9 100644 --- a/cmd/coder/sync.go +++ b/cmd/coder/sync.go @@ -55,7 +55,12 @@ func (s *syncCmd) version() string { log.Fatal(err) } - versionString := strings.Split(r.ReadLine(), "protocol version ") + firstLine, err := r.ReadString('\n') + if err != nil { + return "" + } + + versionString := strings.Split(firstLine, "protocol version ") return versionString[1] } @@ -103,8 +108,8 @@ func (cmd *syncCmd) Run(fl *pflag.FlagSet) { Client: entClient, } - localVersion := s.version() - remoteVersion, rsyncErr := sync.Version() + localVersion := cmd.version() + remoteVersion, rsyncErr := s.Version() if rsyncErr != nil { flog.Info("Unable to determine remote rsync version. Proceeding cautiously.") diff --git a/internal/sync/sync.go b/internal/sync/sync.go index c8158578..2b680b1a 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -287,13 +287,18 @@ func (s Sync) Version() (string, error) { err = process.Wait() if code, ok := err.(wsep.ExitError); ok { - return "", err + return "", fmt.Errorf("Version heck exit status: %v", code) + } + if err != nil { + return "", fmt.Errorf("Server version mismatch") } + + firstLine, err := r.ReadString('\n') if err != nil { return "", err } - versionString := strings.Split(r.ReadLine(), "protocol version ") + versionString := strings.Split(firstLine, "protocol version ") return versionString[1], nil } From 35ee1de3486391990152e541a37fd341c760b358 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Thu, 2 Jul 2020 09:34:29 -0400 Subject: [PATCH 4/7] Update local, remote version checks to avoid blanks. --- cmd/coder/sync.go | 25 +++++-------------------- internal/sync/sync.go | 7 ++++--- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/cmd/coder/sync.go b/cmd/coder/sync.go index 5d7f0fa9..9f209912 100644 --- a/cmd/coder/sync.go +++ b/cmd/coder/sync.go @@ -1,8 +1,7 @@ package main import ( - "bufio" - "errors" + "bytes" "fmt" "log" "os" @@ -34,32 +33,18 @@ func (cmd *syncCmd) RegisterFlags(fl *pflag.FlagSet) { fl.BoolVarP(&cmd.init, "init", "i", false, "do initial transfer and exit") } -// See https://lxadm.com/Rsync_exit_codes#List_of_standard_rsync_exit_codes. -var IncompatRsync = errors.New("rsync: exit status 2") -var StreamErrRsync = errors.New("rsync: exit status 12") - // Returns local rsync protocol version as a string. -func (s *syncCmd) version() string { +func (_ *syncCmd) version() string { cmd := exec.Command("rsync", "--version") - stdout, err := cmd.StdoutPipe() + out, err := cmd.CombinedOutput() if err != nil { log.Fatal(err) } - if err := cmd.Start(); err != nil { - log.Fatal(err) - } - - r := bufio.NewReader(stdout) - if err := cmd.Wait(); err != nil { - log.Fatal(err) - } - - firstLine, err := r.ReadString('\n') + firstLine, err := bytes.NewBuffer(out).ReadString('\n') if err != nil { - return "" + log.Fatal(err) } - versionString := strings.Split(firstLine, "protocol version ") return versionString[1] diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 2b680b1a..79aa00c1 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -1,7 +1,7 @@ package sync import ( - "bufio" + "bytes" "context" "errors" "fmt" @@ -283,7 +283,8 @@ func (s Sync) Version() (string, error) { if err != nil { return "", err } - r := bufio.NewReader(process.Stdout()) + buf := &bytes.Buffer{} + go io.Copy(buf, process.Stdout()) err = process.Wait() if code, ok := err.(wsep.ExitError); ok { @@ -293,7 +294,7 @@ func (s Sync) Version() (string, error) { return "", fmt.Errorf("Server version mismatch") } - firstLine, err := r.ReadString('\n') + firstLine, err := buf.ReadString('\n') if err != nil { return "", err } From 21976c65f28b437d9df10a00ca4851444666ce32 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Thu, 2 Jul 2020 10:26:51 -0400 Subject: [PATCH 5/7] Remove go from io.Copy in internal/sync/sync.go#Version --- internal/sync/sync.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 79aa00c1..868c70ca 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -284,11 +284,11 @@ func (s Sync) Version() (string, error) { return "", err } buf := &bytes.Buffer{} - go io.Copy(buf, process.Stdout()) + io.Copy(buf, process.Stdout()) err = process.Wait() if code, ok := err.(wsep.ExitError); ok { - return "", fmt.Errorf("Version heck exit status: %v", code) + return "", fmt.Errorf("Version check exit status: %v", code) } if err != nil { return "", fmt.Errorf("Server version mismatch") From d853066ae320cf5fbd04d5896e8f9aaf3a249265 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Thu, 2 Jul 2020 10:52:03 -0400 Subject: [PATCH 6/7] Return error directly in internal/sync/sync.go#Version --- cmd/coder/sync.go | 2 +- internal/sync/sync.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/coder/sync.go b/cmd/coder/sync.go index 9f209912..e8ec9ac0 100644 --- a/cmd/coder/sync.go +++ b/cmd/coder/sync.go @@ -99,7 +99,7 @@ func (cmd *syncCmd) Run(fl *pflag.FlagSet) { if rsyncErr != nil { flog.Info("Unable to determine remote rsync version. Proceeding cautiously.") } else if localVersion != remoteVersion { - flog.Fatal(fmt.Sprintf("rsync protocol mismatch. local is %s; remote is %s.", localVersion, remoteVersion)) + flog.Fatal(fmt.Sprintf("rsync protocol mismatch. %s.", localVersion, rsyncErr)) } for err == nil || err == sync.ErrRestartSync { diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 868c70ca..52968ea7 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -287,11 +287,11 @@ func (s Sync) Version() (string, error) { io.Copy(buf, process.Stdout()) err = process.Wait() - if code, ok := err.(wsep.ExitError); ok { - return "", fmt.Errorf("Version check exit status: %v", code) + if _, ok := err.(wsep.ExitError); ok { + return "", err } if err != nil { - return "", fmt.Errorf("Server version mismatch") + return "", err } firstLine, err := buf.ReadString('\n') From e8abef25061e41f355f7f4b05f08f0e3e9ce6dd5 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Thu, 2 Jul 2020 11:17:32 -0400 Subject: [PATCH 7/7] Update [vV]ersion comment strings; remove extraneous error check. --- cmd/coder/sync.go | 2 +- internal/sync/sync.go | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/coder/sync.go b/cmd/coder/sync.go index e8ec9ac0..e0d62708 100644 --- a/cmd/coder/sync.go +++ b/cmd/coder/sync.go @@ -33,7 +33,7 @@ func (cmd *syncCmd) RegisterFlags(fl *pflag.FlagSet) { fl.BoolVarP(&cmd.init, "init", "i", false, "do initial transfer and exit") } -// Returns local rsync protocol version as a string. +// version returns local rsync protocol version as a string. func (_ *syncCmd) version() string { cmd := exec.Command("rsync", "--version") out, err := cmd.CombinedOutput() diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 52968ea7..b00cbeb1 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -263,7 +263,7 @@ const ( maxAcceptableDispatch = time.Millisecond * 50 ) -// Returns remote protocol version as a string. +// Version returns remote protocol version as a string. // Or, an error if one exists. func (s Sync) Version() (string, error) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) @@ -287,9 +287,6 @@ func (s Sync) Version() (string, error) { io.Copy(buf, process.Stdout()) err = process.Wait() - if _, ok := err.(wsep.ExitError); ok { - return "", err - } if err != nil { return "", err }