Skip to content

Commit

Permalink
interp: implement here-documents via os.Pipe
Browse files Browse the repository at this point in the history
os/exec.Cmd.Stdin's documentation says:

    If Stdin is an *os.File, the process's standard input is connected
    directly to that file.

    Otherwise, during the execution of the command a separate
    goroutine reads from Stdin and delivers that data to the command
    over a pipe. In this case, Wait does not complete until the goroutine
    stops copying, either because it has reached the end of Stdin
    (EOF or a read error), or because writing to the pipe returned an error,
    or because a nonzero WaitDelay was set and expired.

Hence, with the command

		while read a; do
        echo $a
        ls
    done <<EOF
    foo
    bar
    baz
    EOF

the first iteration would read the "foo" line correctly,
but it would execute "ls" and let it consume the rest of the heredoc
as we currently implement heredocs via bytes.Buffer and strings.Reader,
causing os/exec to create a pipe and copy all contents over.

Since we don't want external commands to consume stdin entirely,
we must feed os/exec.Cmd.Stdin with an os.File.
We don't want to be leaving temporary files around, so an os.Pipe works;
we just need to spawn goroutines to avoid hangs when buffers get filled.

Note that this likely happens when giving a buffer io.Reader as stdin
via the Go API as well, hence the added TODO.

Fixes #1085.
  • Loading branch information
mvdan committed Aug 8, 2024
1 parent 4a8ae22 commit 980b6fc
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 31 deletions.
4 changes: 4 additions & 0 deletions interp/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ type Runner struct {
// statHandler is a function responsible for getting file stat. It must be non-nil.
statHandler StatHandlerFunc

// TODO: we should force stdin to always be an *os.File,
// otherwise the first os/exec command we execute with stdin
// will always consume the entirety of the contents.

stdin io.Reader
stdout io.Writer
stderr io.Writer
Expand Down
19 changes: 19 additions & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ var hasBash52 bool
func TestMain(m *testing.M) {
if os.Getenv("GOSH_PROG") != "" {
switch os.Getenv("GOSH_CMD") {
case "exec_ok":
fmt.Printf("exec ok\n")
os.Exit(0)
case "exec_fail":
fmt.Printf("exec fail\n")
os.Exit(1)
case "pid_and_hang":
fmt.Println(os.Getpid())
time.Sleep(time.Hour)
Expand Down Expand Up @@ -2890,6 +2896,19 @@ done <<< 2`,
"echo line\\\ncontinuation | while read a; do echo $a; done",
"linecontinuation\n",
},
{
"while read a; do echo $a; GOSH_CMD=exec_ok $GOSH_PROG; done <<< 'a\nb\nc'",
"a\nexec ok\nb\nexec ok\nc\nexec ok\n",
},
{
"while read a; do echo $a; GOSH_CMD=exec_ok $GOSH_PROG; done <<EOF\na\nb\nc\nEOF",
"a\nexec ok\nb\nexec ok\nc\nexec ok\n",
},
// TODO: our final exit status here isn't right.
// {
// "while read a; do echo $a; GOSH_CMD=exec_fail $GOSH_PROG; done <<< 'a\nb\nc'",
// "a\nexec fail\nb\nexec fail\nc\nexec fail\nexit status 1",
// },
{
`read -r a <<< '\\'; echo "$a"`,
"\\\\\n",
Expand Down
90 changes: 59 additions & 31 deletions interp/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package interp

import (
"bytes"
"context"
"errors"
"fmt"
Expand Down Expand Up @@ -797,43 +796,61 @@ func (r *Runner) stmts(ctx context.Context, stmts []*syntax.Stmt) {
}
}

func (r *Runner) hdocReader(rd *syntax.Redirect) io.Reader {
if rd.Op != syntax.DashHdoc {
hdoc := r.document(rd.Hdoc)
return strings.NewReader(hdoc)
func (r *Runner) hdocReader(rd *syntax.Redirect) (io.ReadCloser, error) {
pr, pw, err := os.Pipe()
if err != nil {
return nil, err
}
var buf bytes.Buffer
var cur []syntax.WordPart
flushLine := func() {
if buf.Len() > 0 {
buf.WriteByte('\n')
// We write to the pipe in a new goroutine,
// as pipe writes may block once the buffer gets full.
// TODO: r.document calls below buffer into a string;
// it would be nice to have them write to the pipe directly.
go func() {
if rd.Op != syntax.DashHdoc {
hdoc := r.document(rd.Hdoc)
pw.WriteString(hdoc)
pw.Close()
return
}
buf.WriteString(r.document(&syntax.Word{Parts: cur}))
cur = cur[:0]
}
for _, wp := range rd.Hdoc.Parts {
lit, ok := wp.(*syntax.Lit)
if !ok {
cur = append(cur, wp)
continue
var cur []syntax.WordPart
firstLine := true
flushLine := func() {
if !firstLine {
pw.WriteString("\n")
}
firstLine = false
pw.WriteString(r.document(&syntax.Word{Parts: cur}))
cur = cur[:0]
}
for i, part := range strings.Split(lit.Value, "\n") {
if i > 0 {
flushLine()
cur = cur[:0]
for _, wp := range rd.Hdoc.Parts {
lit, ok := wp.(*syntax.Lit)
if !ok {
cur = append(cur, wp)
continue
}
for i, part := range strings.Split(lit.Value, "\n") {
if i > 0 {
flushLine()
cur = cur[:0]
}
part = strings.TrimLeft(part, "\t")
cur = append(cur, &syntax.Lit{Value: part})
}
part = strings.TrimLeft(part, "\t")
cur = append(cur, &syntax.Lit{Value: part})
}
}
flushLine()
return &buf
flushLine()
pw.Close()
}()
return pr, nil
}

func (r *Runner) redir(ctx context.Context, rd *syntax.Redirect) (io.Closer, error) {
if rd.Hdoc != nil {
r.stdin = r.hdocReader(rd)
return nil, nil
pr, err := r.hdocReader(rd)
if err != nil {
return nil, err
}
r.stdin = pr
return pr, nil
}
orig := &r.stdout
if rd.N != nil {
Expand All @@ -846,8 +863,19 @@ func (r *Runner) redir(ctx context.Context, rd *syntax.Redirect) (io.Closer, err
arg := r.literal(rd.Word)
switch rd.Op {
case syntax.WordHdoc:
r.stdin = strings.NewReader(arg + "\n")
return nil, nil
pr, pw, err := os.Pipe()
if err != nil {
return nil, err
}
r.stdin = pr
// We write to the pipe in a new goroutine,
// as pipe writes may block once the buffer gets full.
go func() {
pw.WriteString(arg)
pw.WriteString("\n")
pw.Close()
}()
return pr, nil
case syntax.DplOut:
switch arg {
case "1":
Expand Down

0 comments on commit 980b6fc

Please sign in to comment.