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 urlFromDid() logic to correctly handle path segments #19

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

zscott
Copy link
Contributor

@zscott zscott commented Sep 21, 2022

Wrote a few tests and implemented logic to support optional paths according to Web DID Method Specification.

Previously the urlFromDid() logic supported:
did:web:domain/path/subpath -> https://domain/path/subpath

This is an invalid DID.

With this PR urlFromDid() supports:
did:web:domain:path:subpath -> https://domain/path/subpath/did.json

@dmitrizagidulin
Copy link
Member

@zscott thank you so much for the PR.
In case you're curious why it was handling / path segments before -- I think I was front-running the discussion on issue w3c-ccg/did-method-web#52 (I expected the spec to go in the direction where did:web:domain/path/subpath was a valid DID).
But, as you can probably tell by the length of that issue, there's no community consensus for doing that.

So, you're absolutely right to want to fix it so that it matches the current spec. I'll make a new release.

Copy link
Member

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

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

👍

@dmitrizagidulin dmitrizagidulin merged commit 8268df2 into interop-alliance:main Sep 21, 2022
@dmitrizagidulin
Copy link
Member

@zscott - @interop/[email protected] published to npm.

@zscott
Copy link
Contributor Author

zscott commented Sep 21, 2022

@dmitrizagidulin thanks for the incredibly quick response! I agree with you that / makes more sense for delimiting paths, but my focus is on getting something to work in an interoperable way. I appreciate the merge. Thanks!

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