Browse code

Fix wrong kill signal parsing

Signed-off-by: Antonio Murdaca <antonio.murdaca@gmail.com>

Antonio Murdaca authored on 2015/06/03 00:00:44
Showing 2 changed files
... ...
@@ -272,14 +272,20 @@ func (s *Server) postContainersKill(version version.Version, w http.ResponseWrit
272 272
 	name := vars["name"]
273 273
 
274 274
 	// If we have a signal, look at it. Otherwise, do nothing
275
-	if sigStr := vars["signal"]; sigStr != "" {
275
+	if sigStr := r.Form.Get("signal"); sigStr != "" {
276 276
 		// Check if we passed the signal as a number:
277 277
 		// The largest legal signal is 31, so let's parse on 5 bits
278
-		sig, err := strconv.ParseUint(sigStr, 10, 5)
278
+		sigN, err := strconv.ParseUint(sigStr, 10, 5)
279 279
 		if err != nil {
280 280
 			// The signal is not a number, treat it as a string (either like
281 281
 			// "KILL" or like "SIGKILL")
282
-			sig = uint64(signal.SignalMap[strings.TrimPrefix(sigStr, "SIG")])
282
+			syscallSig, ok := signal.SignalMap[strings.TrimPrefix(sigStr, "SIG")]
283
+			if !ok {
284
+				return fmt.Errorf("Invalid signal: %s", sigStr)
285
+			}
286
+			sig = uint64(syscallSig)
287
+		} else {
288
+			sig = sigN
283 289
 		}
284 290
 
285 291
 		if sig == 0 {
... ...
@@ -58,3 +58,62 @@ func (s *DockerSuite) TestKillDifferentUserContainer(c *check.C) {
58 58
 		c.Fatal("killed container is still running")
59 59
 	}
60 60
 }
61
+
62
+// regression test about correct signal parsing see #13665
63
+func (s *DockerSuite) TestKillWithSignal(c *check.C) {
64
+	runCmd := exec.Command(dockerBinary, "run", "-d", "busybox", "top")
65
+	out, _, err := runCommandWithOutput(runCmd)
66
+	c.Assert(err, check.IsNil)
67
+
68
+	cid := strings.TrimSpace(out)
69
+	c.Assert(waitRun(cid), check.IsNil)
70
+
71
+	killCmd := exec.Command(dockerBinary, "kill", "-s", "SIGWINCH", cid)
72
+	_, err = runCommand(killCmd)
73
+	c.Assert(err, check.IsNil)
74
+
75
+	running, err := inspectField(cid, "State.Running")
76
+	if running != "true" {
77
+		c.Fatal("Container should be in running state after SIGWINCH")
78
+	}
79
+}
80
+
81
+func (s *DockerSuite) TestKillWithInvalidSignal(c *check.C) {
82
+	runCmd := exec.Command(dockerBinary, "run", "-d", "busybox", "top")
83
+	out, _, err := runCommandWithOutput(runCmd)
84
+	c.Assert(err, check.IsNil)
85
+
86
+	cid := strings.TrimSpace(out)
87
+	c.Assert(waitRun(cid), check.IsNil)
88
+
89
+	killCmd := exec.Command(dockerBinary, "kill", "-s", "0", cid)
90
+	out, _, err = runCommandWithOutput(killCmd)
91
+	c.Assert(err, check.NotNil)
92
+	if !strings.ContainsAny(out, "Invalid signal: 0") {
93
+		c.Fatal("Kill with an invalid signal didn't error out correctly")
94
+	}
95
+
96
+	running, err := inspectField(cid, "State.Running")
97
+	if running != "true" {
98
+		c.Fatal("Container should be in running state after an invalid signal")
99
+	}
100
+
101
+	runCmd = exec.Command(dockerBinary, "run", "-d", "busybox", "top")
102
+	out, _, err = runCommandWithOutput(runCmd)
103
+	c.Assert(err, check.IsNil)
104
+
105
+	cid = strings.TrimSpace(out)
106
+	c.Assert(waitRun(cid), check.IsNil)
107
+
108
+	killCmd = exec.Command(dockerBinary, "kill", "-s", "SIG42", cid)
109
+	out, _, err = runCommandWithOutput(killCmd)
110
+	c.Assert(err, check.NotNil)
111
+	if !strings.ContainsAny(out, "Invalid signal: SIG42") {
112
+		c.Fatal("Kill with an invalid signal error out correctly")
113
+	}
114
+
115
+	running, err = inspectField(cid, "State.Running")
116
+	if running != "true" {
117
+		c.Fatal("Container should be in running state after an invalid signal")
118
+	}
119
+}