From bb85cb8b529dd9161637b7f89573b0ec0de3c037 Mon Sep 17 00:00:00 2001 From: brent s Date: Sat, 25 Dec 2021 01:51:49 -0500 Subject: [PATCH] cleanly close, catch Dbus errors --- TODO | 2 + collection_funcs.go | 34 +++++++++++----- collection_funcs_test.go | 13 ++++--- item_funcs.go | 29 ++++++++++---- multierr_funcs.go | 1 + service_funcs.go | 84 +++++++++++++++++++++++++++++----------- session_funcs.go | 20 +++++++--- 7 files changed, 132 insertions(+), 51 deletions(-) diff --git a/TODO b/TODO index fc47707..adc830a 100644 --- a/TODO +++ b/TODO @@ -1 +1,3 @@ - Benchmarking? + +- call .Close() on dbus.Conns diff --git a/collection_funcs.go b/collection_funcs.go index 6dc854b..5fc2068 100644 --- a/collection_funcs.go +++ b/collection_funcs.go @@ -65,6 +65,7 @@ func NewCollection(service *Service, path dbus.ObjectPath) (coll *Collection, er */ func (c *Collection) CreateItem(label string, attrs map[string]string, secret *Secret, replace bool, itemType ...string) (item *Item, err error) { + var call *dbus.Call var prompt *Prompt var path dbus.ObjectPath var promptPath dbus.ObjectPath @@ -84,9 +85,13 @@ func (c *Collection) CreateItem(label string, attrs map[string]string, secret *S props[DbusItemCreated] = dbus.MakeVariant(uint64(time.Now().Unix())) // props[DbusItemModified] = dbus.MakeVariant(uint64(time.Now().Unix())) - if err = c.Dbus.Call( + if call = c.Dbus.Call( DbusCollectionCreateItem, 0, props, secret, replace, - ).Store(&path, &promptPath); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&path, &promptPath); err != nil { return } @@ -114,10 +119,17 @@ func (c *Collection) CreateItem(label string, attrs map[string]string, secret *S */ func (c *Collection) Delete() (err error) { + var call *dbus.Call var promptPath dbus.ObjectPath var prompt *Prompt - if err = c.Dbus.Call(DbusCollectionDelete, 0).Store(&promptPath); err != nil { + if call = c.Dbus.Call( + DbusCollectionDelete, 0, + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&promptPath); err != nil { return } @@ -243,6 +255,7 @@ func (c *Collection) Relabel(newLabel string) (err error) { */ func (c *Collection) SearchItems(profile string) (items []*Item, err error) { + var call *dbus.Call var paths []dbus.ObjectPath var errs []error = make([]error, 0) var attrs map[string]string = make(map[string]string, 0) @@ -250,9 +263,13 @@ func (c *Collection) SearchItems(profile string) (items []*Item, err error) { attrs["profile"] = profile - if err = c.Dbus.Call( + if call = c.Dbus.Call( DbusCollectionSearchItems, 0, attrs, - ).Store(&paths); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&paths); err != nil { return } @@ -277,11 +294,10 @@ func (c *Collection) SetAlias(alias string) (err error) { var call *dbus.Call - call = c.service.Dbus.Call( + if call = c.service.Dbus.Call( DbusServiceSetAlias, 0, alias, c.Dbus.Path(), - ) - - if err = call.Err; err != nil { + ); call.Err != nil { + err = call.Err return } diff --git a/collection_funcs_test.go b/collection_funcs_test.go index 7014b4b..3067abc 100644 --- a/collection_funcs_test.go +++ b/collection_funcs_test.go @@ -1,9 +1,9 @@ package gosecret import ( - `testing` + "testing" - `github.com/godbus/dbus/v5` + "github.com/godbus/dbus/v5" ) // Some functions are covered in the Service tests. @@ -57,7 +57,8 @@ func TestCollection_Items(t *testing.T) { var collection *Collection var items []*Item var item *Item - var searchItemResults []*Item + var searchResultsUnlocked []*Item + var searchResultsLocked []*Item var secret *Secret var err error @@ -109,12 +110,12 @@ func TestCollection_Items(t *testing.T) { ) } else { - if searchItemResults, err = collection.SearchItems(testItemLabel); err != nil { + if searchResultsUnlocked, searchResultsLocked, err = collection.service.SearchItems(itemAttrs); err != nil { t.Errorf("failed to find item '%v' via Collection.SearchItems: %v", string(item.Dbus.Path()), err.Error()) - } else if len(searchItemResults) == 0 { + } else if (len(searchResultsLocked) + len(searchResultsUnlocked)) == 0 { t.Errorf("failed to find item '%v' via Collection.SearchItems, returned 0 results (should be at least 1)", testItemLabel) } else { - t.Logf("found %v results for Collection.SearchItems", len(searchItemResults)) + t.Logf("found %v results for Collection.SearchItems", len(searchResultsUnlocked)+len(searchResultsLocked)) } if err = item.Delete(); err != nil { diff --git a/item_funcs.go b/item_funcs.go index 04f463d..145eaaf 100644 --- a/item_funcs.go +++ b/item_funcs.go @@ -106,10 +106,17 @@ func (i *Item) ChangeItemType(newItemType string) (err error) { // Delete removes an Item from a Collection. func (i *Item) Delete() (err error) { + var call *dbus.Call var promptPath dbus.ObjectPath var prompt *Prompt - if err = i.Dbus.Call(DbusItemDelete, 0).Store(&promptPath); err != nil { + if call = i.Dbus.Call( + DbusItemDelete, 0, + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&promptPath); err != nil { return } @@ -127,6 +134,8 @@ func (i *Item) Delete() (err error) { // GetSecret returns the Secret in an Item using a Session. func (i *Item) GetSecret(session *Session) (secret *Secret, err error) { + var call *dbus.Call + if session == nil { err = ErrNoDbusConn } @@ -135,9 +144,13 @@ func (i *Item) GetSecret(session *Session) (secret *Secret, err error) { return } - if err = i.Dbus.Call( + if call = i.Dbus.Call( DbusItemGetSecret, 0, session.Dbus.Path(), - ).Store(&secret); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&secret); err != nil { return } @@ -246,15 +259,15 @@ func (i *Item) ReplaceAttributes(newAttrs map[string]string) (err error) { // SetSecret sets the Secret for an Item. func (i *Item) SetSecret(secret *Secret) (err error) { - var c *dbus.Call + var call *dbus.Call - c = i.Dbus.Call( + if call = i.Dbus.Call( DbusItemSetSecret, 0, - ) - if c.Err != nil { - err = c.Err + ); call.Err != nil { + err = call.Err return } + i.Secret = secret if _, _, err = i.Modified(); err != nil { diff --git a/multierr_funcs.go b/multierr_funcs.go index 074be25..2bd86c4 100644 --- a/multierr_funcs.go +++ b/multierr_funcs.go @@ -35,6 +35,7 @@ func NewErrors(errs ...error) (err error) { return } +// Error makes a MultiError conform to the error interface. func (e *MultiError) Error() (errStr string) { var numErrs int diff --git a/service_funcs.go b/service_funcs.go index c633f18..0472506 100644 --- a/service_funcs.go +++ b/service_funcs.go @@ -5,7 +5,7 @@ import ( "fmt" "path/filepath" "strings" - `time` + "time" "github.com/godbus/dbus/v5" ) @@ -38,7 +38,13 @@ func NewService() (service *Service, err error) { // Close cleanly closes a Service and all its underlying connections (e.g. Service.Session). func (s *Service) Close() (err error) { - err = s.Session.Close() + if err = s.Session.Close(); err != nil { + return + } + + if err = s.Conn.Close(); err != nil { + return + } return } @@ -80,6 +86,7 @@ func (s *Service) Collections() (collections []*Collection, err error) { */ func (s *Service) CreateAliasedCollection(label, alias string) (collection *Collection, err error) { + var call *dbus.Call var variant *dbus.Variant var path dbus.ObjectPath var promptPath dbus.ObjectPath @@ -90,9 +97,13 @@ func (s *Service) CreateAliasedCollection(label, alias string) (collection *Coll props[DbusCollectionCreated] = dbus.MakeVariant(uint64(time.Now().Unix())) props[DbusCollectionModified] = dbus.MakeVariant(uint64(time.Now().Unix())) - if err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceCreateCollection, 0, props, alias, - ).Store(&path, &promptPath); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&path, &promptPath); err != nil { return } @@ -197,6 +208,7 @@ func (s *Service) GetSecrets(itemPaths ...dbus.ObjectPath) (secrets map[dbus.Obj } */ var results map[dbus.ObjectPath][]interface{} + var call *dbus.Call if itemPaths == nil || len(itemPaths) == 0 { err = ErrMissingPaths @@ -207,9 +219,13 @@ func (s *Service) GetSecrets(itemPaths ...dbus.ObjectPath) (secrets map[dbus.Obj results = make(map[dbus.ObjectPath][]interface{}, len(itemPaths)) // TODO: trigger a Service.Unlock for any locked items? - if err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceGetSecrets, 0, itemPaths, s.Session.Dbus.Path(), - ).Store(&results); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&results); err != nil { return } @@ -236,6 +252,7 @@ func (s *Service) GetSession() (ssn *Session, err error) { // Lock locks an Unlocked Collection or Item (LockableObject). func (s *Service) Lock(objects ...LockableObject) (err error) { + var call *dbus.Call var toLock []dbus.ObjectPath // We only use these as destinations. var locked []dbus.ObjectPath @@ -253,9 +270,13 @@ func (s *Service) Lock(objects ...LockableObject) (err error) { toLock[idx] = o.path() } - if err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceLock, 0, toLock, - ).Store(&locked, &promptPath); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&locked, &promptPath); err != nil { return } @@ -285,6 +306,7 @@ func (s *Service) Lock(objects ...LockableObject) (err error) { */ func (s *Service) OpenSession(algo, input string) (session *Session, output dbus.Variant, err error) { + var call *dbus.Call var path dbus.ObjectPath var inputVariant dbus.Variant @@ -298,9 +320,13 @@ func (s *Service) OpenSession(algo, input string) (session *Session, output dbus // TODO: confirm this. // Possible flags are dbus.Flags consts: https://pkg.go.dev/github.com/godbus/dbus#Flags // Oddly, there is no "None" flag. So it's explicitly specified as a null byte. - if err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceOpenSession, 0, algo, inputVariant, - ).Store(&output, &path); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&output, &path); err != nil { return } @@ -316,17 +342,20 @@ func (s *Service) OpenSession(algo, input string) (session *Session, output dbus */ func (s *Service) ReadAlias(alias string) (collection *Collection, err error) { + var call *dbus.Call var objectPath dbus.ObjectPath - err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceReadAlias, 0, alias, - ).Store(&objectPath) - + ); call.Err != nil { + err = call.Err + return + } /* TODO: Confirm that a nonexistent alias will NOT cause an error to return. If it does, alter the below logic. */ - if err != nil { + if err = call.Store(&objectPath); err != nil { return } @@ -358,6 +387,7 @@ func (s *Service) RemoveAlias(alias string) (err error) { */ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*Item, lockedItems []*Item, err error) { + var call *dbus.Call var locked []dbus.ObjectPath var unlocked []dbus.ObjectPath var collectionObjs []*Collection @@ -373,9 +403,13 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It return } - err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceSearchItems, 0, attributes, - ).Store(&unlocked, &locked) + ); call.Err != nil { + } + if err = call.Store(&unlocked, &locked); err != nil { + return + } lockedItems = make([]*Item, 0) unlockedItems = make([]*Item, 0) @@ -450,18 +484,17 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It */ func (s *Service) SetAlias(alias string, objectPath dbus.ObjectPath) (err error) { - var c *dbus.Call + var call *dbus.Call var collection *Collection if collection, err = s.GetCollection(alias); err != nil { return } - c = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceSetAlias, 0, alias, objectPath, - ) - - if err = c.Err; err != nil { + ); call.Err != nil { + err = call.Err return } @@ -477,6 +510,7 @@ func (s *Service) SetAlias(alias string, objectPath dbus.ObjectPath) (err error) // Unlock unlocks a locked Collection or Item (LockableObject). func (s *Service) Unlock(objects ...LockableObject) (err error) { + var call *dbus.Call var toUnlock []dbus.ObjectPath // We only use these as destinations. var unlocked []dbus.ObjectPath @@ -494,9 +528,13 @@ func (s *Service) Unlock(objects ...LockableObject) (err error) { toUnlock[idx] = o.path() } - if err = s.Dbus.Call( + if call = s.Dbus.Call( DbusServiceUnlock, 0, toUnlock, - ).Store(&unlocked, &resultPath); err != nil { + ); call.Err != nil { + err = call.Err + return + } + if err = call.Store(&unlocked, &resultPath); err != nil { return } diff --git a/session_funcs.go b/session_funcs.go index 0c4fb8b..0fffafa 100644 --- a/session_funcs.go +++ b/session_funcs.go @@ -1,6 +1,8 @@ package gosecret import ( + "fmt" + "github.com/godbus/dbus/v5" ) @@ -32,13 +34,21 @@ func NewSession(service *Service, path dbus.ObjectPath) (session *Session, err e // Close cleanly closes a Session. func (s *Session) Close() (err error) { - var c *dbus.Call + var call *dbus.Call - c = s.Dbus.Call( + if call = s.Dbus.Call( DbusSessionClose, 0, - ) - - _ = c + ); call.Err != nil { + /* + I... still haven't 100% figured out why this happens, but the session DOES seem to close...? + PRs or input welcome. + TODO: figure out why this error gets triggered. + */ + if call.Err.Error() != fmt.Sprintf("The name %v was not provided by any .service files", DbusInterfaceSession) { + err = call.Err + return + } + } return }