Merge pull request #14322 from runcom/14320-fix-regression-attach-cont-notfound

Return not found before hijacking in attach/wsattach

api/server/server.go

@@ -1116,6 +1116,11 @@ func (s *Server) postContainersAttach(version version.Version, w http.ResponseWr
 		return fmt.Errorf("Missing parameter")
 	}
 
+	cont, err := s.daemon.Get(vars["name"])
+	if err != nil {
+		return err
+	}
+
 	inStream, outStream, err := hijackServer(w)
 	if err != nil {
 		return err
@@ -1138,7 +1143,7 @@ func (s *Server) postContainersAttach(version version.Version, w http.ResponseWr
 		Stream:    boolValue(r, "stream"),
 	}
 
-	if err := s.daemon.ContainerAttachWithLogs(vars["name"], attachWithLogsConfig); err != nil {
+	if err := s.daemon.ContainerAttachWithLogs(cont, attachWithLogsConfig); err != nil {
 		fmt.Fprintf(outStream, "Error attaching: %s\n", err)
 	}
 
@@ -1153,6 +1158,11 @@ func (s *Server) wsContainersAttach(version version.Version, w http.ResponseWrit
 		return fmt.Errorf("Missing parameter")
 	}
 
+	cont, err := s.daemon.Get(vars["name"])
+	if err != nil {
+		return err
+	}
+
 	h := websocket.Handler(func(ws *websocket.Conn) {
 		defer ws.Close()
 
@@ -1164,7 +1174,7 @@ func (s *Server) wsContainersAttach(version version.Version, w http.ResponseWrit
 			Stream:    boolValue(r, "stream"),
 		}
 
-		if err := s.daemon.ContainerWsAttachWithLogs(vars["name"], wsAttachWithLogsConfig); err != nil {
+		if err := s.daemon.ContainerWsAttachWithLogs(cont, wsAttachWithLogsConfig); err != nil {
 			logrus.Errorf("Error attaching websocket: %s", err)
 		}
 	})

daemon/attach.go

@@ -13,12 +13,7 @@ type ContainerAttachWithLogsConfig struct {
 	Logs, Stream                   bool
 }
 
-func (daemon *Daemon) ContainerAttachWithLogs(name string, c *ContainerAttachWithLogsConfig) error {
+func (daemon *Daemon) ContainerAttachWithLogs(container *Container, c *ContainerAttachWithLogsConfig) error {
-	container, err := daemon.Get(name)
-	if err != nil {
-		return err
-	}
-
 	var errStream io.Writer
 
 	if !container.Config.Tty {
@@ -50,11 +45,6 @@ type ContainerWsAttachWithLogsConfig struct {
 	Logs, Stream         bool
 }
 
-func (daemon *Daemon) ContainerWsAttachWithLogs(name string, c *ContainerWsAttachWithLogsConfig) error {
+func (daemon *Daemon) ContainerWsAttachWithLogs(container *Container, c *ContainerWsAttachWithLogsConfig) error {
-	container, err := daemon.Get(name)
-	if err != nil {
-		return err
-	}
-
 	return container.AttachWithLogs(c.InStream, c.OutStream, c.ErrStream, c.Logs, c.Stream)
 }

integration-cli/docker_api_attach_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"net/http"
 	"os/exec"
 	"strings"
 	"time"
@@ -76,3 +77,24 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) {
 		c.Fatal("Expected output on websocket to match input")
 	}
 }
+
+// regression gh14320
+func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) {
+	status, body, err := sockRequest("POST", "/containers/doesnotexist/attach", nil)
+	c.Assert(status, check.Equals, http.StatusNotFound)
+	c.Assert(err, check.IsNil)
+	expected := "no such id: doesnotexist\n"
+	if !strings.Contains(string(body), expected) {
+		c.Fatalf("Expected response body to contain %q", expected)
+	}
+}
+
+func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) {
+	status, body, err := sockRequest("GET", "/containers/doesnotexist/attach/ws", nil)
+	c.Assert(status, check.Equals, http.StatusNotFound)
+	c.Assert(err, check.IsNil)
+	expected := "no such id: doesnotexist\n"
+	if !strings.Contains(string(body), expected) {
+		c.Fatalf("Expected response body to contain %q", expected)
+	}
+}