client: check tty before creating exec job

This is necessary in order to avoid execId leaks in the case where a
`docker exec -it` is run without a terminal available for the client.
You can reproduce this issue by running the following command many
times.

  % nohup docker exec -it some_container true

The container `some_container` will have execIDs that will never
normally be cleaned up (because the client died before they were
started).

In addition, this patch adds a docker-inspect step to ensure that we
give "container does not exist" errors consistently.

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
This commit is contained in:
Aleksa Sarai 2017-05-09 21:07:40 +10:00
parent 3d58c3feac
commit ee7a956c54
No known key found for this signature in database
GPG Key ID: 9E18AA267DDB8DB4
1 changed files with 15 additions and 6 deletions

View File

@ -80,6 +80,19 @@ func runExec(dockerCli *command.DockerCli, opts *execOptions, container string,
ctx := context.Background() ctx := context.Background()
client := dockerCli.Client() client := dockerCli.Client()
// We need to check the tty _before_ we do the ContainerExecCreate, because
// otherwise if we error out we will leak execIDs on the server (and
// there's no easy way to clean those up). But also in order to make "not
// exist" errors take precedence we do a dummy inspect first.
if _, err := client.ContainerInspect(ctx, container); err != nil {
return err
}
if !execConfig.Detach {
if err := dockerCli.In().CheckTty(execConfig.AttachStdin, execConfig.Tty); err != nil {
return err
}
}
response, err := client.ContainerExecCreate(ctx, container, *execConfig) response, err := client.ContainerExecCreate(ctx, container, *execConfig)
if err != nil { if err != nil {
return err return err
@ -91,12 +104,8 @@ func runExec(dockerCli *command.DockerCli, opts *execOptions, container string,
return nil return nil
} }
//Temp struct for execStart so that we don't need to transfer all the execConfig // Temp struct for execStart so that we don't need to transfer all the execConfig.
if !execConfig.Detach { if execConfig.Detach {
if err := dockerCli.In().CheckTty(execConfig.AttachStdin, execConfig.Tty); err != nil {
return err
}
} else {
execStartCheck := types.ExecStartCheck{ execStartCheck := types.ExecStartCheck{
Detach: execConfig.Detach, Detach: execConfig.Detach,
Tty: execConfig.Tty, Tty: execConfig.Tty,