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 empty objects in credential throws error #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cykoder
Copy link
Member

@cykoder cykoder commented Apr 4, 2023

No description provided.

@cykoder cykoder force-pushed the fix/empty-objects branch from b8194f9 to ff3741d Compare April 4, 2023 03:35
@cykoder cykoder force-pushed the fix/empty-objects branch from ff3741d to 254aa17 Compare April 4, 2023 03:37
@cykoder cykoder requested a review from lovesh April 4, 2023 04:01

// If empty object, skip it here otherwise causes problems downstream
if (schemaKeys.size === 0) {
delete schemaProps[key];
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed as it will ignore credential values for which the schema has an empty object type. Such a schema is not valid and an error should be thrown. Secondly, is ignoring those fields what you want?

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't needed as it will ignore credential values for which the schema has an empty object type

that is intended, because that value cant exactly be encoded (unless its stringified to {} but that seems pointless to me?)

Copy link
Member

Choose a reason for hiding this comment

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

Why? The schema should never have an empty object.

builder.schema = new CredentialSchema(CredentialSchema.essential());
builder.subject = {
fname: 'John',
emptyObject: {},
Copy link
Member

Choose a reason for hiding this comment

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

There should be a similar test for an empty array.

@@ -56,7 +57,8 @@ export function dockSaverEncryptionGensUncompressed(): SaverEncryptionGensUncomp
export function flattenTill2ndLastKey(obj: object): [string[], object[]] {
const flattened = {};
const temp = flatten(obj) as object;
for (const k of Object.keys(temp)) {
const tempKeys = Object.keys(temp).filter((key) => typeof temp[key] !== 'object' || !isEmptyObject(temp[key]));
Copy link
Member

Choose a reason for hiding this comment

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

The following for loop is going to iterate over keys of temp. Rather than iterating again, move these checks inside the loop.

src/bbs-plus/encoder.ts Outdated Show resolved Hide resolved
Signed-off-by: Samuel Hellawell <[email protected]>
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.

2 participants