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

Bugfix: Implement escape logic. #90

Closed
wants to merge 1 commit into from

Conversation

mgelde
Copy link

@mgelde mgelde commented Aug 7, 2021

GNOME Keyring uses a specific escape logic for non alphanumeric
characters in collection names. Due to this, collections with names
containing such characters (e.g. whitespace, underscores) cannot be
found using this library (string comparison fails).

This patch implements the GNOME Keyring escape logic and uses it when
searching for the desired collection.

GNOME Keyring uses a specific escape logic for non alphanumeric
characters in collection names. Due to this, collections with names
containing such characters (e.g. whitespace, underscores) cannot be
found using this library (string comparison fails).

This patch implements the GNOME Keyring escape logic and uses it when
searching for the desired collection.
@mtibben
Copy link
Member

mtibben commented Oct 11, 2021

@mgelde Could you please add some tests

@mtibben
Copy link
Member

mtibben commented Oct 11, 2021

Is #83 doing something similar?

}
}
return string(output_buffer)
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this algorithm documented somewhere that we can reference? I can't find similar code in https://github.com/GNOME/gnome-keyring

Copy link
Author

Choose a reason for hiding this comment

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

I realize you merged #83, which works for me. But for future reference: https://github.com/GNOME/gnome-keyring/blob/f24e374/daemon/dbus/gkd-secret-util.c#L111
I did not see any documentation other than the code...

@mtibben
Copy link
Member

mtibben commented Oct 11, 2021

Fixed in #83

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

2 participants