Fix: Resources are not closed when reading tar from dir

Signed-off-by: Javier Romero <jromero@pivotal.io>

blob/blob.go

@@ -25,9 +25,7 @@ func NewBlob(path string) Blob {
 }
 
 // Open returns an io.ReadCloser whose contents are in tar archive format
-func (b blob) Open() (io.ReadCloser, error) {
-	var err error
-
+func (b blob) Open() (r io.ReadCloser, err error) {
 	fi, err := os.Stat(b.path)
 	if err != nil {
 		return nil, errors.Wrapf(err, "read blob at path '%s'", b.path)
@@ -41,7 +39,7 @@ func (b blob) Open() (io.ReadCloser, error) {
 		return nil, errors.Wrap(err, "open buildpack archive")
 	}
 	defer func() {
-		if err != nil && fh != nil {
+		if err != nil {
 			fh.Close()
 		}
 	}()

builder/lifecycle.go

@@ -7,12 +7,11 @@ import (
 	"path"
 	"regexp"
 
-	"github.com/buildpack/pack/api"
-
 	"github.com/BurntSushi/toml"
 	"github.com/Masterminds/semver"
 	"github.com/pkg/errors"
 
+	"github.com/buildpack/pack/api"
 	"github.com/buildpack/pack/internal/archive"
 )
 
@@ -87,6 +86,8 @@ type lifecycle struct {
 }
 
 func NewLifecycle(blob Blob) (Lifecycle, error) {
+	var err error
+
 	br, err := blob.Open()
 	if err != nil {
 		return nil, errors.Wrap(err, "open lifecycle blob")
@@ -96,7 +97,7 @@ func NewLifecycle(blob Blob) (Lifecycle, error) {
 	var descriptor LifecycleDescriptor
 	_, buf, err := archive.ReadTarEntry(br, "lifecycle.toml")
 
-	// TODO: make lifecycle descriptor required after v0.4.0 release [https://github.com/buildpack/pack/issues/267]
+	//TODO: make lifecycle descriptor required after v0.4.0 release [https://github.com/buildpack/pack/issues/267]
 	if err != nil && errors.Cause(err) == archive.ErrEntryNotExist {
 		return &lifecycle{
 			Blob:       blob,

internal/archive/archive.go

@@ -9,9 +9,9 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 
+	"github.com/docker/docker/pkg/ioutils"
 	"github.com/pkg/errors"
 )
 
@@ -30,24 +30,56 @@ func ReadZipAsTar(srcPath, basePath string, uid, gid int, mode int64) io.ReadClo
 }
 
 func readAsTar(src, basePath string, uid, gid int, mode int64, writeFn func(tw *tar.Writer, srcDir, basePath string, uid, gid int, mode int64) error) io.ReadCloser {
-	r, w := io.Pipe()
-	go func() {
-		var err error
-		defer func() {
-			w.CloseWithError(err)
-		}()
+	var (
+		errChan = make(chan error)
+		r, w    = io.Pipe()
+	)
 
+	go func() {
 		tw := tar.NewWriter(w)
 		defer func() {
-			// only close if no errors have occurred
-			if err == nil {
+			if r := recover(); r != nil {
 				tw.Close()
+				w.CloseWithError(errors.Errorf("panic: %v", r))
 			}
 		}()
 
-		err = writeFn(tw, src, basePath, uid, gid, mode)
+		err := writeFn(tw, src, basePath, uid, gid, mode)
+
+		closeErr := tw.Close()
+		closeErr = aggregateError(closeErr, w.CloseWithError(err))
+
+		errChan <- closeErr
 	}()
-	return r
+
+	return ioutils.NewReadCloserWrapper(r, func() error {
+		var compErr error
+
+		// closing the reader ensures that if anything attempts
+		// further reading it doesn't block waiting for content
+		if err := r.Close(); err != nil {
+			compErr = aggregateError(compErr, err)
+		}
+
+		// wait until everything closes properly specially contents inside `writeFn`
+		if err := <-errChan; err != nil {
+			compErr = aggregateError(compErr, err)
+		}
+
+		return compErr
+	})
+}
+
+func aggregateError(base, addition error) error {
+	if addition == nil {
+		return base
+	}
+
+	if base == nil {
+		return addition
+	}
+
+	return errors.Wrap(addition, base.Error())
 }
 
 func CreateSingleFileTarReader(path, txt string) (io.Reader, error) {
@@ -111,7 +143,7 @@ func ReadTarEntry(rc io.Reader, entryPath string) (*tar.Header, []byte, error) {
 			return nil, nil, errors.Wrap(err, "failed to get next tar entry")
 		}
 
-		if strings.Contains(header.Name, entryPath) {
+		if header.Name == entryPath {
 			buf, err := ioutil.ReadAll(tr)
 			if err != nil {
 				return nil, nil, errors.Wrapf(err, "failed to read contents of '%s'", entryPath)