Merge pull request #135 from erikh/fix-tar-files

image/manifest.go: implement compression detection / mediatype validation

Author: Zhou Hao

Makefile

@@ -60,7 +60,7 @@ lint:
 	@./.tool/lint
 
 test:
-	go test -race -cover $(shell go list ./... | grep -v /vendor/)
+	go test -v -race -cover $(shell go list ./... | grep -v /vendor/)
 
 
 ## this uses https://github.com/Masterminds/glide and https://github.com/sgotti/glide-vc

image/manifest.go

@@ -16,7 +16,9 @@ package image
 
 import (
 	"archive/tar"
+	"bufio"
 	"bytes"
+	"compress/bzip2"
 	"compress/gzip"
 	"encoding/json"
 	"fmt"
@@ -27,6 +29,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/opencontainers/image-spec/schema"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -120,8 +123,8 @@ func (m *manifest) unpack(w walker, dest string) (retErr error) {
 				return nil
 			}
 
-			if err := unpackLayer(dest, r); err != nil {
-				return errors.Wrap(err, "unpack: error extracting layer")
+			if err := unpackLayer(d.MediaType, path, dest, r); err != nil {
+				return errors.Wrap(err, "error unpack: extracting layer")
 			}
 
 			return errEOW
@@ -136,16 +139,72 @@ func (m *manifest) unpack(w walker, dest string) (retErr error) {
 	return nil
 }
 
-func unpackLayer(dest string, r io.Reader) error {
+func getReader(path, mediaType, comp string, buf io.Reader) (io.Reader, error) {
+	switch comp {
+	case "gzip":
+		if !strings.HasSuffix(mediaType, "+gzip") {
+			logrus.Debugf("%q: %s media type with non-%s file", path, comp, comp)
+		}
+
+		return gzip.NewReader(buf)
+	case "bzip2":
+		if !strings.HasSuffix(mediaType, "+bzip2") {
+			logrus.Debugf("%q: %s media type with non-%s file", path, comp, comp)
+		}
+
+		return bzip2.NewReader(buf), nil
+	case "xz":
+		return nil, errors.New("xz layers are not supported")
+	default:
+		if strings.Contains(mediaType, "+") {
+			logrus.Debugf("%q: %s media type with non-%s file", path, comp, comp)
+		}
+
+		return buf, nil
+	}
+}
+
+// DetectCompression detects the compression algorithm of the source.
+func DetectCompression(r *bufio.Reader) (string, error) {
+	source, err := r.Peek(10)
+	if err != nil {
+		return "", err
+	}
+
+	for compression, m := range map[string][]byte{
+		"bzip2": {0x42, 0x5A, 0x68},
+		"gzip":  {0x1F, 0x8B, 0x08},
+		// FIXME needs decompression support
+		// "xz":    {0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00},
+	} {
+		if len(source) < len(m) {
+			logrus.Debug("Len too short")
+			continue
+		}
+		if bytes.Equal(m, source[:len(m)]) {
+			return compression, nil
+		}
+	}
+	return "plain", nil
+}
+
+func unpackLayer(mediaType, path, dest string, r io.Reader) error {
 	entries := make(map[string]bool)
-	gz, err := gzip.NewReader(r)
+
+	buf := bufio.NewReader(r)
+
+	comp, err := DetectCompression(buf)
+	if err != nil {
+		return err
+	}
+
+	reader, err := getReader(path, mediaType, comp, buf)
 	if err != nil {
-		return errors.Wrap(err, "error creating gzip reader")
+		return err
 	}
-	defer gz.Close()
 
 	var dirs []*tar.Header
-	tr := tar.NewReader(gz)
+	tr := tar.NewReader(reader)
 
 loop:
 	for {

image/manifest_test.go

@@ -25,9 +25,17 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/opencontainers/go-digest"
+	"github.com/pkg/errors"
+
+	bz2 "github.com/dsnet/compress/bzip2"
 )
 
+func init() {
+	logrus.SetLevel(logrus.DebugLevel)
+}
+
 func TestUnpackLayerDuplicateEntries(t *testing.T) {
 	tmp1, err := ioutil.TempDir("", "test-dup")
 	if err != nil {
@@ -60,12 +68,12 @@ func TestUnpackLayerDuplicateEntries(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(tmp2)
-	if err := unpackLayer(tmp2, r); err != nil && !strings.Contains(err.Error(), "duplicate entry for") {
+	if err := unpackLayer("application/vnd.oci.image.layer.v1.tar+gzip", f.Name(), tmp2, r); err != nil && !strings.Contains(err.Error(), "duplicate entry for") {
 		t.Fatalf("Expected to fail with duplicate entry, got %v", err)
 	}
 }
 
-func TestUnpackLayer(t *testing.T) {
+func testUnpackLayer(t *testing.T, compression string, invalid bool) {
 	tmp1, err := ioutil.TempDir("", "test-layer")
 	if err != nil {
 		t.Fatal(err)
@@ -81,14 +89,34 @@ func TestUnpackLayer(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	gw := gzip.NewWriter(f)
-	tw := tar.NewWriter(gw)
+	var writer io.WriteCloser = f
+
+	if !invalid {
+		switch compression {
+		case "gzip":
+			writer = gzip.NewWriter(f)
+		case "bzip2":
+			writer, err = bz2.NewWriter(f, nil)
+			if err != nil {
+				t.Fatal(errors.Wrap(err, "compiling bzip compressor"))
+			}
+		}
+	} else if invalid && compression == "" {
+		writer = gzip.NewWriter(f)
+	}
+
+	tw := tar.NewWriter(writer)
+
+	if headerErr := tw.WriteHeader(&tar.Header{Name: "test", Size: 4, Mode: 0600}); headerErr != nil {
+		t.Fatal(headerErr)
+	}
+
+	if _, copyErr := io.Copy(tw, bytes.NewReader([]byte("test"))); copyErr != nil {
+		t.Fatal(copyErr)
+	}
 
-	tw.WriteHeader(&tar.Header{Name: "test", Size: 4, Mode: 0600})
-	io.Copy(tw, bytes.NewReader([]byte("test")))
 	tw.Close()
-	gw.Close()
-	f.Close()
+	writer.Close()
 
 	digester := digest.SHA256.Digester()
 	file, err := os.Open(tarfile)
@@ -100,28 +128,44 @@ func TestUnpackLayer(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = os.Rename(tarfile, filepath.Join(tmp1, "blobs", "sha256", digester.Digest().Hex()))
-	if err != nil {
-		t.Fatal(err)
+
+	blobPath := filepath.Join(tmp1, "blobs", "sha256", digester.Digest().Hex())
+
+	if renameErr := os.Rename(tarfile, blobPath); renameErr != nil {
+		t.Fatal(errors.Wrap(renameErr, blobPath))
+	}
+
+	mediatype := "application/vnd.oci.image.layer.v1.tar"
+	if compression != "" {
+		mediatype += "+" + compression
 	}
 
 	testManifest := manifest{
 		Layers: []descriptor{descriptor{
-			MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
+			MediaType: mediatype,
 			Digest:    digester.Digest().String(),
 		}},
 	}
 	err = testManifest.unpack(newPathWalker(tmp1), filepath.Join(tmp1, "rootfs"))
 	if err != nil {
-		t.Fatal(err)
+		t.Fatal(errors.Wrapf(err, "%q / %s", blobPath, compression))
 	}
 
 	_, err = os.Stat(filepath.Join(tmp1, "rootfs", "test"))
 	if err != nil {
-		t.Fatal(err)
+		t.Fatal(errors.Wrapf(err, "%q / %s", blobPath, compression))
 	}
 }
 
+func TestUnpackLayer(t *testing.T) {
+	testUnpackLayer(t, "gzip", true)
+	testUnpackLayer(t, "gzip", false)
+	testUnpackLayer(t, "", true)
+	testUnpackLayer(t, "", false)
+	testUnpackLayer(t, "bzip2", true)
+	testUnpackLayer(t, "bzip2", false)
+}
+
 func TestUnpackLayerRemovePartialyUnpackedFile(t *testing.T) {
 	// generate a tar file has duplicate entry which will failed on unpacking
 	tmp1, err := ioutil.TempDir("", "test-layer")