search

build-trust / did

A golang package to work with Decentralized Identifiers (DIDs)
https://w3c-ccg.github.io/did-spec
Apache License 2.0
91 stars 21 forks source link

Comply with the new DID ABNF #8

Closed mrinalwadhwa closed 4 years ago

mrinalwadhwa commented 5 years ago

There is ongoing discussion around updating the DID ABNF that we would need to update the parser for

https://github.com/w3c-ccg/did-spec/pull/168 https://pr-preview.s3.amazonaws.com/w3c-ccg/did-spec/pull/168.html

screen shot 2019-02-21 at 7 56 50 pm
mrinalwadhwa commented 5 years ago

The DID part looks unaffected. https://github.com/ockam-network/did/blob/bdd1431c112d9fe2579a0d4ae73e680d196146b6/did.abnf#L10-L16

mrinalwadhwa commented 5 years ago

did-reference will need some tweaks.

https://github.com/ockam-network/did/blob/bdd1431c112d9fe2579a0d4ae73e680d196146b6/did.abnf#L8

https://github.com/ockam-network/did/blob/bdd1431c112d9fe2579a0d4ae73e680d196146b6/did.abnf#L20-L31

mrinalwadhwa commented 5 years ago

did-fragment is also likely unaffected

petersng commented 4 years ago

Hi, just wondering if this will be updated at some point? Seems like this project has been somewhat abandoned while the DID spec has been updated to 1.0. I've been using this lib to parse and validate my DIDs and it has been working great, except for when I encounter new characters that weren't in the ABNF before. I could submit a simple patch for my case if that helps as I prob wouldn't have time to work on completely updating the spec.

This code works great otherwise, thanks!

mrinalwadhwa commented 4 years ago

Hi @petersng. Glad you found the code useful ❤️ We don't have an immediate need to update so this its low on priority list ... however we would love to get a PR if you or anyone else is interested in working on updating it.

hollyfeld commented 4 years ago

Hi @mrinalwadhwa. I'm not too far off from having a pull request for this, but I do have a couple of questions first:

  1. In the current DID String method, Fragment is only serialized to the string when a Path does not exist. I don't see that restriction in the grammar, and it looks like a did-reference with a Path, Query, and Fragment will parse, but a subsequent call to String on the DID would lose the Fragment data.

Am I reading this wrong, or is there some other reason etc.? Please clarify.

https://github.com/ockam-network/did/blob/863346eac7086a346625dfaf4c461326b2da7cf4/did.go#L101-L105

  1. I've added some parserStep methods that model the existing steps. The code to set the next parserStep gets a bit redundant. A possible solution would be to extract out a method that takes a bitmask with options for transitioning, along with a lambda for validation. Then each parser step configures as needed.

Let me know what you think. I can finish up the current approach and push up and then decide if #2 is worth the effort. Thanks!

mrinalwadhwa commented 4 years ago

Hey @hollyfeld that is awesome! I haven't read the did spec in a some months. I'll give it a quick refresher this afternoon and come back to you with answers. Thank you for working on a PR.

hollyfeld commented 4 years ago

Hi @mrinalwadhwa,

I went ahead and generated a PR here. On the previous questions, I went ahead and removed the constraint on not allowing a fragment when a path or segments exist.

The second item I would just ignore. I looked at it again and it didn't seem worth the trouble. Thanks!

mrinalwadhwa commented 4 years ago

Awesome. Thank you 🙏. I'll read through the changes tonight.

mrinalwadhwa commented 4 years ago

@hollyfeld Thank you for sending the PR, it's merged! 🎉