client.go

@@ -3,6 +3,7 @@
  * terms of the Mozilla Public License 2.0, which is distributed
  * along with the source code.
  */
+
 package scp
 
 import (
@@ -23,27 +24,27 @@ import (
 type PassThru func(r io.Reader, total int64) io.Reader
 
 type Client struct {
-	// the host to connect to
+	// Host the host to connect to.
 	Host string
 
-	// the client config to use
+	// ClientConfig the client config to use.
 	ClientConfig *ssh.ClientConfig
 
-	// stores the SSH session while the connection is running
+	// Session stores the SSH session while the connection is running.
 	Session *ssh.Session
 
-	// stores the SSH connection itself in order to close it after transfer
+	// Conn stores the SSH connection itself in order to close it after transfer.
 	Conn ssh.Conn
 
-	// the maximal amount of time to wait for a file transfer to complete
+	// Timeout the maximal amount of time to wait for a file transfer to complete.
 	// Deprecated: use context.Context for each function instead.
 	Timeout time.Duration
 
-	// the absolute path to the remote SCP binary
+	// RemoteBinary the absolute path to the remote SCP binary.
 	RemoteBinary string
 }
 
-// Connects to the remote SSH server, returns error if it couldn't establish a session to the SSH server
+// Connect connects to the remote SSH server, returns error if it couldn't establish a session to the SSH server.
 func (a *Client) Connect() error {
 	if a.Session != nil {
 		return nil
@@ -62,35 +63,41 @@ func (a *Client) Connect() error {
 	return nil
 }
 
-// Copies the contents of an os.File to a remote location, it will get the length of the file by looking it up from the filesystem
+// CopyFromFile copies the contents of an os.File to a remote location, it will get the length of the file by looking it up from the filesystem.
 func (a *Client) CopyFromFile(ctx context.Context, file os.File, remotePath string, permissions string) error {
 	return a.CopyFromFilePassThru(ctx, file, remotePath, permissions, nil)
 }
 
-// Copies the contents of an os.File to a remote location, it will get the length of the file by looking it up from the filesystem.
-// Access copied bytes by providing a PassThru reader factory
+// CopyFromFilePassThru copies the contents of an os.File to a remote location, it will get the length of the file by looking it up from the filesystem.
+// Access copied bytes by providing a PassThru reader factory.
 func (a *Client) CopyFromFilePassThru(ctx context.Context, file os.File, remotePath string, permissions string, passThru PassThru) error {
-	stat, _ := file.Stat()
+	stat, err := file.Stat()
+	if err != nil {
+		return fmt.Errorf("failed to stat file: %w", err)
+	}
 	return a.CopyPassThru(ctx, &file, remotePath, permissions, stat.Size(), passThru)
 }
 
-// Copies the contents of an io.Reader to a remote location, the length is determined by reading the io.Reader until EOF
-// if the file length in know in advance please use "Copy" instead
+// CopyFile copies the contents of an io.Reader to a remote location, the length is determined by reading the io.Reader until EOF
+// if the file length in know in advance please use "Copy" instead.
 func (a *Client) CopyFile(ctx context.Context, fileReader io.Reader, remotePath string, permissions string) error {
 	return a.CopyFilePassThru(ctx, fileReader, remotePath, permissions, nil)
 }
 
-// Copies the contents of an io.Reader to a remote location, the length is determined by reading the io.Reader until EOF
+// CopyFilePassThru copies the contents of an io.Reader to a remote location, the length is determined by reading the io.Reader until EOF
 // if the file length in know in advance please use "Copy" instead.
-// Access copied bytes by providing a PassThru reader factory
+// Access copied bytes by providing a PassThru reader factory.
 func (a *Client) CopyFilePassThru(ctx context.Context, fileReader io.Reader, remotePath string, permissions string, passThru PassThru) error {
-	contents_bytes, _ := ioutil.ReadAll(fileReader)
-	bytes_reader := bytes.NewReader(contents_bytes)
+	contentsBytes, err := ioutil.ReadAll(fileReader)
+	if err != nil {
+		return fmt.Errorf("failed to read all data from reader: %w", err)
+	}
+	bytesReader := bytes.NewReader(contentsBytes)
 
-	return a.CopyPassThru(ctx, bytes_reader, remotePath, permissions, int64(len(contents_bytes)), passThru)
+	return a.CopyPassThru(ctx, bytesReader, remotePath, permissions, int64(len(contentsBytes)), passThru)
 }
 
-// waitTimeout waits for the waitgroup for the specified max timeout.
+// wait waits for the waitgroup for the specified max timeout.
 // Returns true if waiting timed out.
 func wait(wg *sync.WaitGroup, ctx context.Context) error {
 	c := make(chan struct{})
@@ -108,8 +115,8 @@ func wait(wg *sync.WaitGroup, ctx context.Context) error {
 	}
 }
 
-// Checks the response it reads from the remote, and will return a single error in case
-// of failure
+// checkResponse checks the response it reads from the remote, and will return a single error in case
+// of failure.
 func checkResponse(r io.Reader) error {
 	response, err := ParseResponse(r)
 	if err != nil {
@@ -124,18 +131,23 @@ func checkResponse(r io.Reader) error {
 
 }
 
-// Copies the contents of an io.Reader to a remote location
+// Copy copies the contents of an io.Reader to a remote location.
 func (a *Client) Copy(ctx context.Context, r io.Reader, remotePath string, permissions string, size int64) error {
 	return a.CopyPassThru(ctx, r, remotePath, permissions, size, nil)
 }
 
-// Copies the contents of an io.Reader to a remote location.
+// CopyPassThru copies the contents of an io.Reader to a remote location.
 // Access copied bytes by providing a PassThru reader factory
 func (a *Client) CopyPassThru(ctx context.Context, r io.Reader, remotePath string, permissions string, size int64, passThru PassThru) error {
 	stdout, err := a.Session.StdoutPipe()
 	if err != nil {
 		return err
 	}
+	w, err := a.Session.StdinPipe()
+	if err != nil {
+		return err
+	}
+	defer w.Close()
 
 	if passThru != nil {
 		r = passThru(r, size)
@@ -150,12 +162,6 @@ func (a *Client) CopyPassThru(ctx context.Context, r io.Reader, remotePath strin
 
 	go func() {
 		defer wg.Done()
-		w, err := a.Session.StdinPipe()
-		if err != nil {
-			errCh <- err
-			return
-		}
-
 		defer w.Close()
 
 		_, err = fmt.Fprintln(w, "C"+permissions, size, filename)
@@ -215,30 +221,30 @@ func (a *Client) CopyPassThru(ctx context.Context, r io.Reader, remotePath strin
 	return nil
 }
 
-// Copy a file from the remote to the local file given by the `file`
+// CopyFromRemote copies a file from the remote to the local file given by the `file`
 // parameter. Use `CopyFromRemotePassThru` if a more generic writer
 // is desired instead of writing directly to a file on the file system.?
 func (a *Client) CopyFromRemote(ctx context.Context, file *os.File, remotePath string) error {
 	return a.CopyFromRemotePassThru(ctx, file, remotePath, nil)
 }
 
-// Copy a file from the remote to the given writer. The passThru parameter can be used
+// CopyFromRemotePassThru copies a file from the remote to the given writer. The passThru parameter can be used
 // to keep track of progress and how many bytes that were download from the remote.
 // `passThru` can be set to nil to disable this behaviour.
 func (a *Client) CopyFromRemotePassThru(ctx context.Context, w io.Writer, remotePath string, passThru PassThru) error {
 	wg := sync.WaitGroup{}
-	errCh := make(chan error, 1)
+	errCh := make(chan error, 4)
 
 	wg.Add(1)
 	go func() {
 		var err error
 
 		defer func() {
-			if err != nil {
-				errCh <- err
-			}
+			// NOTE: this might send an already sent error another time, but since we only receive opne, this is fine. On the "happy-path" of this function, the error will be `nil` therefore completing the "err<-errCh" at the bottom of the function.
 			errCh <- err
+			// We must unblock the go routine first as we block on reading the channel later
 			wg.Done()
+
 		}()
 
 		r, err := a.Session.StdoutPipe()
@@ -271,6 +277,10 @@ func (a *Client) CopyFromRemotePassThru(ctx context.Context, w io.Writer, remote
 			errCh <- err
 			return
 		}
+		if res.IsFailure() {
+			errCh <- errors.New(res.GetMessage())
+			return
+		}
 
 		infos, err := res.ParseFileInfos()
 		if err != nil {
@@ -316,9 +326,9 @@ func (a *Client) CopyFromRemotePassThru(ctx context.Context, w io.Writer, remote
 	if err := wait(&wg, ctx); err != nil {
 		return err
 	}
-
+	finalErr := <-errCh
 	close(errCh)
-	return <-errCh
+	return finalErr
 }
 
 func (a *Client) Close() {

configurer.go

@@ -7,11 +7,12 @@
 package scp
 
 import (
-	"golang.org/x/crypto/ssh"
 	"time"
+
+	"golang.org/x/crypto/ssh"
 )
 
-// A struct containing all the configuration options
+// ClientConfigurer a struct containing all the configuration options
 // used by an scp client.
 type ClientConfigurer struct {
 	host         string
@@ -21,7 +22,7 @@ type ClientConfigurer struct {
 	remoteBinary string
 }
 
-// Creates a new client configurer.
+// NewConfigurer creates a new client configurer.
 // It takes the required parameters: the host and the ssh.ClientConfig and
 // returns a configurer populated with the default values for the optional
 // parameters.
@@ -37,39 +38,39 @@ func NewConfigurer(host string, config *ssh.ClientConfig) *ClientConfigurer {
 	}
 }
 
-// Sets the path of the location of the remote scp binary
-// Defaults to: /usr/bin/scp
+// RemoteBinary sets the path of the location of the remote scp binary
+// Defaults to: /usr/bin/scp.
 func (c *ClientConfigurer) RemoteBinary(path string) *ClientConfigurer {
 	c.remoteBinary = path
 	return c
 }
 
-// Alters the host of the client connects to
+// Host alters the host of the client connects to.
 func (c *ClientConfigurer) Host(host string) *ClientConfigurer {
 	c.host = host
 	return c
 }
 
-// Changes the connection timeout.
-// Defaults to one minute
+// Timeout Changes the connection timeout.
+// Defaults to one minute.
 func (c *ClientConfigurer) Timeout(timeout time.Duration) *ClientConfigurer {
 	c.timeout = timeout
 	return c
 }
 
-// Alters the ssh.ClientConfig
+// ClientConfig alters the ssh.ClientConfig.
 func (c *ClientConfigurer) ClientConfig(config *ssh.ClientConfig) *ClientConfigurer {
 	c.clientConfig = config
 	return c
 }
 
-// Alters the ssh.Session
+// Session alters the ssh.Session.
 func (c *ClientConfigurer) Session(session *ssh.Session) *ClientConfigurer {
 	c.session = session
 	return c
 }
 
-// Builds a client with the configuration stored within the ClientConfigurer
+// Create builds a client with the configuration stored within the ClientConfigurer.
 func (c *ClientConfigurer) Create() Client {
 	return Client{
 		Host:         c.host,

protocol.go

@@ -3,6 +3,7 @@
  * terms of the Mozilla Public License 2.0, which is distributed
  * along with the source code.
  */
+
 package scp
 
 import (
@@ -21,8 +22,7 @@ const (
 	Error   ResponseType = 2
 )
 
-const buffSize = 1024 * 256
-
+// Response represent a response from the SCP command.
 // There are tree types of responses that the remote can send back:
 // ok, warning and error
 //
@@ -39,25 +39,25 @@ type Response struct {
 	Message string
 }
 
-// Reads from the given reader (assuming it is the output of the remote) and parses it into a Response structure
+// ParseResponse reads from the given reader (assuming it is the output of the remote) and parses it into a Response structure.
 func ParseResponse(reader io.Reader) (Response, error) {
 	buffer := make([]uint8, 1)
 	_, err := reader.Read(buffer)
 	if err != nil {
 		return Response{}, err
 	}
 
-	response_type := buffer[0]
+	responseType := buffer[0]
 	message := ""
-	if response_type > 0 {
-		buffered_reader := bufio.NewReader(reader)
-		message, err = buffered_reader.ReadString('\n')
+	if responseType > 0 {
+		bufferedReader := bufio.NewReader(reader)
+		message, err = bufferedReader.ReadString('\n')
 		if err != nil {
 			return Response{}, err
 		}
 	}
 
-	return Response{response_type, message}, nil
+	return Response{responseType, message}, nil
 }
 
 func (r *Response) IsOk() bool {
@@ -68,17 +68,17 @@ func (r *Response) IsWarning() bool {
 	return r.Type == Warning
 }
 
-// Returns true when the remote responded with an error
+// IsError returns true when the remote responded with an error.
 func (r *Response) IsError() bool {
 	return r.Type == Error
 }
 
-// Returns true when the remote answered with a warning or an error
+// IsFailure returns true when the remote answered with a warning or an error.
 func (r *Response) IsFailure() bool {
-	return r.Type > 0
+	return r.IsWarning() || r.IsError()
 }
 
-// Returns the message the remote sent back
+// GetMessage returns the message the remote sent back.
 func (r *Response) GetMessage() string {
 	return r.Message
 }
@@ -94,7 +94,7 @@ func (r *Response) ParseFileInfos() (*FileInfos, error) {
 	message := strings.ReplaceAll(r.Message, "\n", "")
 	parts := strings.Split(message, " ")
 	if len(parts) < 3 {
-		return nil, errors.New("Unable to parse message as file infos")
+		return nil, errors.New("unable to parse message as file infos")
 	}
 
 	size, err := strconv.Atoi(parts[1])
@@ -110,16 +110,16 @@ func (r *Response) ParseFileInfos() (*FileInfos, error) {
 	}, nil
 }
 
-// Writes an `Ack` message to the remote, does not await its response, a seperate call to ParseResponse is
-// therefore required to check if the acknowledgement succeeded
+// Ack writes an `Ack` message to the remote, does not await its response, a seperate call to ParseResponse is
+// therefore required to check if the acknowledgement succeeded.
 func Ack(writer io.Writer) error {
 	var msg = []byte{0}
 	n, err := writer.Write(msg)
 	if err != nil {
 		return err
 	}
 	if n < len(msg) {
-		return errors.New("Failed to write ack buffer")
+		return errors.New("failed to write ack buffer")
 	}
 	return nil
 }