Skip to content

Commit

Permalink
Fix panic 'provided file is not a console'
Browse files Browse the repository at this point in the history
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
  • Loading branch information
apostasie committed Sep 22, 2024
1 parent ce15e59 commit c3314d9
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 14 deletions.
10 changes: 1 addition & 9 deletions cmd/nerdctl/container/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ package container

import (
"errors"
"os"

"github.com/moby/term"
"github.com/spf13/cobra"

containerd "github.com/containerd/containerd/v2/client"
Expand Down Expand Up @@ -58,6 +56,7 @@ func NewExecCommand() *cobra.Command {
}

func processExecCommandOptions(cmd *cobra.Command) (types.ContainerExecOptions, error) {
// We do not check if we have a terminal here, as container.Exec calling console.Current will ensure that
globalOptions, err := helpers.ProcessRootCmdFlags(cmd)
if err != nil {
return types.ContainerExecOptions{}, err
Expand Down Expand Up @@ -88,13 +87,6 @@ func processExecCommandOptions(cmd *cobra.Command) (types.ContainerExecOptions,
}
}

_, isTerminal := term.GetFdInfo(os.Stdin)
if !flagD {
if flagT && flagI && !isTerminal {
return types.ContainerExecOptions{}, errors.New("the input device is not a TTY")
}
}

workdir, err := cmd.Flags().GetString("workdir")
if err != nil {
return types.ContainerExecOptions{}, err
Expand Down
5 changes: 4 additions & 1 deletion cmd/nerdctl/container/container_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,10 @@ func runAction(cmd *cobra.Command, args []string) error {

var con console.Console
if createOpt.TTY && !createOpt.Detach {
con = console.Current()
con, err = consoleutil.Current()
if err != nil {
return err
}
defer con.Reset()
if err := con.SetRaw(); err != nil {
return err
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/container/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ func Attach(ctx context.Context, client *containerd.Client, req string, options
con console.Console
)
if spec.Process.Terminal {
con = console.Current()
con, err = consoleutil.Current()
if err != nil {
return err
}
defer con.Reset()
if err := con.SetRaw(); err != nil {
return fmt.Errorf("failed to set the console to raw mode: %w", err)
Expand Down
11 changes: 9 additions & 2 deletions pkg/cmd/container/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ func execActionWithContainer(ctx context.Context, client *containerd.Client, con

var con console.Console
if options.TTY {
con = console.Current()
con, err = consoleutil.Current()
if err != nil {
return err
}
defer con.Reset()
if err := con.SetRaw(); err != nil {
return err
Expand Down Expand Up @@ -164,7 +167,11 @@ func generateExecProcessSpec(ctx context.Context, client *containerd.Client, con
pspec := spec.Process
pspec.Terminal = options.TTY
if pspec.Terminal {
if size, err := console.Current().Size(); err == nil {
con, err := consoleutil.Current()
if err != nil {
return nil, err
}
if size, err := con.Size(); err == nil {
pspec.ConsoleSize = &specs.Box{Height: uint(size.Height), Width: uint(size.Width)}
}
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/consoleutil/consoleutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,22 @@ package consoleutil

import (
"context"
"os"

"github.com/containerd/console"
)

// Current is from https://github.com/containerd/console/blob/v1.0.4/console.go#L68-L81
// adapted so that it does not panic
func Current() (c console.Console, err error) {
for _, s := range []*os.File{os.Stderr, os.Stdout, os.Stdin} {
if c, err = console.ConsoleFromFile(s); err == nil {
return c, nil
}
}
return nil, console.ErrNotAConsole
}

// resizer is from https://github.com/containerd/containerd/blob/v1.7.0-rc.2/cmd/ctr/commands/tasks/tasks.go#L25-L27
type resizer interface {
Resize(ctx context.Context, w, h uint32) error
Expand Down
5 changes: 4 additions & 1 deletion pkg/containerutil/containerutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,10 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie
flagT := process.Process.Terminal
var con console.Console
if flagA && flagT {
con = console.Current()
con, err = consoleutil.Current()
if err != nil {
return err
}
defer con.Reset()
if err := con.SetRaw(); err != nil {
return err
Expand Down

0 comments on commit c3314d9

Please sign in to comment.