Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compatibility with Docker 25+ #500

Merged
merged 1 commit into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .data/test-oci-docker-image.tar
Binary file not shown.
52 changes: 52 additions & 0 deletions dive/image/docker/image_archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package docker

import (
"archive/tar"
"bytes"
"compress/gzip"
"encoding/json"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -46,6 +48,7 @@ func NewImageArchive(tarFile io.ReadCloser) (*ImageArchive, error) {

// some layer tars can be relative layer symlinks to other layer tars
if header.Typeflag == tar.TypeSymlink || header.Typeflag == tar.TypeReg {
// For the Docker image format, use file name conventions
if strings.HasSuffix(name, ".tar") {
currentLayer++
layerReader := tar.NewReader(tarReader)
Expand Down Expand Up @@ -82,6 +85,55 @@ func NewImageArchive(tarFile io.ReadCloser) (*ImageArchive, error) {
return img, err
}
jsonFiles[name] = fileBuffer
} else if strings.HasPrefix(name, "blobs/") {
// For the OCI-compatible image format (used since Docker 25), use mime sniffing
// but limit this to only the blobs/ (containing the config, and the layers)

// The idea here is that we try various formats in turn, and those tries should
// never consume more bytes than this buffer contains so we can start again.

// 512 bytes ought to be enough (as that's the size of a TAR entry header),
// but play it safe with 1024 bytes. This should also include very small layers
// (unless they've also been gzipped, but Docker does not appear to do it)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a counter-example that's breaking this assumption, here's the layer from an image manifest that I've provided as a repro in #507

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config" { ... },
  "layers": [
    { ... },
    { ... },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",  // <- This layer is gzipped
      "digest": "sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1",
      "size": 32 // <- this layer is also very small
    }
  ]
}

buffer := make([]byte, 1024)
n, err := io.ReadFull(tarReader, buffer)
if err != nil && err != io.ErrUnexpectedEOF {
return img, err
}

// Only try reading a TAR if file is "big enough"
if n == cap(buffer) {
var unwrappedReader io.Reader
unwrappedReader, err = gzip.NewReader(io.MultiReader(bytes.NewReader(buffer[:n]), tarReader))
if err != nil {
// Not a gzipped entry
unwrappedReader = io.MultiReader(bytes.NewReader(buffer[:n]), tarReader)
}

// Try reading a TAR
layerReader := tar.NewReader(unwrappedReader)
tree, err := processLayerTar(name, layerReader)
if err == nil {
currentLayer++
// add the layer to the image
img.layerMap[tree.Name] = tree
continue
}
}

// Not a TAR (or smaller than our buffer), might be a JSON file
decoder := json.NewDecoder(bytes.NewReader(buffer[:n]))
token, err := decoder.Token()
if _, ok := token.(json.Delim); err == nil && ok {
// Looks like a JSON object (or array)
// XXX: should we add a header.Size check too?
fileBuffer, err := io.ReadAll(io.MultiReader(bytes.NewReader(buffer[:n]), tarReader))
if err != nil {
return img, err
}
jsonFiles[name] = fileBuffer
}
// Ignore every other unknown file type
}
}
}
Expand Down
Loading