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

[20.10 backport] replace json.Unmarshal with NewFromJSON in Create #41976

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

thaJeztah
Copy link
Member

backport of #41701

fixes #41683 /image/store.go Empty configuration causes runtime panic

- What I did

Reuse NewFromJSON to handle no rootfs key situation in image/store.go to solve the issue in #41683

- How I did it

Simply replace the json.Unmarshal function with NewFromJSON in image/image.go

- How to verify it

Running this (mainly modified from #41683) will show invalid image JSON, no RootFS:

package main

import (
    "github.com/docker/docker/image"
    "os"
    "io/ioutil"
    "runtime"
    "github.com/docker/docker/layer"
    "fmt"
)


type mockLayerGetReleaser struct{}

func (ls *mockLayerGetReleaser) Get(layer.ChainID) (layer.Layer, error) {
        return nil, nil
}

func (ls *mockLayerGetReleaser) Release(layer.Layer) ([]layer.Metadata, error) {
        return nil, nil
}

func main(){
    tmpdir, err := ioutil.TempDir("", "images-fs-store")
    defer os.RemoveAll(tmpdir)
    if err != nil {
        fmt.Print(err)
        os.Exit(1)
    }
    fsBackend, err := image.NewFSStoreBackend(tmpdir)
    if err != nil {
        fmt.Print(err)
        os.Exit(1)
    }
    mlgrMap := make(map[string]image.LayerGetReleaser)
    mlgrMap[runtime.GOOS] = &mockLayerGetReleaser{}
    store, err := image.NewImageStore(fsBackend, mlgrMap)
    if err != nil {
        fmt.Print(err)
        os.Exit(1)
    }
    _, err = store.Create([]byte(`{}`))
    if err != nil {
        fmt.Print(err)
        os.Exit(1)
    }
}

- Description for the changelog

Prevent a panic when handling an image json with no `rootfs` key present.

Signed-off-by: Jim Lin <b04705003@ntu.edu.tw>
(cherry picked from commit c9ec21e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

rebased to trigger CI with test-fixes that were merged

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@tiborvass tiborvass merged commit b55d9e1 into moby:20.10 Feb 18, 2021
@thaJeztah thaJeztah deleted the 20.10_backport_reuse branch February 18, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants