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: use URI encoded host in web-did-doc-router #384

Merged
merged 1 commit into from
Mar 26, 2021
Merged

fix: use URI encoded host in web-did-doc-router #384

merged 1 commit into from
Mar 26, 2021

Conversation

sk91
Copy link
Contributor

@sk91 sk91 commented Feb 15, 2021

Closes #383

@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #384 (70c0e09) into next (0bf7b24) will decrease coverage by 1.33%.
The diff coverage is 59.45%.

@@            Coverage Diff             @@
##             next     #384      +/-   ##
==========================================
- Coverage   67.58%   66.24%   -1.34%     
==========================================
  Files          62       64       +2     
  Lines        1530     1570      +40     
  Branches      247      254       +7     
==========================================
+ Hits         1034     1040       +6     
- Misses        400      428      +28     
- Partials       96      102       +6     

@mirceanis
Copy link
Member

mirceanis commented Feb 16, 2021

Thank you for the contribution.

This depends on how the did:web spec will evolve, after which the web-did-resolver will also have to be upgraded to match the spec.
It is very likely that the proposal won't pass, because the % character is not part of the method-specific-id portion of the DID syntax.
I will mark this as blocked until the web-did spec and the resolver are upgraded.

Also, keep in mind that DIDs such as did:web:localhost%3A8080 will still need to be resolved using https, so you will need a valid certificate for localhost

@sk91
Copy link
Contributor Author

sk91 commented Feb 17, 2021

Yeh, it seems like I was to quick to act here :)

@mirceanis
Copy link
Member

Yeh, it seems like I was to quick to act here :)

I didn't mean to put a wrench in your machine :)
Your contribution is very much appreciated and I look forward to your next proposals!

We are fully aware that all the specs we're working against are in flux, so one of the goals of this framework is to be an enabler for quick experimentation, like the proposal you made here.

mirceanis added a commit that referenced this pull request Mar 26, 2021
BREAKING CHANGES:
* `IResolver` interface adapted to core spec
* No more `EcdsaSignature` for signers, only `string` output
* Signers must accept `Uint8Array` input as well as `string`
* Using EcdsaSecp256k1VerificationKey2019 and ...RecoveryMethod2020
* using `verificationMethod` and `blockchainAccountId` from DID documents
* deprecated `UniversalResolver` class in favor of `getUniversalResolver` method

closes #418
closes #428
closes #417
closes #416
closes #412
closes #397
unblocks #384
fixes #394
@mirceanis mirceanis removed the blocked label Mar 26, 2021
Copy link
Member

@mirceanis mirceanis left a comment

Choose a reason for hiding this comment

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

This was recently unblocked so it makes sense to merge it now.
Thank you for your contribution!

@mirceanis mirceanis merged commit 37186d5 into decentralized-identity:next Mar 26, 2021
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.

remote server does not support web did with port
2 participants