Repository: mvdan/sh
Pull request #497 (2020-02-07 16:21:13)

Set runner exitShell to true before calling the exec handler for exec builtin
Set runner exitShell to true before calling the exec handler for exec builtin, this way exec handlers could retrieve this information via runner.Exited() and take appropriate actions.

Not sure if this is the best way to handle that but in a first time it would allow to retrieve this information without dealing with statements and parsing to know if it's an exec builtin or a sub-process.

mvdan: I'm not sure I follow - could you provide a real example? The `ExecHandlerFunc` interface only has a context with some data, and that doesn't include the `*Runner`, so I'm not sure how you'd call `Runner.Exited`.
mvdan: Having said the above, I'm not opposed to the change. I'd like a test to go along with the change, though, as otherwise it might break again in a future refactor.
cclerget: By embedding runner in the context :

```
package main

import (
    "context"
    "os"
    "log"
    "fmt"

    "mvdan.cc/sh/v3/interp"
    "mvdan.cc/sh/v3/syntax"
)

var runnerCtx = struct{}{}

var DefaultExec = interp.DefaultExecHandler(2)

func ExecHandler() interp.ExecHandlerFunc {
    return func(ctx context.Context, args []string) error {
        runner := ctx.Value(&runnerCtx).(*interp.Runner)
        if runner.Exited() {
            fmt.Println("exec builtin")
        }
        return DefaultExec(ctx, args)
    }
}

func main() {
    script := "/tmp/test.sh"

    f, err := os.Open(script)
    if err != nil {
        log.Fatal(err)
    }
    defer f.Close()

    runner, _ := interp.New(
        interp.StdIO(os.Stdin, os.Stdout, os.Stderr),
        interp.ExecHandler(ExecHandler()),
    )
    parser := syntax.NewParser()

    ctx := context.WithValue(context.Background(), &runnerCtx, runner)

    sf := func(st *syntax.Stmt) bool {
        runner.Run(ctx, st)
        return true
    }

    parser.Stmts(f, sf)
}
```

> Having said the above, I'm not opposed to the change. I'd like a test to go along with the change, though, as otherwise it might break again in a future refactor.

Sure
mvdan: Thanks - your use case seems fine :) Adding that as a standalone test shouldn't be too hard.
cclerget: @mvdan Thanks ! Test added
cclerget: Should be good to go and commit squashed

Patch:
diff --git a/interp/builtin.go b/interp/builtin.go
index 3da16fe3..854e4999 100644
--- a/interp/builtin.go
+++ b/interp/builtin.go
@@ -305,8 +305,8 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
 			r.keepRedirs = true
 			break
 		}
-		r.exec(ctx, args)
 		r.exitShell = true
+		r.exec(ctx, args)
 		return r.exit
 	case "command":
 		show := false


Test patch:
diff --git a/interp/handler_test.go b/interp/handler_test.go
index 8a81ec47..f5780e76 100644
--- a/interp/handler_test.go
+++ b/interp/handler_test.go
@@ -38,6 +38,17 @@ func blacklistNondevOpen(ctx context.Context, path string, flags int, mode os.Fi
 	return testOpenHandler(ctx, path, flags, mode)
 }
 
+// runnerCtx allows us to give handler functions access to the Runner, if needed.
+var runnerCtx = new(int)
+
+func execBuiltin(ctx context.Context, args []string) error {
+	runner, ok := ctx.Value(runnerCtx).(*Runner)
+	if ok && runner.Exited() {
+		return fmt.Errorf("exec builtin: %s", args[0])
+	}
+	return nil
+}
+
 var modCases = []struct {
 	name string
 	exec ExecHandlerFunc
@@ -81,6 +92,12 @@ var modCases = []struct {
 		src:  "{ malicious; true; } & { malicious; true; } & wait",
 		want: "blacklisted: malicious",
 	},
+	{
+		name: "ExecBuiltin",
+		exec: execBuiltin,
+		src:  "exec /bin/sh",
+		want: "exec builtin: /bin/sh",
+	},
 	{
 		name: "OpenForbidNonDev",
 		open: blacklistNondevOpen,
@@ -106,7 +123,7 @@ func TestRunnerHandlers(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			ctx := context.Background()
+			ctx := context.WithValue(context.Background(), runnerCtx, r)
 			if err := r.Run(ctx, file); err != nil {
 				cb.WriteString(err.Error())
 			}