diff --git a/dbus/dbus.go b/dbus/dbus.go index 7a51d98..b993d52 100644 --- a/dbus/dbus.go +++ b/dbus/dbus.go @@ -12,7 +12,7 @@ import ( // Proxy holds references to a xdg-dbus-proxy process, and should never be copied. // Once sealed, configuration changes will no longer be possible and attempting to do so will result in a panic. type Proxy struct { - helper *helper.Helper + helper helper.Helper path string session [2]string @@ -35,7 +35,7 @@ func (p *Proxy) String() string { defer p.lock.RUnlock() if p.helper != nil { - return p.helper.String() + return p.helper.Unwrap().String() } if p.seal != nil { diff --git a/dbus/run.go b/dbus/run.go index da0bbc8..5e489ba 100644 --- a/dbus/run.go +++ b/dbus/run.go @@ -27,12 +27,13 @@ func (p *Proxy) Start(ready chan error, output io.Writer) error { } }, ) + cmd := h.Unwrap() // xdg-dbus-proxy does not need to inherit the environment - h.Env = []string{} + cmd.Env = []string{} if output != nil { - h.Stdout = output - h.Stderr = output + cmd.Stdout = output + cmd.Stderr = output } if err := h.StartNotify(ready); err != nil { return err diff --git a/helper/direct.go b/helper/direct.go new file mode 100644 index 0000000..24f9e75 --- /dev/null +++ b/helper/direct.go @@ -0,0 +1,93 @@ +package helper + +import ( + "errors" + "io" + "os/exec" + "sync" +) + +// direct wraps *exec.Cmd and manages status and args fd. +// Args is always 3 and status if set is always 4. +type direct struct { + // helper pipes + // cannot be nil + p *pipes + + // returns an array of arguments passed directly + // to the helper process + argF func(argsFD, statFD int) []string + + lock sync.RWMutex + *exec.Cmd +} + +func (h *direct) StartNotify(ready chan error) error { + h.lock.Lock() + defer h.lock.Unlock() + + // Check for doubled Start calls before we defer failure cleanup. If the prior + // call to Start succeeded, we don't want to spuriously close its pipes. + if h.Cmd.Process != nil { + return errors.New("exec: already started") + } + + h.p.ready = ready + if argsFD, statFD, err := h.p.prepareCmd(h.Cmd); err != nil { + return err + } else { + h.Cmd.Args = append(h.Cmd.Args, h.argF(argsFD, statFD)...) + } + + if ready != nil { + h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=1") + } else { + h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=0") + } + + if err := h.Cmd.Start(); err != nil { + return err + } + if err := h.p.readyWriteArgs(); err != nil { + return err + } + + return nil +} + +func (h *direct) Wait() error { + h.lock.RLock() + defer h.lock.RUnlock() + + if h.Cmd.Process == nil { + return errors.New("exec: not started") + } + if h.Cmd.ProcessState != nil { + return errors.New("exec: Wait was already called") + } + + defer h.p.mustClosePipes() + return h.Cmd.Wait() +} + +func (h *direct) Close() error { + return h.p.closeStatus() +} + +func (h *direct) Start() error { + return h.StartNotify(nil) +} + +func (h *direct) Unwrap() *exec.Cmd { + return h.Cmd +} + +// New initialises a new direct Helper instance with wt as the null-terminated argument writer. +// Function argF returns an array of arguments passed directly to the child process. +func New(wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) Helper { + if wt == nil { + panic("attempted to create helper with invalid argument writer") + } + + return &direct{p: &pipes{args: wt}, argF: argF, Cmd: execCommand(name)} +} diff --git a/helper/helper_test.go b/helper/direct_test.go similarity index 96% rename from helper/helper_test.go rename to helper/direct_test.go index 0bbc569..12a7b52 100644 --- a/helper/helper_test.go +++ b/helper/direct_test.go @@ -49,9 +49,10 @@ func TestHelper_StartNotify_Close_Wait(t *testing.T) { t.Run("start helper with status channel", func(t *testing.T) { h := helper.New(argsWt, "crash-test-dummy", argFStatus) ready := make(chan error, 1) + cmd := h.Unwrap() stdout, stderr := new(strings.Builder), new(strings.Builder) - h.Stdout, h.Stderr = stdout, stderr + cmd.Stdout, cmd.Stderr = stdout, stderr t.Run("wait not yet started helper", func(t *testing.T) { wantErr := "exec: not started" @@ -87,7 +88,7 @@ func TestHelper_StartNotify_Close_Wait(t *testing.T) { t.Errorf("never got a ready response") t.Errorf("stdout:\n%s", stdout.String()) t.Errorf("stderr:\n%s", stderr.String()) - if err := h.Cmd.Process.Kill(); err != nil { + if err := cmd.Process.Kill(); err != nil { panic(err.Error()) } return @@ -143,9 +144,10 @@ func TestHelper_Start_Close_Wait(t *testing.T) { t.Run("start helper", func(t *testing.T) { h := helper.New(wt, "crash-test-dummy", argF) + cmd := h.Unwrap() stdout, stderr := new(strings.Builder), new(strings.Builder) - h.Stdout, h.Stderr = stdout, stderr + cmd.Stdout, cmd.Stderr = stdout, stderr if err := h.Start(); err != nil { t.Errorf("Start() error = %v", diff --git a/helper/helper.go b/helper/helper.go index ed4e457..a634d59 100644 --- a/helper/helper.go +++ b/helper/helper.go @@ -5,9 +5,7 @@ package helper import ( "errors" - "io" "os/exec" - "sync" ) var ( @@ -22,81 +20,19 @@ const ( FortifyStatus = "FORTIFY_STATUS" ) -// Helper wraps *exec.Cmd and manages status and args fd. -// Args is always 3 and status if set is always 4. -type Helper struct { - p *pipes - - argF func(argsFD, statFD int) []string - *exec.Cmd - - lock sync.RWMutex -} - -func (h *Helper) StartNotify(ready chan error) error { - h.lock.Lock() - defer h.lock.Unlock() - - // Check for doubled Start calls before we defer failure cleanup. If the prior - // call to Start succeeded, we don't want to spuriously close its pipes. - if h.Cmd.Process != nil { - return errors.New("exec: already started") - } - - h.p.ready = ready - if argsFD, statFD, err := h.p.prepareCmd(h.Cmd); err != nil { - return err - } else { - h.Cmd.Args = append(h.Cmd.Args, h.argF(argsFD, statFD)...) - } - - if ready != nil { - h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=1") - } else { - h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=0") - } - - if err := h.Cmd.Start(); err != nil { - return err - } - if err := h.p.readyWriteArgs(); err != nil { - return err - } - - return nil -} - -func (h *Helper) Wait() error { - h.lock.RLock() - defer h.lock.RUnlock() - - if h.Cmd.Process == nil { - return errors.New("exec: not started") - } - if h.Cmd.ProcessState != nil { - return errors.New("exec: Wait was already called") - } - - defer h.p.mustClosePipes() - return h.Cmd.Wait() -} - -func (h *Helper) Close() error { - return h.p.closeStatus() -} - -func (h *Helper) Start() error { - return h.StartNotify(nil) +type Helper interface { + // StartNotify starts the helper process. + // A status pipe is passed to the helper if ready is not nil. + StartNotify(ready chan error) error + // Start starts the helper process. + Start() error + // Close closes the status pipe. + // If helper is started without the status pipe, Close panics. + Close() error + // Wait calls wait on the child process and cleans up pipes. + Wait() error + // Unwrap returns the underlying exec.Cmd instance. + Unwrap() *exec.Cmd } var execCommand = exec.Command - -// New initialises a new Helper instance with wt as the null-terminated argument writer. -// Function argF returns an array of arguments passed directly to the child process. -func New(wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) *Helper { - if wt == nil { - panic("attempted to create helper with invalid argument writer") - } - - return &Helper{p: &pipes{args: wt}, argF: argF, Cmd: execCommand(name)} -}