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

Allow resolving did:webs with ports #519

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

TaylorBeeston
Copy link
Contributor

A few months ago, while maintaining the fork for LEF, I noticed that did:web's with ports currently aren't being resolved! The spec states to percent encode the colon as %3A for ports, so I added that in!

This is particularly useful when hosting dids locally and trying to resolve a did:web:localhost%3A3000 did during development. I also accounted for this by adjusting the http check from domain_name == "localhost" to domain_name.starts_with("localhost"), thereby allowing the domain localhost%3A3000 to still use http rather than https.

@CLAassistant
Copy link

CLAassistant commented Jul 13, 2023

CLA assistant check
All committers have signed the CLA.

@obstropolos obstropolos requested a review from sbihel July 13, 2023 17:39
@TaylorBeeston
Copy link
Contributor Author

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

This is already done, and I've already got two other PRs merged here! (https://github.com/spruceid/ssi/pulls?q=is%3Apr+author%3ATaylorBeeston+is%3Aclosed)

(see #442 (comment))

@sbihel sbihel merged commit db906df into spruceid:main Jul 14, 2023
@sbihel
Copy link
Member

sbihel commented Jul 14, 2023

Thank you for the PR

@TaylorBeeston TaylorBeeston deleted the didweb-port-support branch July 14, 2023 21:33
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.

3 participants