From 985c4dd2fc175d8c52304ee387a71e7d855b421a Mon Sep 17 00:00:00 2001 From: Ophestra Date: Mon, 8 Sep 2025 04:17:39 +0900 Subject: [PATCH] system/xhost: wrap revert error correctly This otherwise creates a confusing error message on a revert failure. Signed-off-by: Ophestra --- system/dispatcher.go | 8 +++++ system/dispatcher_test.go | 9 +++++ system/output_test.go | 67 ---------------------------------- system/xhost.go | 16 ++++----- system/xhost_test.go | 75 +++++++++++++++++++++++++-------------- 5 files changed, 74 insertions(+), 101 deletions(-) diff --git a/system/dispatcher.go b/system/dispatcher.go index 502c8e9..f8de2ff 100644 --- a/system/dispatcher.go +++ b/system/dispatcher.go @@ -7,6 +7,7 @@ import ( "hakurei.app/system/acl" "hakurei.app/system/dbus" + "hakurei.app/system/internal/xcb" ) type osFile interface { @@ -39,6 +40,9 @@ type syscallDispatcher interface { // aclUpdate provides [acl.Update]. aclUpdate(name string, uid int, perms ...acl.Perm) error + // xcbChangeHosts provides [xcb.ChangeHosts]. + xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address string) error + // dbusAddress provides [dbus.Address]. dbusAddress() (session, system string) // dbusFinalise provides [dbus.Finalise]. @@ -71,6 +75,10 @@ func (k direct) aclUpdate(name string, uid int, perms ...acl.Perm) error { return acl.Update(name, uid, perms...) } +func (k direct) xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address string) error { + return xcb.ChangeHosts(mode, family, address) +} + func (k direct) dbusAddress() (session, system string) { return dbus.Address() } diff --git a/system/dispatcher_test.go b/system/dispatcher_test.go index f798047..7bd4e6d 100644 --- a/system/dispatcher_test.go +++ b/system/dispatcher_test.go @@ -13,6 +13,7 @@ import ( "hakurei.app/container/stub" "hakurei.app/system/acl" "hakurei.app/system/dbus" + "hakurei.app/system/internal/xcb" ) // call initialises a [stub.Call]. @@ -280,6 +281,14 @@ func (k *kstub) aclUpdate(name string, uid int, perms ...acl.Perm) error { stub.CheckArgReflect(k.Stub, "perms", perms, 2)) } +func (k *kstub) xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address string) error { + k.Helper() + return k.Expects("xcbChangeHosts").Error( + stub.CheckArg(k.Stub, "mode", mode, 0), + stub.CheckArg(k.Stub, "family", family, 1), + stub.CheckArg(k.Stub, "address", address, 2)) +} + func (k *kstub) dbusAddress() (session, system string) { k.Helper() ret := k.Expects("dbusAddress").Ret.([2]string) diff --git a/system/output_test.go b/system/output_test.go index b091f5e..80abcf8 100644 --- a/system/output_test.go +++ b/system/output_test.go @@ -158,70 +158,3 @@ func TestPrintJoinedError(t *testing.T) { }) } } - -type tcOp struct { - et Enablement - path string -} - -// test an instance of the Op interface -func (ptc tcOp) test(t *testing.T, gotOps []Op, wantOps []Op, fn string) { - if len(gotOps) != len(wantOps) { - t.Errorf("%s: inserted %v Ops, want %v", fn, - len(gotOps), len(wantOps)) - return - } - - t.Run("path", func(t *testing.T) { - if len(gotOps) > 0 { - if got := gotOps[0].Path(); got != ptc.path { - t.Errorf("Path() = %q, want %q", - got, ptc.path) - return - } - } - }) - - for i := range gotOps { - o := gotOps[i] - - t.Run("is", func(t *testing.T) { - if !o.Is(o) { - t.Errorf("Is returned false on self") - return - } - if !o.Is(wantOps[i]) { - t.Errorf("%s: inserted %#v, want %#v", - fn, - o, wantOps[i]) - return - } - }) - - t.Run("criteria", func(t *testing.T) { - testCases := []struct { - name string - ec Enablement - want bool - }{ - {"nil", 0xff, ptc.et != User}, - {"self", ptc.et, true}, - {"all", EWayland | EX11 | EDBus | EPulse | User | Process, true}, - {"enablements", EWayland | EX11 | EDBus | EPulse, ptc.et != User && ptc.et != Process}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var criteria *Criteria - if tc.ec != 0xff { - criteria = (*Criteria)(&tc.ec) - } - if got := criteria.hasType(o.Type()); got != tc.want { - t.Errorf("hasType: got %v, want %v", - got, tc.want) - } - }) - } - }) - } -} diff --git a/system/xhost.go b/system/xhost.go index cdec129..4ee4993 100644 --- a/system/xhost.go +++ b/system/xhost.go @@ -15,23 +15,23 @@ type xhostOp string func (x xhostOp) Type() Enablement { return EX11 } -func (x xhostOp) apply(*I) error { - msg.Verbosef("inserting entry %s to X11", x) +func (x xhostOp) apply(sys *I) error { + sys.verbosef("inserting entry %s to X11", x) return newOpError("xhost", - xcb.ChangeHosts(xcb.HostModeInsert, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), false) + sys.xcbChangeHosts(xcb.HostModeInsert, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), false) } -func (x xhostOp) revert(_ *I, ec *Criteria) error { +func (x xhostOp) revert(sys *I, ec *Criteria) error { if ec.hasType(x.Type()) { - msg.Verbosef("deleting entry %s from X11", x) + sys.verbosef("deleting entry %s from X11", x) return newOpError("xhost", - xcb.ChangeHosts(xcb.HostModeDelete, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), false) + sys.xcbChangeHosts(xcb.HostModeDelete, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), true) } else { - msg.Verbosef("skipping entry %s in X11", x) + sys.verbosef("skipping entry %s in X11", x) return nil } } func (x xhostOp) Is(o Op) bool { target, ok := o.(xhostOp); return ok && x == target } -func (x xhostOp) Path() string { return string(x) } +func (x xhostOp) Path() string { return "/tmp/.X11-unix" } func (x xhostOp) String() string { return string("SI:localuser:" + x) } diff --git a/system/xhost_test.go b/system/xhost_test.go index 32cb063..68601e8 100644 --- a/system/xhost_test.go +++ b/system/xhost_test.go @@ -2,33 +2,56 @@ package system import ( "testing" + + "hakurei.app/container/stub" + "hakurei.app/system/internal/xcb" ) -func TestChangeHosts(t *testing.T) { - testCases := []string{"chronos", "keyring", "cat", "kbd", "yonah"} - for _, tc := range testCases { - t.Run("append ChangeHosts operation for "+tc, func(t *testing.T) { - sys := New(t.Context(), 150) - sys.ChangeHosts(tc) - (&tcOp{EX11, tc}).test(t, sys.ops, []Op{ - xhostOp(tc), - }, "ChangeHosts") - }) - } -} +func TestXHostOp(t *testing.T) { + checkOpBehaviour(t, []opBehaviourTestCase{ + {"xcbChangeHosts revert", 0xbeef, EX11, xhostOp("chronos"), []stub.Call{ + call("verbosef", stub.ExpectArgs{"inserting entry %s to X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeInsert), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, stub.UniqueError(1)), + }, &OpError{Op: "xhost", Err: stub.UniqueError(1)}, nil, nil}, -func TestXHost_String(t *testing.T) { - testCases := []struct { - username string - want string - }{ - {"chronos", "SI:localuser:chronos"}, - } - for _, tc := range testCases { - t.Run(tc.want, func(t *testing.T) { - if got := xhostOp(tc.username).String(); got != tc.want { - t.Errorf("String() = %v, want %v", got, tc.want) - } - }) - } + {"xcbChangeHosts revert", 0xbeef, EX11, xhostOp("chronos"), []stub.Call{ + call("verbosef", stub.ExpectArgs{"inserting entry %s to X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeInsert), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, nil), + }, nil, []stub.Call{ + call("verbosef", stub.ExpectArgs{"deleting entry %s from X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeDelete), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, stub.UniqueError(0)), + }, &OpError{Op: "xhost", Err: stub.UniqueError(0), Revert: true}}, + + {"success skip", 0xbeef, 0, xhostOp("chronos"), []stub.Call{ + call("verbosef", stub.ExpectArgs{"inserting entry %s to X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeInsert), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, nil), + }, nil, []stub.Call{ + call("verbosef", stub.ExpectArgs{"skipping entry %s in X11", []any{xhostOp("chronos")}}, nil, nil), + }, nil}, + + {"success", 0xbeef, EX11, xhostOp("chronos"), []stub.Call{ + call("verbosef", stub.ExpectArgs{"inserting entry %s to X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeInsert), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, nil), + }, nil, []stub.Call{ + call("verbosef", stub.ExpectArgs{"deleting entry %s from X11", []any{xhostOp("chronos")}}, nil, nil), + call("xcbChangeHosts", stub.ExpectArgs{xcb.HostMode(xcb.HostModeDelete), xcb.Family(xcb.FamilyServerInterpreted), "localuser\x00chronos"}, nil, nil), + }, nil}, + }) + + checkOpsBuilder(t, "ChangeHosts", []opsBuilderTestCase{ + {"xhost", 0xcafebabe, func(_ *testing.T, sys *I) { + sys.ChangeHosts("chronos") + }, []Op{ + xhostOp("chronos"), + }, stub.Expect{}}, + }) + + checkOpIs(t, []opIsTestCase{ + {"differs", xhostOp("kbd"), xhostOp("chronos"), false}, + {"equals", xhostOp("chronos"), xhostOp("chronos"), true}, + }) + + checkOpMeta(t, []opMetaTestCase{ + {"xhost", xhostOp("chronos"), EX11, "/tmp/.X11-unix", "SI:localuser:chronos"}, + }) }