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

Empty object produces an error when passed as document to oa-verify #111

Closed
Neketek opened this issue Aug 25, 2020 · 4 comments · Fixed by #115
Closed

Empty object produces an error when passed as document to oa-verify #111

Neketek opened this issue Aug 25, 2020 · 4 comments · Fixed by #115
Labels

Comments

@Neketek
Copy link

Neketek commented Aug 25, 2020

Hi, noticed that

//  Cannot read property 'issuers' of undefined
const result = await verify({});

will throw an error because some validators are unable to find issuers data inside the empty object. Prevention of this behavior requires double-checking when validating, and that's not good. I assume that it's better to handle missing keys and just return proper validation error instead of plain JS error.

@Nebulis
Copy link
Contributor

Nebulis commented Aug 25, 2020

Hi,

What do you mean by proper validation error? Do you expect us to throw an error with a message like "invalid document" or something else?

@Neketek
Copy link
Author

Neketek commented Aug 25, 2020

@Nebulis I mean, that it's clearly a validator encountered the situation that it didn't expect, unexpected error, in other words. In this case an empty document. So, I think it will be logical if instead of failing to perform validation and interrupting the whole process it will simply return INVALID result with info pointing to the incorrect/unexpected document structure.

@Nebulis
Copy link
Contributor

Nebulis commented Aug 26, 2020

3 verifiers trigger an error when an empty object is passed down:

  • OpenAttestationEthereumDocumentStoreStatus
  • OpenAttestationDnsTxt
  • OpenAttestationEthereumTokenRegistryStatus

For all those verifiers the error happens during the test function (which determines if a verifier should run or not)

The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.

On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.

Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

Nebulis added a commit that referenced this issue Sep 1, 2020
closes #111

3 verifiers trigger an error when an empty object is passed down:

OpenAttestationEthereumDocumentStoreStatus
OpenAttestationDnsTxt
OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not)

The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.

On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.

Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)
Nebulis added a commit that referenced this issue Sep 1, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
Nebulis added a commit that referenced this issue Sep 2, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
Nebulis added a commit that referenced this issue Sep 2, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
Nebulis added a commit that referenced this issue Sep 2, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
Nebulis added a commit that referenced this issue Sep 2, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
Nebulis added a commit that referenced this issue Sep 3, 2020
3 verifiers trigger an error when an empty object is passed down:
- OpenAttestationEthereumDocumentStoreStatus
- OpenAttestationDnsTxt
- OpenAttestationEthereumTokenRegistryStatus
For all those verifiers the error happens during the test function (which determines if a verifier should run or not).
The plan is to fix those test functions so that the verifiers doesn't run if issuers property (in that case) is not provided => the result for those verifiers will be skipped fragment.
On top of that OpenAttestationHash verifier currently return invalid while it shouldn't run because targethash/merkleroot/data are missing => the plan is to fix this behaviour so that the verifier is skipped in that case.
Also to make sure that verifiers test function handle correctly, we should provide a partial object (deeply)

closes #111
@john-dot-oa
Copy link
Contributor

🎉 This issue has been resolved in version 4.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants