From d942c59b696d16def85f6b65ae65c176f66a5562 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Thu, 11 Dec 2014 04:56:21 -0800 Subject: [PATCH] Wrap strings that could look like ints in quotes When we use the engine/env object we can run into a situation where a string is passed in as the value but later on when we json serialize the name/value pairs, because the string is made up of just numbers it appears as an integer and not a string - meaning no quotes. This can cause parsing issues for clients. I tried to find all spots where we call env.Set() and the type of the name being set might end up having a value that could look like an int (like author). In those cases I switched it to use env.SetJson() instead because that will wrap it in quotes. One interesting thing to note about the testcase that I modified is that the escaped quotes should have been there all along and we were incorrectly letting it thru. If you look at the metadata stored for that resource you can see the quotes were escaped and we lost them during the serialization steps because of the env.Set() stuff. The use of env is probably not the best way to do all of this. Closes: #9602 Signed-off-by: Doug Davis --- daemon/image_delete.go | 2 +- daemon/info.go | 4 ++-- daemon/inspect.go | 6 +++--- daemon/list.go | 4 ++-- graph/history.go | 2 +- graph/list.go | 8 ++++---- graph/service.go | 16 +++++++-------- integration-cli/docker_cli_build_test.go | 26 ++++++++++++++++++++++-- 8 files changed, 45 insertions(+), 23 deletions(-) diff --git a/daemon/image_delete.go b/daemon/image_delete.go index b0b0c3a023..f39b0dd617 100644 --- a/daemon/image_delete.go +++ b/daemon/image_delete.go @@ -113,7 +113,7 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine. return err } out := &engine.Env{} - out.Set("Deleted", img.ID) + out.SetJson("Deleted", img.ID) imgs.Add(out) eng.Job("log", "delete", img.ID, "").Run() if img.Parent != "" && !noprune { diff --git a/daemon/info.go b/daemon/info.go index 518722b6c2..bf7ec99680 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -56,7 +56,7 @@ func (daemon *Daemon) CmdInfo(job *engine.Job) engine.Status { return job.Error(err) } v := &engine.Env{} - v.Set("ID", daemon.ID) + v.SetJson("ID", daemon.ID) v.SetInt("Containers", len(daemon.List())) v.SetInt("Images", imgcount) v.Set("Driver", daemon.GraphDriver().String()) @@ -78,7 +78,7 @@ func (daemon *Daemon) CmdInfo(job *engine.Job) engine.Status { v.SetInt64("MemTotal", meminfo.MemTotal) v.Set("DockerRootDir", daemon.Config().Root) if hostname, err := os.Hostname(); err == nil { - v.Set("Name", hostname) + v.SetJson("Name", hostname) } v.SetList("Labels", daemon.Config().Labels) if _, err := v.WriteTo(job.Stdout); err != nil { diff --git a/daemon/inspect.go b/daemon/inspect.go index d8397127c0..c930cdd7fc 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -29,18 +29,18 @@ func (daemon *Daemon) ContainerInspect(job *engine.Job) engine.Status { } out := &engine.Env{} - out.Set("Id", container.ID) + out.SetJson("Id", container.ID) out.SetAuto("Created", container.Created) out.SetJson("Path", container.Path) out.SetList("Args", container.Args) out.SetJson("Config", container.Config) out.SetJson("State", container.State) - out.Set("Image", container.Image) + out.SetJson("Image", container.Image) out.SetJson("NetworkSettings", container.NetworkSettings) out.Set("ResolvConfPath", container.ResolvConfPath) out.Set("HostnamePath", container.HostnamePath) out.Set("HostsPath", container.HostsPath) - out.Set("Name", container.Name) + out.SetJson("Name", container.Name) out.SetInt("RestartCount", container.RestartCount) out.Set("Driver", container.Driver) out.Set("ExecDriver", container.ExecDriver) diff --git a/daemon/list.go b/daemon/list.go index 29d7298fc2..188a9861ec 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -114,9 +114,9 @@ func (daemon *Daemon) Containers(job *engine.Job) engine.Status { } displayed++ out := &engine.Env{} - out.Set("Id", container.ID) + out.SetJson("Id", container.ID) out.SetList("Names", names[container.ID]) - out.Set("Image", daemon.Repositories().ImageName(container.Image)) + out.SetJson("Image", daemon.Repositories().ImageName(container.Image)) if len(container.Args) > 0 { args := []string{} for _, arg := range container.Args { diff --git a/graph/history.go b/graph/history.go index 2030c4c789..356340673f 100644 --- a/graph/history.go +++ b/graph/history.go @@ -31,7 +31,7 @@ func (s *TagStore) CmdHistory(job *engine.Job) engine.Status { outs := engine.NewTable("Created", 0) err = foundImage.WalkHistory(func(img *image.Image) error { out := &engine.Env{} - out.Set("Id", img.ID) + out.SetJson("Id", img.ID) out.SetInt64("Created", img.Created.Unix()) out.Set("CreatedBy", strings.Join(img.ContainerConfig.Cmd, " ")) out.SetList("Tags", lookupMap[img.ID]) diff --git a/graph/list.go b/graph/list.go index 0e0e97e447..63a906b9a9 100644 --- a/graph/list.go +++ b/graph/list.go @@ -62,9 +62,9 @@ func (s *TagStore) CmdImages(job *engine.Job) engine.Status { delete(allImages, id) if filt_tagged { out := &engine.Env{} - out.Set("ParentId", image.Parent) + out.SetJson("ParentId", image.Parent) out.SetList("RepoTags", []string{fmt.Sprintf("%s:%s", name, tag)}) - out.Set("Id", image.ID) + out.SetJson("Id", image.ID) out.SetInt64("Created", image.Created.Unix()) out.SetInt64("Size", image.Size) out.SetInt64("VirtualSize", image.GetParentsSize(0)+image.Size) @@ -85,9 +85,9 @@ func (s *TagStore) CmdImages(job *engine.Job) engine.Status { if job.Getenv("filter") == "" { for _, image := range allImages { out := &engine.Env{} - out.Set("ParentId", image.Parent) + out.SetJson("ParentId", image.Parent) out.SetList("RepoTags", []string{":"}) - out.Set("Id", image.ID) + out.SetJson("Id", image.ID) out.SetInt64("Created", image.Created.Unix()) out.SetInt64("Size", image.Size) out.SetInt64("VirtualSize", image.GetParentsSize(0)+image.Size) diff --git a/graph/service.go b/graph/service.go index a27c9a8e38..2858d9b3e6 100644 --- a/graph/service.go +++ b/graph/service.go @@ -109,12 +109,12 @@ func (s *TagStore) CmdGet(job *engine.Job) engine.Status { // metaphor, in practice people either ignore it or use it as a // generic description field which it isn't. On deprecation shortlist. res.SetAuto("Created", img.Created) - res.Set("Author", img.Author) + res.SetJson("Author", img.Author) res.Set("Os", img.OS) res.Set("Architecture", img.Architecture) res.Set("DockerVersion", img.DockerVersion) - res.Set("Id", img.ID) - res.Set("Parent", img.Parent) + res.SetJson("Id", img.ID) + res.SetJson("Parent", img.Parent) } res.WriteTo(job.Stdout) return engine.StatusOK @@ -137,14 +137,14 @@ func (s *TagStore) CmdLookup(job *engine.Job) engine.Status { } out := &engine.Env{} - out.Set("Id", image.ID) - out.Set("Parent", image.Parent) - out.Set("Comment", image.Comment) + out.SetJson("Id", image.ID) + out.SetJson("Parent", image.Parent) + out.SetJson("Comment", image.Comment) out.SetAuto("Created", image.Created) - out.Set("Container", image.Container) + out.SetJson("Container", image.Container) out.SetJson("ContainerConfig", image.ContainerConfig) out.Set("DockerVersion", image.DockerVersion) - out.Set("Author", image.Author) + out.SetJson("Author", image.Author) out.SetJson("Config", image.Config) out.Set("Architecture", image.Architecture) out.Set("Os", image.OS) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 0fd5b1363d..768fb8a153 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2922,13 +2922,35 @@ docker.com>" t.Fatal(err) } - if res != "Docker IO " { - t.Fatal("Parsed string did not match the escaped string") + if res != "\"Docker IO \"" { + t.Fatalf("Parsed string did not match the escaped string. Got: %q", res) } logDone("build - validate escaping whitespace") } +func TestBuildVerifyIntString(t *testing.T) { + // Verify that strings that look like ints are still passed as strings + name := "testbuildstringing" + defer deleteImages(name) + + _, err := buildImage(name, ` + FROM busybox + MAINTAINER 123 + `, true) + + out, rc, err := runCommandWithOutput(exec.Command(dockerBinary, "inspect", name)) + if rc != 0 || err != nil { + t.Fatalf("Unexcepted error from inspect: rc: %v err: %v", rc, err) + } + + if !strings.Contains(out, "\"123\"") { + t.Fatalf("Output does not contain the int as a string:\n%s", out) + } + + logDone("build - verify int/strings as strings") +} + func TestBuildDockerignore(t *testing.T) { name := "testbuilddockerignore" defer deleteImages(name)