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

Add support for sandboxed socket path #10

Merged
merged 2 commits into from
Jul 16, 2022

Conversation

WhyNotHugo
Copy link
Contributor

Rather than try a single path, try all supported paths, in the preferred
order. This avoids breaking compatibility with previous KeePassXC
versions.

See: keepassxreboot/keepassxc#8018
Fixes: #8
Supercedes: #7

Hugo Osvaldo Barrera added 2 commits June 1, 2022 18:46
`usize` is made exactly for this (e.g.: the size of a buffer), and
signatures are a bit more idiomatic.

These changes also try to cast the u32 to a usize as soon as we read it,
and will panic if there is an issue (whereas using `as usize` may result
in silent conversion errors on uncommon architectures.
Rather than try a single path, try all supported paths, in the preferred
order. This avoids breaking compatibility with previous KeePassXC
versions.

See: keepassxreboot/keepassxc#8018
Fixes: varjolintu#8
Supercedes: varjolintu#7
@WhyNotHugo
Copy link
Contributor Author

Oh, I had dda48b6 in the same branch, lemme know if you'd prefer to review this separately.

@varjolintu
Copy link
Owner

Thank you. I haven't been updating the repo for a while. I really appreciate the help!

@WhyNotHugo
Copy link
Contributor Author

You're welcome! I found this simple proxy implementation nice and easy to read, which was a great way to understand how KP and the browser communicate.

My intention is to make a few other changes, so we can use something like inotify to reach when the socket is created (if it doesn't exist at startup).

@WhyNotHugo
Copy link
Contributor Author

This is backwards-compatible and works with both previous versions of KPXC and with the master version.

Anything blocking this from being merged?

I can move dda48b6 onto a separate PR if you dislike the scope-creep of it.

@varjolintu varjolintu merged commit 5b993d1 into varjolintu:master Jul 16, 2022
This pull request was closed.
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.

Socket has moved on latest keepassxc master
2 participants