diff --git a/docker/tarfile/src.go b/docker/tarfile/src.go index 2a0d9966e..b3f0012c2 100644 --- a/docker/tarfile/src.go +++ b/docker/tarfile/src.go @@ -3,7 +3,6 @@ package tarfile import ( "archive/tar" "bytes" - "compress/gzip" "context" "encoding/json" "io" @@ -43,8 +42,7 @@ type layerInfo struct { // the source of an image. // To do for both the NewSourceFromFile and NewSourceFromStream functions -// NewSourceFromFile returns a tarfile.Source for the specified path -// NewSourceFromFile supports both conpressed and uncompressed input +// NewSourceFromFile returns a tarfile.Source for the specified path. func NewSourceFromFile(path string) (*Source, error) { file, err := os.Open(path) if err != nil { @@ -52,19 +50,23 @@ func NewSourceFromFile(path string) (*Source, error) { } defer file.Close() - reader, err := gzip.NewReader(file) + // If the file is already not compressed we can just return the file itself + // as a source. Otherwise we pass the stream to NewSourceFromStream. + stream, isCompressed, err := compression.AutoDecompress(file) if err != nil { + return nil, errors.Wrapf(err, "detecting compression for file %q", path) + } + if !isCompressed { return &Source{ tarPath: path, }, nil } - defer reader.Close() - - return NewSourceFromStream(reader) + return NewSourceFromStream(stream) } -// NewSourceFromStream returns a tarfile.Source for the specified inputStream, which must be uncompressed. -// The caller can close the inputStream immediately after NewSourceFromFile returns. +// NewSourceFromStream returns a tarfile.Source for the specified inputStream, +// which can be either compressed or uncompressed. The caller can close the +// inputStream immediately after NewSourceFromFile returns. func NewSourceFromStream(inputStream io.Reader) (*Source, error) { // FIXME: use SystemContext here. // Save inputStream to a temporary file @@ -81,7 +83,18 @@ func NewSourceFromStream(inputStream io.Reader) (*Source, error) { } }() - // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. + // In order to be compatible with docker-load, we need to support + // auto-decompression (it's also a nice quality-of-life thing to avoid + // giving users really confusing "invalid tar header" errors). + inputStream, _, err = compression.AutoDecompress(inputStream) + if err != nil { + return nil, errors.Wrap(err, "auto-decompress docker-archive source") + } + + // Copy the plain archive to the temporary file. + // + // TODO: This can take quite some time, and should ideally be cancellable + // using a context.Context. if _, err := io.Copy(tarCopyFile, inputStream); err != nil { return nil, errors.Wrapf(err, "error copying contents to temporary file %q", tarCopyFile.Name()) } @@ -292,7 +305,23 @@ func (s *Source) prepareLayerData(tarManifest *ManifestItem, parsedConfig *manif return nil, err } if li, ok := unknownLayerSizes[h.Name]; ok { - li.size = h.Size + // Since GetBlob will decompress layers that are compressed we need + // to do the decompression here as well, otherwise we will + // incorrectly report the size. Pretty critical, since tools like + // umoci always compress layer blobs. Obviously we only bother with + // the slower method of checking if it's compressed. + stream, isCompressed, err := compression.AutoDecompress(t) + if err != nil { + return nil, errors.Wrapf(err, "auto-decompress %s to find size", h.Name) + } + uncompressedSize := h.Size + if isCompressed { + uncompressedSize, err = io.Copy(ioutil.Discard, stream) + if err != nil { + return nil, errors.Wrapf(err, "reading %s to find size", h.Name) + } + } + li.size = uncompressedSize delete(unknownLayerSizes, h.Name) } } @@ -386,16 +415,9 @@ func (s *Source) GetBlob(ctx context.Context, info types.BlobInfo) (io.ReadClose // be verifing a "digest" which is not the actual layer's digest (but // is instead the DiffID). - decompressFunc, reader, err := compression.DetectCompression(stream) + reader, _, err := compression.AutoDecompress(stream) if err != nil { - return nil, 0, errors.Wrapf(err, "Detecting compression in blob %s", info.Digest) - } - - if decompressFunc != nil { - reader, err = decompressFunc(reader) - if err != nil { - return nil, 0, errors.Wrapf(err, "Decompressing blob %s stream", info.Digest) - } + return nil, 0, errors.Wrapf(err, "auto-decompress blob %s", info.Digest) } newStream := readCloseWrapper{ diff --git a/pkg/compression/compression.go b/pkg/compression/compression.go index c19d962ee..f94255c53 100644 --- a/pkg/compression/compression.go +++ b/pkg/compression/compression.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + "github.com/ulikunitz/xz" ) // DecompressorFunc returns the decompressed stream, given a compressed stream. @@ -26,7 +27,7 @@ func Bzip2Decompressor(r io.Reader) (io.Reader, error) { // XzDecompressor is a DecompressorFunc for the xz compression algorithm. func XzDecompressor(r io.Reader) (io.Reader, error) { - return nil, errors.New("Decompressing xz streams is not supported") + return xz.NewReader(r) } // compressionAlgos is an internal implementation detail of DetectCompression @@ -65,3 +66,20 @@ func DetectCompression(input io.Reader) (DecompressorFunc, io.Reader, error) { return decompressor, io.MultiReader(bytes.NewReader(buffer[:n]), input), nil } + +// AutoDecompress takes a stream and returns an uncompressed version of the +// same stream. It's a simple wrapper around DetectCompression that removes the +// need to touch DecompressorFuncs. +func AutoDecompress(stream io.Reader) (io.Reader, bool, error) { + decompressFunc, stream, err := DetectCompression(stream) + if err != nil { + return nil, false, errors.Wrapf(err, "detect compression") + } + if decompressFunc != nil { + stream, err = decompressFunc(stream) + if err != nil { + return nil, false, errors.Wrapf(err, "auto decompression") + } + } + return stream, decompressFunc != nil, nil +} diff --git a/pkg/compression/compression_test.go b/pkg/compression/compression_test.go index 2dd429317..ab5c67b61 100644 --- a/pkg/compression/compression_test.go +++ b/pkg/compression/compression_test.go @@ -15,13 +15,12 @@ import ( func TestDetectCompression(t *testing.T) { cases := []struct { - filename string - unimplemented bool + filename string }{ - {"fixtures/Hello.uncompressed", false}, - {"fixtures/Hello.gz", false}, - {"fixtures/Hello.bz2", false}, - {"fixtures/Hello.xz", true}, + {"fixtures/Hello.uncompressed"}, + {"fixtures/Hello.gz"}, + {"fixtures/Hello.bz2"}, + {"fixtures/Hello.xz"}, } // The original stream is preserved. @@ -54,10 +53,6 @@ func TestDetectCompression(t *testing.T) { switch { case decompressor == nil: uncompressedStream = updatedStream - case c.unimplemented: - _, err := decompressor(updatedStream) - assert.Error(t, err) - continue default: s, err := decompressor(updatedStream) require.NoError(t, err) diff --git a/vendor.conf b/vendor.conf index 123bde98a..fd3e8b6ce 100644 --- a/vendor.conf +++ b/vendor.conf @@ -40,3 +40,4 @@ github.com/ostreedev/ostree-go aeb02c6b6aa2889db3ef62f7855650755befd460 github.com/gogo/protobuf fcdc5011193ff531a548e9b0301828d5a5b97fd8 github.com/pquerna/ffjson master github.com/syndtr/gocapability master +github.com/ulikunitz/xz v0.5.4