Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dashes in secret service backend break roundtrip (fix #82) #83

Merged
merged 1 commit into from Oct 11, 2021

Conversation

leejw51crypto
Copy link
Contributor

fix reading in gnome keyring

libsecret.go Outdated Show resolved Hide resolved
@@ -56,6 +59,28 @@ func (e *secretsError) Error() string {

var errCollectionNotFound = errors.New("The collection does not exist. Please add a key first")

func decodeKeyringString(src string) string {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good function to have some unit tests for ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i'll add unit test

@nickatsegment
Copy link

master...nickatsegment:repro-secret-service-dashes?expand=1#diff-8b366881d0 might want to incorporate this test I created to show that it fixes the bug

remove debug code

tidy up

remove un-necessary condition

add unit test
@leejw51crypto
Copy link
Contributor Author

add unit test

@tomtau
Copy link

tomtau commented Apr 22, 2021

@mtibben

@mtibben
Copy link
Member

mtibben commented Oct 11, 2021

Is #90 doing something similar?

@tomtau
Copy link

tomtau commented Oct 11, 2021

Yes, it is

@mtibben
Copy link
Member

mtibben commented Oct 11, 2021

@tomtau which approach is better?

@mtibben mtibben merged commit 74483f7 into 99designs:master Oct 11, 2021
@tomtau
Copy link

tomtau commented Oct 11, 2021

#90 may be simpler (it encodes the provided secret path once, while this PR attempts to decode every retrieved path in collections)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants