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

Fix crash when creating 'KeychainSettings' without name on mac. #175

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

MJegorovas
Copy link

There is an edge case with creating KeychainSettings on Mac with no name and calling keys variable. Because kSecAttrService is not set you get a bunch of random values which might not have kSecAttrAccount set so the call fails with NullPointerException. Not sure if the fix style and new test assertion work for you, just wanted to raise the issue. Tested on m2 mac.

@russhwolf
Copy link
Owner

Thanks. I don't completely understand what happens here but I see that we can sometimes get null returns from CFDictionaryGetValue which I didn't realize would actually happen in practice. I should have done more testing when adding the parameterless constructor because it's a little different/dangerous how it picks up everything and we can't make as many assumptions about the data that's in there.

@russhwolf
Copy link
Owner

Pushed some edits. Sorry that it kills some of the code you had in the git blame, but I'll leave the commits separate so you don't completely lose credit.

@russhwolf russhwolf merged commit 1891ed8 into russhwolf:main Nov 21, 2023
3 of 4 checks passed
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