Replace io.Copy (#2934)

* Replace io.Copy

* Test coverage

* Improve test

* Fix fmt

* Improve error handling in test

* Fix code

* Improve test error log

* Fix fmt

* Fix unix file handles

* Fix error message

* Resolve code climate issue

pkg/command/command.go

@@ -11,6 +11,7 @@ import (
 	"syscall"
 
 	"github.com/SAP/jenkins-library/pkg/log"
+	"github.com/SAP/jenkins-library/pkg/piperutils"
 	"github.com/pkg/errors"
 )
 
@@ -250,12 +251,12 @@ func (c *Command) startCmd(cmd *exec.Cmd) (*execution, error) {
 	}
 
 	go func() {
-		_, execution.errCopyStdout = io.Copy(c.stdout, srcOut)
+		_, execution.errCopyStdout = piperutils.CopyData(c.stdout, srcOut)
 		execution.wg.Done()
 	}()
 
 	go func() {
-		_, execution.errCopyStderr = io.Copy(c.stderr, srcErr)
+		_, execution.errCopyStderr = piperutils.CopyData(c.stderr, srcErr)
 		execution.wg.Done()
 	}()
 

pkg/http/downloader.go

@@ -1,11 +1,13 @@
 package http
 
 import (
-	"github.com/pkg/errors"
-	"io"
 	"net/http"
 	"os"
 	"path/filepath"
+
+	"github.com/pkg/errors"
+
+	"github.com/SAP/jenkins-library/pkg/piperutils"
 )
 
 //Downloader ...
@@ -38,7 +40,7 @@ func (c *Client) DownloadRequest(method, url, filename string, header http.Heade
 	}
 	defer fileHandler.Close()
 
-	_, err = io.Copy(fileHandler, response.Body)
+	_, err = piperutils.CopyData(fileHandler, response.Body)
 	if err != nil {
 		return errors.Wrapf(err, "unable to copy content from url to file %v", filename)
 	}

pkg/http/http.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/SAP/jenkins-library/pkg/log"
+	"github.com/SAP/jenkins-library/pkg/piperutils"
 	"github.com/hashicorp/go-retryablehttp"
 	"github.com/motemen/go-nuts/roundtime"
 	"github.com/pkg/errors"
@@ -146,7 +147,7 @@ func (c *Client) Upload(data UploadRequestData) (*http.Response, error) {
 		return &http.Response{}, errors.Wrapf(err, "error creating form file %v for field %v", data.File, data.FileFieldName)
 	}
 
-	_, err = io.Copy(fileWriter, data.FileContent)
+	_, err = piperutils.CopyData(fileWriter, data.FileContent)
 	if err != nil {
 		return &http.Response{}, errors.Wrapf(err, "unable to copy file content of %v into request body", data.File)
 	}

pkg/piperutils/FileUtils.go

@@ -4,7 +4,6 @@ import (
 	"archive/zip"
 	"errors"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -91,7 +90,7 @@ func (f Files) Copy(src, dst string) (int64, error) {
 		return 0, err
 	}
 	defer func() { _ = destination.Close() }()
-	nBytes, err := io.Copy(destination, source)
+	nBytes, err := CopyData(destination, source)
 	return nBytes, err
 }
 
@@ -170,7 +169,7 @@ func Unzip(src, dest string) ([]string, error) {
 			return filenames, err
 		}
 
-		_, err = io.Copy(outFile, rc)
+		_, err = CopyData(outFile, rc)
 
 		// Close the file without defer to close before next iteration of loop
 		_ = outFile.Close()

pkg/piperutils/ioUtils.go

@@ -0,0 +1,39 @@
+package piperutils
+
+import (
+	"fmt"
+	"io"
+
+	"github.com/pkg/errors"
+)
+
+// CopyData transfers the bytes from src to dst without doing close handling implicitly.
+func CopyData(dst io.Writer, src io.Reader) (int64, error) {
+	tmp := make([]byte, 256)
+	bytesRead := int64(0)
+	bytesWritten := int64(0)
+	done := false
+
+	for {
+		nr, err := src.Read(tmp)
+		bytesRead += int64(nr)
+		if err != nil {
+			if err != io.EOF {
+				return bytesRead, errors.Wrap(err, "read error")
+			}
+			done = true
+		}
+		nw, err := dst.Write(tmp[:nr])
+		bytesWritten += int64(nw)
+		if err != nil {
+			return bytesWritten, errors.Wrap(err, "write error")
+		}
+		if done {
+			break
+		}
+	}
+	if bytesRead != bytesWritten {
+		return bytesRead, errors.New(fmt.Sprintf("transfer error: read %v bytes but wrote %v bytes", bytesRead, bytesWritten))
+	}
+	return bytesWritten, nil
+}

pkg/piperutils/ioUtils_test.go

@@ -0,0 +1,132 @@
+package piperutils
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCopyData(t *testing.T) {
+	runInTempDir(t, "copying file succeeds small", "dir1", func(t *testing.T) {
+		srcName := "testFileSrc"
+		src, err := os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to create src file")
+		}
+		data := make([]byte, 3)
+		data[0] = byte(32)
+		data[1] = byte(42)
+		data[2] = byte(52)
+		_, err = src.Write(data)
+		if err != nil {
+			t.Fatalf("Failed to write data to src file: %v", err)
+		}
+		src.Close()
+		src, err = os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to reopen src file")
+		}
+
+		dstName := "testFileTgt"
+		dst, err := os.OpenFile(dstName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to create dst file")
+		}
+
+		result, err := CopyData(dst, src)
+		src.Close()
+		dst.Close()
+		dst, err = os.OpenFile(dstName, os.O_CREATE|os.O_RDWR, 0700)
+		dataRead := make([]byte, 3)
+		dst.Read(dataRead)
+		dst.Close()
+
+		assert.NoError(t, err, "Didn't expect error but got one")
+		assert.Equal(t, int64(3), result, "Expected true but got false")
+		assert.Equal(t, data, dataRead, "data written %v is different to data read %v")
+	})
+	runInTempDir(t, "copying file succeeds larger", "dir2", func(t *testing.T) {
+		srcName := "testFile"
+		src, err := os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to create src file")
+		}
+		data := make([]byte, 300)
+		for i := 0; i < 300; i++ {
+			data[i] = byte(i)
+		}
+		_, err = src.Write(data)
+		src.Close()
+		src, err = os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+
+		dstName := "testFile2"
+		dst, err := os.OpenFile(dstName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to create dst file")
+		}
+
+		result, err := CopyData(dst, src)
+		src.Close()
+		dst.Close()
+
+		assert.NoError(t, err, "Didn't expect error but got one")
+		assert.Equal(t, int64(300), result, "Expected true but got false")
+	})
+	runInTempDir(t, "copying file fails on read", "dir3", func(t *testing.T) {
+		srcName := "testFileExcl"
+		src, err := os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatalf("Failed to create src file %v", err)
+		}
+		data := make([]byte, 300)
+		for i := 0; i < 300; i++ {
+			data[i] = byte(i)
+		}
+		_, err = src.Write(data)
+		src.Close()
+		src, err = os.OpenFile(srcName, os.O_WRONLY, 0700)
+
+		dstName := "testFile2"
+		dst, err := os.OpenFile(dstName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatal("Failed to create dst file")
+		}
+
+		result, err := CopyData(dst, src)
+		src.Close()
+		dst.Close()
+
+		assert.Error(t, err, "Expected error but got none")
+		assert.Equal(t, int64(0), result, "Expected true but got false")
+	})
+	runInTempDir(t, "copying file fails on write", "dir4", func(t *testing.T) {
+		srcName := "testFileExcl"
+		src, err := os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatalf("Failed to create src file %v", err)
+		}
+		data := make([]byte, 300)
+		for i := 0; i < 300; i++ {
+			data[i] = byte(i)
+		}
+		_, err = src.Write(data)
+		src.Close()
+		src, err = os.OpenFile(srcName, os.O_CREATE|os.O_RDWR, 0700)
+
+		dstName := "testFileExclus"
+		dst, err := os.OpenFile(dstName, os.O_CREATE|os.O_RDWR, 0700)
+		if err != nil {
+			t.Fatalf("Failed to create dst file: %v", err)
+		}
+		dst.Close()
+		dst, err = os.OpenFile(dstName, os.O_RDONLY, 0700)
+
+		result, err := CopyData(dst, src)
+		src.Close()
+		dst.Close()
+
+		assert.Error(t, err, "Expected error but got none")
+		assert.Equal(t, int64(0), result, "Expected true but got false")
+	})
+}