Skip to content

Commit

Permalink
interp: fix using Params as an option to New
Browse files Browse the repository at this point in the history
As correctly pointed out by Marcin Bilski, using the exported function
in that way would lead to a panic. And, even after working around the
panic, any shell flags used would be overwritten by the first implicit
call to Reset.

To fix the crash, move the updateExpandOpts method call to the end of
the "set" builtin. After all, it's only needed there; the Run function
already ends up calling the method later via fillExpandConfig.

To keep the shell options from being deleted in Reset, simply whitelist
them from the list of fields to clear. We do that for Params, so it
follows that the same should happen for the opts field.

Finally, add a couple of tests covering both scenarios to
TestRunnerOpts.

Fixes mvdan#365.
  • Loading branch information
mvdan committed Mar 10, 2019
1 parent 55e3a63 commit b3eabde
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
1 change: 1 addition & 0 deletions interp/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
r.errf("set: %v\n", err)
return 2
}
r.updateExpandOpts()
case "shift":
n := 1
switch len(args) {
Expand Down
2 changes: 1 addition & 1 deletion interp/interp.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ func Params(args ...string) func(*Runner) error {
args = args[1:]
}
r.Params = args
r.updateExpandOpts()
return nil
}
}
Expand Down Expand Up @@ -501,6 +500,7 @@ func (r *Runner) Reset() {
Exec: r.Exec,
Open: r.Open,
KillTimeout: r.KillTimeout,
opts: r.opts,

// emptied below, to reuse the space
Vars: r.Vars,
Expand Down
10 changes: 10 additions & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2743,6 +2743,16 @@ func TestRunnerOpts(t *testing.T) {
"[[ $PWD == foo ]]",
"exit status 1",
},
{
opts(Params("foo")),
"echo $@",
"foo\n",
},
{
opts(Params("-u", "--", "foo")),
"echo $@; echo $unset",
"foo\nunset: unbound variable\nexit status 1",
},
}
p := syntax.NewParser()
for i, c := range cases {
Expand Down

0 comments on commit b3eabde

Please sign in to comment.