From 908db51804635ce002e97e4efb867f7352204f8e Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Fri, 10 Apr 2015 14:23:09 -0400 Subject: [PATCH] Send archive options via pipe in chrootarchive After finding our initial thinking on env. space versus arg list space was wrong, we need to solve this by using a pipe between the caller and child to marshall the (potentially very large) options array to the archiver. Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) --- pkg/chrootarchive/archive.go | 43 ++++++++++++++++++++----------- pkg/chrootarchive/archive_test.go | 37 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/pkg/chrootarchive/archive.go b/pkg/chrootarchive/archive.go index 17d3739d1a..a1454f7b9f 100644 --- a/pkg/chrootarchive/archive.go +++ b/pkg/chrootarchive/archive.go @@ -1,6 +1,7 @@ package chrootarchive import ( + "bytes" "encoding/json" "flag" "fmt" @@ -29,7 +30,8 @@ func untar() { var options *archive.TarOptions - if err := json.Unmarshal([]byte(os.Getenv("OPT")), &options); err != nil { + //read the options from the pipe "ExtraFiles" + if err := json.NewDecoder(os.NewFile(3, "options")).Decode(&options); err != nil { fatal(err) } @@ -62,28 +64,39 @@ func Untar(tarArchive io.Reader, dest string, options *archive.TarOptions) error } } - // We can't pass the exclude list directly via cmd line - // because we easily overrun the shell max argument list length - // when the full image list is passed (e.g. when this is used - // by `docker load`). Instead we will add the JSON marshalled - // and placed in the env, which has significantly larger - // max size - data, err := json.Marshal(options) - if err != nil { - return fmt.Errorf("Untar json encode: %v", err) - } decompressedArchive, err := archive.DecompressStream(tarArchive) if err != nil { return err } defer decompressedArchive.Close() + // We can't pass a potentially large exclude list directly via cmd line + // because we easily overrun the kernel's max argument/environment size + // when the full image list is passed (e.g. when this is used by + // `docker load`). We will marshall the options via a pipe to the + // child + r, w, err := os.Pipe() + if err != nil { + return fmt.Errorf("Untar pipe failure: %v", err) + } cmd := reexec.Command("docker-untar", dest) cmd.Stdin = decompressedArchive - cmd.Env = append(cmd.Env, fmt.Sprintf("OPT=%s", data)) - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("Untar %s %s", err, out) + cmd.ExtraFiles = append(cmd.ExtraFiles, r) + var output bytes.Buffer + cmd.Stdout = &output + cmd.Stderr = &output + + if err := cmd.Start(); err != nil { + return fmt.Errorf("Untar error on re-exec cmd: %v", err) + } + //write the options to the pipe for the untar exec to read + if err := json.NewEncoder(w).Encode(options); err != nil { + return fmt.Errorf("Untar json encode to pipe failed: %v", err) + } + w.Close() + + if err := cmd.Wait(); err != nil { + return fmt.Errorf("Untar re-exec error: %v: output: %s", err, output) } return nil } diff --git a/pkg/chrootarchive/archive_test.go b/pkg/chrootarchive/archive_test.go index 45397d38fe..18f7a93f94 100644 --- a/pkg/chrootarchive/archive_test.go +++ b/pkg/chrootarchive/archive_test.go @@ -8,6 +8,7 @@ import ( "os" "path" "path/filepath" + "strings" "testing" "time" @@ -48,6 +49,42 @@ func TestChrootTarUntar(t *testing.T) { } } +// gh#10426: Verify the fix for having a huge excludes list (like on `docker load` with large # of +// local images) +func TestChrootUntarWithHugeExcludesList(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "docker-TestChrootUntarHugeExcludes") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + src := filepath.Join(tmpdir, "src") + if err := os.MkdirAll(src, 0700); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil { + t.Fatal(err) + } + stream, err := archive.Tar(src, archive.Uncompressed) + if err != nil { + t.Fatal(err) + } + dest := filepath.Join(tmpdir, "dest") + if err := os.MkdirAll(dest, 0700); err != nil { + t.Fatal(err) + } + options := &archive.TarOptions{} + //65534 entries of 64-byte strings ~= 4MB of environment space which should overflow + //on most systems when passed via environment or command line arguments + excludes := make([]string, 65534, 65534) + for i := 0; i < 65534; i++ { + excludes[i] = strings.Repeat(string(i), 64) + } + options.ExcludePatterns = excludes + if err := Untar(stream, dest, options); err != nil { + t.Fatal(err) + } +} + func TestChrootUntarEmptyArchive(t *testing.T) { tmpdir, err := ioutil.TempDir("", "docker-TestChrootUntarEmptyArchive") if err != nil {