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

Accounts with user ids containing forward slashes can't be loaded correctly #842

Open
TobiasFella opened this issue Dec 2, 2024 · 3 comments · May be fixed by #846
Open

Accounts with user ids containing forward slashes can't be loaded correctly #842

TobiasFella opened this issue Dec 2, 2024 · 3 comments · May be fixed by #846
Assignees
Labels
bug/fix The library doesn't work as expected

Comments

@TobiasFella
Copy link
Member

Reported as https://bugs.kde.org/show_bug.cgi?id=491252

The user id is used as a key for QSettings, which uses forward slashes as separators. When listing the account, wrong groups are thus returned.

We need to somehow come up with a way of escaping forward slashes while not breaking existing logged-in accounts (at least those that do not contain forward slashes; for the other ones, logging in again is probably inevitable but also not problematic. I haven't been able to come up with a nice way to do that yet.

@TobiasFella TobiasFella added the bug/fix The library doesn't work as expected label Dec 2, 2024
@KitsuneRal
Copy link
Member

I was about to say that forward slashes in user ids are a deprecated legacy but no ;(
Why can't we "simply" percent-encode the forward slashes though?

@KitsuneRal KitsuneRal moved this to In work in libQuotient 1 Dec 3, 2024
@KitsuneRal KitsuneRal self-assigned this Dec 20, 2024
@KitsuneRal
Copy link
Member

KitsuneRal commented Dec 21, 2024

Okay, I think I understand at least a part of the challenge - there's plenty of places where they key is joined with the group name and Settings already gets the full key name, with "right" and "wrong" slashes indistinguishable from each other. Otherwise, percent-encoded slashes are double-percent-encoded by QSettings in the actually stored keys (if I encode / to %2F what actually gets written is %252F) but that's not a big deal as long as it's consistently used to match/store/remove keys I think?

@KitsuneRal
Copy link
Member

A bigger problem is with SettingsGroup, specifically childGroups() and remove(), used in both Quaternion and NeoChat to respectively collect and remove accounts from the settings. The problem is that not all SettingsGroups are groups of accounts and we might not want to aggressively unescape keys from any group. This can be worked around for now by checking directly whether a given group is named Accounts; and for 0.10 we can make a dedicated API for the accounts group, and even warn in the logs at runtime if someone tries to use SettingsGroup(u"Accounts"_s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/fix The library doesn't work as expected
Projects
Status: In work
Development

Successfully merging a pull request may close this issue.

2 participants