kotovalexarian-likes-github
/
moby--moby
mirror of https://github.com/moby/moby.git
1
0
Fork 0
Code Releases Activity

Let client print error when speicify wrong detach keys 

Fix #21064

Let client print error message explicitly when user specifies wrong
detach keys.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
This commit is contained in:
Zhang Wei 2016-03-23 19:34:47 +08:00
parent e1a4332c13
commit 91e5bb9541
10 changed files with 126 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
api/client/attach.go
View File

@ -3,6 +3,7 @@ package client
import (
"fmt"
"io"
"net/http/httputil"
"golang.org/x/net/context"
@ -66,9 +67,12 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
defer signal.StopCatch(sigc)
}
resp, err := cli.client.ContainerAttach(context.Background(), options)
if err != nil {
return err
resp, errAttach := cli.client.ContainerAttach(context.Background(), options)
if errAttach != nil && errAttach != httputil.ErrPersistEOF {
// ContainerAttach returns an ErrPersistEOF (connection closed)
// means server met an error and put it in Hijacked connection
// keep the error and read detailed error message from hijacked connection later
return errAttach
}
defer resp.Close()
@ -90,6 +94,10 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
return err
}
if errAttach != nil {
return errAttach
}
_, status, err := getExitCode(cli, options.ContainerID)
if err != nil {
return err

18
api/client/run.go
View File

@ -3,6 +3,7 @@ package client
import (
"fmt"
"io"
"net/http/httputil"
"os"
"runtime"
"strings"
@ -205,19 +206,26 @@ func (cli *DockerCli) CmdRun(args ...string) error {
DetachKeys: cli.configFile.DetachKeys,
}
resp, err := cli.client.ContainerAttach(context.Background(), options)
if err != nil {
return err
resp, errAttach := cli.client.ContainerAttach(context.Background(), options)
if errAttach != nil && errAttach != httputil.ErrPersistEOF {
// ContainerAttach returns an ErrPersistEOF (connection closed)
// means server met an error and put it in Hijacked connection
// keep the error and read detailed error message from hijacked connection later
return errAttach
}
ctx, cancelFun = context.WithCancel(context.Background())
errCh = promise.Go(func() error {
return cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp)
errHijack := cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp)
if errHijack == nil {
return errAttach
}
return errHijack
})
}
if *flAutoRemove {
defer func() {
if err := cli.removeContainer(createResponse.ID, true, false, false); err != nil {
if err := cli.removeContainer(createResponse.ID, true, false, true); err != nil {
fmt.Fprintf(cli.err, "%v\n", err)
}
}()

16
api/client/start.go
View File

@ -3,6 +3,7 @@ package client
import (
"fmt"
"io"
"net/http/httputil"
"os"
"strings"
@ -94,14 +95,21 @@ func (cli *DockerCli) CmdStart(args ...string) error {
in = cli.in
}
resp, err := cli.client.ContainerAttach(context.Background(), options)
if err != nil {
return err
resp, errAttach := cli.client.ContainerAttach(context.Background(), options)
if errAttach != nil && errAttach != httputil.ErrPersistEOF {
// ContainerAttach return an ErrPersistEOF (connection closed)
// means server met an error and put it in Hijacked connection
// keep the error and read detailed error message from hijacked connection
return errAttach
}
defer resp.Close()
ctx, cancelFun := context.WithCancel(context.Background())
cErr := promise.Go(func() error {
return cli.holdHijackedConnection(ctx, c.Config.Tty, in, cli.out, cli.err, resp)
errHijack := cli.holdHijackedConnection(ctx, c.Config.Tty, in, cli.out, cli.err, resp)
if errHijack == nil {
return errAttach
}
return errHijack
})
// 3. Start the container.

23
api/server/httputils/errors.go
View File

@ -24,11 +24,11 @@ type inputValidationError interface {
IsValidationError() bool
}
// WriteError decodes a specific docker error and sends it in the response.
func WriteError(w http.ResponseWriter, err error) {
if err == nil || w == nil {
logrus.WithFields(logrus.Fields{"error": err, "writer": w}).Error("unexpected HTTP error handling")
return
// GetHTTPErrorStatusCode retrieve status code from error message
func GetHTTPErrorStatusCode(err error) int {
if err == nil {
logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling")
return http.StatusInternalServerError
}
var statusCode int
@ -65,5 +65,16 @@ func WriteError(w http.ResponseWriter, err error) {
statusCode = http.StatusInternalServerError
}
http.Error(w, errMsg, statusCode)
return statusCode
}
// WriteError decodes a specific docker error and sends it in the response.
func WriteError(w http.ResponseWriter, err error) {
if err == nil || w == nil {
logrus.WithFields(logrus.Fields{"error": err, "writer": w}).Error("unexpected HTTP error handling")
return
}
statusCode := GetHTTPErrorStatusCode(err)
http.Error(w, err.Error(), statusCode)
}

35
api/server/router/container/container_routes.go
View File

@ -15,7 +15,6 @@ import (
"github.com/docker/docker/api/types/backend"
"github.com/docker/docker/pkg/ioutils"
"github.com/docker/docker/pkg/signal"
"github.com/docker/docker/pkg/term"
"github.com/docker/engine-api/types"
"github.com/docker/engine-api/types/container"
"github.com/docker/engine-api/types/filters"
@ -408,15 +407,7 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
containerName := vars["name"]
_, upgrade := r.Header["Upgrade"]
keys := []byte{}
detachKeys := r.FormValue("detachKeys")
if detachKeys != "" {
keys, err = term.ToBytes(detachKeys)
if err != nil {
logrus.Warnf("Invalid escape keys provided (%s) using default : ctrl-p ctrl-q", detachKeys)
}
}
hijacker, ok := w.(http.Hijacker)
if !ok {
@ -452,11 +443,24 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
UseStderr: httputils.BoolValue(r, "stderr"),
Logs: httputils.BoolValue(r, "logs"),
Stream: httputils.BoolValue(r, "stream"),
DetachKeys: keys,
DetachKeys: detachKeys,
MuxStreams: true,
}
return s.backend.ContainerAttach(containerName, attachConfig)
if err = s.backend.ContainerAttach(containerName, attachConfig); err != nil {
logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err)
// Remember to close stream if error happens
conn, _, errHijack := hijacker.Hijack()
if errHijack == nil {
statusCode := httputils.GetHTTPErrorStatusCode(err)
statusText := http.StatusText(statusCode)
fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n%s\r\n", statusCode, statusText, err.Error())
httputils.CloseStreams(conn)
} else {
logrus.Errorf("Error Hijacking: %v", err)
}
}
return nil
}
func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
@ -465,15 +469,8 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons
}
containerName := vars["name"]
var keys []byte
var err error
detachKeys := r.FormValue("detachKeys")
if detachKeys != "" {
keys, err = term.ToBytes(detachKeys)
if err != nil {
logrus.Warnf("Invalid escape keys provided (%s) using default : ctrl-p ctrl-q", detachKeys)
}
}
done := make(chan struct{})
started := make(chan struct{})
@ -499,7 +496,7 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons
GetStreams: setupStreams,
Logs: httputils.BoolValue(r, "logs"),
Stream: httputils.BoolValue(r, "stream"),
DetachKeys: keys,
DetachKeys: detachKeys,
UseStdin: true,
UseStdout: true,
UseStderr: true,

2
api/types/backend/backend.go
View File

@ -18,7 +18,7 @@ type ContainerAttachConfig struct {
UseStderr bool
Logs bool
Stream bool
DetachKeys []byte
DetachKeys string
// Used to signify that streams are multiplexed and therefore need a StdWriter to encode stdout/sderr messages accordingly.
// TODO @cpuguy83: This shouldn't be needed. It was only added so that http and websocket endpoints can use the same function, and the websocket function was not using a stdwriter prior to this change...

12
daemon/attach.go
View File

@ -11,10 +11,20 @@ import (
"github.com/docker/docker/daemon/logger"
"github.com/docker/docker/errors"
"github.com/docker/docker/pkg/stdcopy"
"github.com/docker/docker/pkg/term"
)
// ContainerAttach attaches to logs according to the config passed in. See ContainerAttachConfig.
func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerAttachConfig) error {
keys := []byte{}
var err error
if c.DetachKeys != "" {
keys, err = term.ToBytes(c.DetachKeys)
if err != nil {
return fmt.Errorf("Invalid escape keys (%s) provided", c.DetachKeys)
}
}
container, err := daemon.GetContainer(prefixOrName)
if err != nil {
return err
@ -48,7 +58,7 @@ func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerA
stderr = errStream
}
if err := daemon.containerAttach(container, stdin, stdout, stderr, c.Logs, c.Stream, c.DetachKeys); err != nil {
if err := daemon.containerAttach(container, stdin, stdout, stderr, c.Logs, c.Stream, keys); err != nil {
fmt.Fprintf(outStream, "Error attaching: %s\n", err)
}
return nil

3
daemon/exec.go
View File

@ -101,7 +101,8 @@ func (d *Daemon) ContainerExecCreate(config *types.ExecConfig) (string, error) {
if config.DetachKeys != "" {
keys, err = term.ToBytes(config.DetachKeys)
if err != nil {
logrus.Warnf("Wrong escape keys provided (%s, error: %s) using default : ctrl-p ctrl-q", config.DetachKeys, err.Error())
err = fmt.Errorf("Invalid escape keys (%s) provided", config.DetachKeys)
return "", err
}
}

15
integration-cli/docker_api_attach_test.go
View File

@ -67,11 +67,18 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) {
// regression gh14320
func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) {
status, body, err := sockRequest("POST", "/containers/doesnotexist/attach", nil)
c.Assert(status, checker.Equals, http.StatusNotFound)
req, client, err := newRequestClient("POST", "/containers/doesnotexist/attach", nil, "")
c.Assert(err, checker.IsNil)
expected := "No such container: doesnotexist\n"
c.Assert(string(body), checker.Contains, expected)
resp, err := client.Do(req)
// connection will shutdown, err should be "persistent connection closed"
c.Assert(err, checker.NotNil) // Server shutdown connection
body, err := readBody(resp.Body)
c.Assert(err, checker.IsNil)
c.Assert(resp.StatusCode, checker.Equals, http.StatusNotFound)
expected := "No such container: doesnotexist\r\n"
c.Assert(string(body), checker.Equals, expected)
}
func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) {

32
integration-cli/docker_cli_run_unix_test.go
View File

@ -197,6 +197,38 @@ func (s *DockerSuite) TestRunAttachDetachFromFlag(c *check.C) {
c.Assert(running, checker.Equals, "true", check.Commentf("expected container to still be running"))
}
// TestRunDetach checks attaching and detaching with the escape sequence specified via flags.
func (s *DockerSuite) TestRunAttachDetachFromInvalidFlag(c *check.C) {
name := "attach-detach"
dockerCmd(c, "run", "--name", name, "-itd", "busybox", "top")
c.Assert(waitRun(name), check.IsNil)
// specify an invalid detach key, container will ignore it and use default
cmd := exec.Command(dockerBinary, "attach", "--detach-keys='ctrl-A,a'", name)
stdout, err := cmd.StdoutPipe()
if err != nil {
c.Fatal(err)
}
cpty, tty, err := pty.Open()
if err != nil {
c.Fatal(err)
}
defer cpty.Close()
cmd.Stdin = tty
if err := cmd.Start(); err != nil {
c.Fatal(err)
}
bufReader := bufio.NewReader(stdout)
out, err := bufReader.ReadString('\n')
if err != nil {
c.Fatal(err)
}
// it should print a warning to indicate the detach key flag is invalid
errStr := "Invalid escape keys (ctrl-A,a) provided"
c.Assert(strings.TrimSpace(out), checker.Equals, errStr)
}
// TestRunDetach checks attaching and detaching with the escape sequence specified via config file.
func (s *DockerSuite) TestRunAttachDetachFromConfig(c *check.C) {
keyCtrlA := []byte{1}