From f745670a6c049976f1e62bdbb688553043287b4c Mon Sep 17 00:00:00 2001
From: Steven Edwards <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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 <steven@stephenwithav.com>
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
 	}
