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

Checking if encodedDetails can be decoded in the parse function #899

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MarkusGarmeister
Copy link

fixes KILTProtocol/ticket#3033

A light DID resolution with encoded details, which can not be decoded, is parsed and decoded twice. I tried to fix this with a check of the encodedDetails in the parse function. However the tests are failling.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

Copy link
Member

@Dudleyneedham Dudleyneedham left a comment

Choose a reason for hiding this comment

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

I think we can use functionality in the library

@@ -107,6 +106,14 @@ export function parse(did: Did | DidUrl): IDidParsingResult {
? parseInt(versionString, 10)
: LIGHT_DID_LATEST_VERSION
const queryParameters = exportQueryParamsFromDidUrl(did as DidUrl)
// checking if encodedDetails can be decoded
if (encodedDetails) {
try {
Copy link
Member

Choose a reason for hiding this comment

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

The library has a validation function. We should be able to use that. @rflechtner What do you think?

base58Validate(encodedDetails, true)

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