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: did:mailto format per DID-core spec #31

Closed
wants to merge 1 commit into from
Closed

Conversation

Gozala
Copy link
Collaborator

@Gozala Gozala commented Jan 4, 2023

Fix #28

I chose to not do percent encoding because:

  1. I think it's going to be more error prone given that browsers do not escape @ in URIs
  2. did:web uses this kind of encoding as opposed to arguably more intuitive / delimiter

@Gozala Gozala requested review from gobengo and hugomrdias January 4, 2023 19:24
Copy link
Contributor

@gobengo gobengo left a comment

Choose a reason for hiding this comment

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

lgtm. I found myself wondering "what if email local-part is not something simple like 'alice', but instead has more unusual chars in it?". I assume we should expect percent-encoded entities in the 4th ':'-delimited segment of did:mailto:domain:local-part?

i.e.

const didMailto = `did:mailto:web.mail:alice-with-weird-chars`
const parts = didMailto.split(':')
const email = `${decodeUriComponent(parts[2])@${parts[1]}`

Copy link
Contributor

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

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

LGTM

@Gozala Gozala mentioned this pull request Jan 20, 2024
Gozala added a commit that referenced this pull request Jan 22, 2024
Document UCAN spec extensions we use and assume. Based on #31

---------

Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
@Gozala Gozala closed this Jan 22, 2024
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.

accounts spec uses dids with invalid syntax ('@' char)
4 participants