Skip to content

Commit

Permalink
Fix nil deref if the env file can't be parsed (#625)
Browse files Browse the repository at this point in the history
Closes #605.
  • Loading branch information
josh-berry authored Jul 16, 2024
1 parent 358f265 commit 8d56e1b
Showing 1 changed file with 42 additions and 20 deletions.
62 changes: 42 additions & 20 deletions temporalcli/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,17 @@ type CommandOptions struct {
AdditionalClientGRPCDialOptions []grpc.DialOption
}

// NewCommandContext creates a CommandContext for use by the rest of the CLI.
// Among other things, this parses the env config file and modifies
// options/flags according to the parameters set there.
//
// A CommandContext and CancelFunc are always returned, even in the event of an
// error; this is so the CommandContext can be used to print an appropriate
// error message.
func NewCommandContext(ctx context.Context, options CommandOptions) (*CommandContext, context.CancelFunc, error) {
cctx := &CommandContext{Context: ctx, Options: options}
if err := cctx.preprocessOptions(); err != nil {
return nil, nil, err
return cctx, func(){}, err
}

// Setup interrupt handler
Expand Down Expand Up @@ -109,6 +116,28 @@ func (c *CommandContext) preprocessOptions() error {
c.Options.Stderr = os.Stderr
}

// Setup default fail callback
if c.Options.Fail == nil {
c.Options.Fail = func(err error) {
// If context is closed, say that the program was interrupted and ignore
// the actual error
if c.Err() != nil {
err = fmt.Errorf("program interrupted")
}
if c.Logger != nil {
c.Logger.Error(err.Error())
} else {
fmt.Fprintln(os.Stderr, err)
}
os.Exit(1)
}
}

// Update options according to the env file. MUST BE DONE LAST.
//
// Why last? Callers need the CommandContext to be usable no matter what,
// because they rely on it to print errors even if env parsing fails. In
// that situation, we will return both the CommandContext AND an error.
if !c.Options.DisableEnvConfig {
if c.Options.EnvConfigFile == "" {
// Default to --env-file, prefetched from CLI args
Expand Down Expand Up @@ -145,22 +174,6 @@ func (c *CommandContext) preprocessOptions() error {
}
}

// Setup default fail callback
if c.Options.Fail == nil {
c.Options.Fail = func(err error) {
// If context is closed, say that the program was interrupted and ignore
// the actual error
if c.Err() != nil {
err = fmt.Errorf("program interrupted")
}
if c.Logger != nil {
c.Logger.Error(err.Error())
} else {
fmt.Fprintln(os.Stderr, err)
}
os.Exit(1)
}
}
return nil
}

Expand Down Expand Up @@ -314,17 +327,26 @@ func (c *CommandContext) promptString(message string, expected string, autoConfi
// intentionally does not return an error but rather invokes Fail on the
// options.
func Execute(ctx context.Context, options CommandOptions) {
// Create context and run
// Create context and run. We always get a context and cancel func back even
// if an error was returned. This is so we can use the context to print an
// error message using the appropriate Fail() method, regardless of why the
// failure occurred.
//
// (In most cases, an error here likely means a problem with the user's env
// config file, or some other issue in their environment.)
cctx, cancel, err := NewCommandContext(ctx, options)
defer cancel()

if err == nil {
defer cancel()
// We have a context; let's actually run the command.
cmd := NewTemporalCommand(cctx)
cmd.Command.SetArgs(cctx.Options.Args)
err = cmd.Command.ExecuteContext(cctx)
}

// Use failure handler, but can still return
if err != nil {
// Either we failed to create the context, OR the command itself failed.
// Either way, we need to print an error message.
cctx.Options.Fail(err)
}

Expand Down

0 comments on commit 8d56e1b

Please sign in to comment.