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(js): empty key entry list metadata in react-native #105

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Feb 27, 2023

This prevents a crash when fetching an existing key in iOS (and likely also in Android) and askar_key_entry_list_get_metadata returns NULL:

image

Not sure if the same should be applied to other parts of the code (it was already done in some other methods such as entryListGetTags and keyEntryListGetTags).

Signed-off-by: Ariel Gentile <gentilester@gmail.com>
@genaris genaris changed the title fix(javascript): empty key entry list metadata in react-native fix(js): empty key entry list metadata in react-native Feb 28, 2023
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

I am fine with either, but I think returning an undefined would be better instead of an empty string. I hope the syntax is correct :).

@@ -1016,7 +1016,7 @@ jsi::Value keyEntryListGetMetadata(jsi::Runtime &rt, jsi::Object options) {
askar_key_entry_list_get_metadata(keyEntryListHandle, index, &out);
handleError(rt, code);

return jsi::String::createFromAscii(rt, out);
return jsi::String::createFromAscii(rt, out ? out : "{}");
Copy link
Contributor

@berendsliedrecht berendsliedrecht Mar 2, 2023

Choose a reason for hiding this comment

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

Suggested change
return jsi::String::createFromAscii(rt, out ? out : "{}");
if out == nullptr {
return jsi::Value::null();
} else {
return jsi::String::createFromAscii(rt, out);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the return type as suggested, and did the same with entryListGetValue and entryListGetTags. Seems to be working fine with the RN sample app from test repo.

Signed-off-by: Ariel Gentile <gentilester@gmail.com>
@TimoGlastra TimoGlastra merged commit 41c7f46 into openwallet-foundation:main Mar 6, 2023
jamshale pushed a commit to jamshale/askar that referenced this pull request Aug 18, 2024
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.

3 participants