Browse code

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 <dug@us.ibm.com>

Doug Davis authored on 2014/12/11 21:56:21
Showing 8 changed files
... ...
@@ -113,7 +113,7 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
113 113
 				return err
114 114
 			}
115 115
 			out := &engine.Env{}
116
-			out.Set("Deleted", img.ID)
116
+			out.SetJson("Deleted", img.ID)
117 117
 			imgs.Add(out)
118 118
 			eng.Job("log", "delete", img.ID, "").Run()
119 119
 			if img.Parent != "" && !noprune {
... ...
@@ -56,7 +56,7 @@ func (daemon *Daemon) CmdInfo(job *engine.Job) engine.Status {
56 56
 		return job.Error(err)
57 57
 	}
58 58
 	v := &engine.Env{}
59
-	v.Set("ID", daemon.ID)
59
+	v.SetJson("ID", daemon.ID)
60 60
 	v.SetInt("Containers", len(daemon.List()))
61 61
 	v.SetInt("Images", imgcount)
62 62
 	v.Set("Driver", daemon.GraphDriver().String())
... ...
@@ -78,7 +78,7 @@ func (daemon *Daemon) CmdInfo(job *engine.Job) engine.Status {
78 78
 	v.SetInt64("MemTotal", meminfo.MemTotal)
79 79
 	v.Set("DockerRootDir", daemon.Config().Root)
80 80
 	if hostname, err := os.Hostname(); err == nil {
81
-		v.Set("Name", hostname)
81
+		v.SetJson("Name", hostname)
82 82
 	}
83 83
 	v.SetList("Labels", daemon.Config().Labels)
84 84
 	if _, err := v.WriteTo(job.Stdout); err != nil {
... ...
@@ -29,18 +29,18 @@ func (daemon *Daemon) ContainerInspect(job *engine.Job) engine.Status {
29 29
 		}
30 30
 
31 31
 		out := &engine.Env{}
32
-		out.Set("Id", container.ID)
32
+		out.SetJson("Id", container.ID)
33 33
 		out.SetAuto("Created", container.Created)
34 34
 		out.SetJson("Path", container.Path)
35 35
 		out.SetList("Args", container.Args)
36 36
 		out.SetJson("Config", container.Config)
37 37
 		out.SetJson("State", container.State)
38
-		out.Set("Image", container.Image)
38
+		out.SetJson("Image", container.Image)
39 39
 		out.SetJson("NetworkSettings", container.NetworkSettings)
40 40
 		out.Set("ResolvConfPath", container.ResolvConfPath)
41 41
 		out.Set("HostnamePath", container.HostnamePath)
42 42
 		out.Set("HostsPath", container.HostsPath)
43
-		out.Set("Name", container.Name)
43
+		out.SetJson("Name", container.Name)
44 44
 		out.SetInt("RestartCount", container.RestartCount)
45 45
 		out.Set("Driver", container.Driver)
46 46
 		out.Set("ExecDriver", container.ExecDriver)
... ...
@@ -114,9 +114,9 @@ func (daemon *Daemon) Containers(job *engine.Job) engine.Status {
114 114
 		}
115 115
 		displayed++
116 116
 		out := &engine.Env{}
117
-		out.Set("Id", container.ID)
117
+		out.SetJson("Id", container.ID)
118 118
 		out.SetList("Names", names[container.ID])
119
-		out.Set("Image", daemon.Repositories().ImageName(container.Image))
119
+		out.SetJson("Image", daemon.Repositories().ImageName(container.Image))
120 120
 		if len(container.Args) > 0 {
121 121
 			args := []string{}
122 122
 			for _, arg := range container.Args {
... ...
@@ -31,7 +31,7 @@ func (s *TagStore) CmdHistory(job *engine.Job) engine.Status {
31 31
 	outs := engine.NewTable("Created", 0)
32 32
 	err = foundImage.WalkHistory(func(img *image.Image) error {
33 33
 		out := &engine.Env{}
34
-		out.Set("Id", img.ID)
34
+		out.SetJson("Id", img.ID)
35 35
 		out.SetInt64("Created", img.Created.Unix())
36 36
 		out.Set("CreatedBy", strings.Join(img.ContainerConfig.Cmd, " "))
37 37
 		out.SetList("Tags", lookupMap[img.ID])
... ...
@@ -62,9 +62,9 @@ func (s *TagStore) CmdImages(job *engine.Job) engine.Status {
62 62
 				delete(allImages, id)
63 63
 				if filt_tagged {
64 64
 					out := &engine.Env{}
65
-					out.Set("ParentId", image.Parent)
65
+					out.SetJson("ParentId", image.Parent)
66 66
 					out.SetList("RepoTags", []string{fmt.Sprintf("%s:%s", name, tag)})
67
-					out.Set("Id", image.ID)
67
+					out.SetJson("Id", image.ID)
68 68
 					out.SetInt64("Created", image.Created.Unix())
69 69
 					out.SetInt64("Size", image.Size)
70 70
 					out.SetInt64("VirtualSize", image.GetParentsSize(0)+image.Size)
... ...
@@ -85,9 +85,9 @@ func (s *TagStore) CmdImages(job *engine.Job) engine.Status {
85 85
 	if job.Getenv("filter") == "" {
86 86
 		for _, image := range allImages {
87 87
 			out := &engine.Env{}
88
-			out.Set("ParentId", image.Parent)
88
+			out.SetJson("ParentId", image.Parent)
89 89
 			out.SetList("RepoTags", []string{"<none>:<none>"})
90
-			out.Set("Id", image.ID)
90
+			out.SetJson("Id", image.ID)
91 91
 			out.SetInt64("Created", image.Created.Unix())
92 92
 			out.SetInt64("Size", image.Size)
93 93
 			out.SetInt64("VirtualSize", image.GetParentsSize(0)+image.Size)
... ...
@@ -109,12 +109,12 @@ func (s *TagStore) CmdGet(job *engine.Job) engine.Status {
109 109
 		//		metaphor, in practice people either ignore it or use it as a
110 110
 		//		generic description field which it isn't. On deprecation shortlist.
111 111
 		res.SetAuto("Created", img.Created)
112
-		res.Set("Author", img.Author)
112
+		res.SetJson("Author", img.Author)
113 113
 		res.Set("Os", img.OS)
114 114
 		res.Set("Architecture", img.Architecture)
115 115
 		res.Set("DockerVersion", img.DockerVersion)
116
-		res.Set("Id", img.ID)
117
-		res.Set("Parent", img.Parent)
116
+		res.SetJson("Id", img.ID)
117
+		res.SetJson("Parent", img.Parent)
118 118
 	}
119 119
 	res.WriteTo(job.Stdout)
120 120
 	return engine.StatusOK
... ...
@@ -137,14 +137,14 @@ func (s *TagStore) CmdLookup(job *engine.Job) engine.Status {
137 137
 		}
138 138
 
139 139
 		out := &engine.Env{}
140
-		out.Set("Id", image.ID)
141
-		out.Set("Parent", image.Parent)
142
-		out.Set("Comment", image.Comment)
140
+		out.SetJson("Id", image.ID)
141
+		out.SetJson("Parent", image.Parent)
142
+		out.SetJson("Comment", image.Comment)
143 143
 		out.SetAuto("Created", image.Created)
144
-		out.Set("Container", image.Container)
144
+		out.SetJson("Container", image.Container)
145 145
 		out.SetJson("ContainerConfig", image.ContainerConfig)
146 146
 		out.Set("DockerVersion", image.DockerVersion)
147
-		out.Set("Author", image.Author)
147
+		out.SetJson("Author", image.Author)
148 148
 		out.SetJson("Config", image.Config)
149 149
 		out.Set("Architecture", image.Architecture)
150 150
 		out.Set("Os", image.OS)
... ...
@@ -2922,13 +2922,35 @@ docker.com>"
2922 2922
 		t.Fatal(err)
2923 2923
 	}
2924 2924
 
2925
-	if res != "Docker IO <io@docker.com>" {
2926
-		t.Fatal("Parsed string did not match the escaped string")
2925
+	if res != "\"Docker IO <io@docker.com>\"" {
2926
+		t.Fatalf("Parsed string did not match the escaped string. Got: %q", res)
2927 2927
 	}
2928 2928
 
2929 2929
 	logDone("build - validate escaping whitespace")
2930 2930
 }
2931 2931
 
2932
+func TestBuildVerifyIntString(t *testing.T) {
2933
+	// Verify that strings that look like ints are still passed as strings
2934
+	name := "testbuildstringing"
2935
+	defer deleteImages(name)
2936
+
2937
+	_, err := buildImage(name, `
2938
+  FROM busybox
2939
+  MAINTAINER 123
2940
+  `, true)
2941
+
2942
+	out, rc, err := runCommandWithOutput(exec.Command(dockerBinary, "inspect", name))
2943
+	if rc != 0 || err != nil {
2944
+		t.Fatalf("Unexcepted error from inspect: rc: %v  err: %v", rc, err)
2945
+	}
2946
+
2947
+	if !strings.Contains(out, "\"123\"") {
2948
+		t.Fatalf("Output does not contain the int as a string:\n%s", out)
2949
+	}
2950
+
2951
+	logDone("build - verify int/strings as strings")
2952
+}
2953
+
2932 2954
 func TestBuildDockerignore(t *testing.T) {
2933 2955
 	name := "testbuilddockerignore"
2934 2956
 	defer deleteImages(name)