w3c-ccg / did-method-web

DRAFT: did:web Decentralized Identifier Method Specification
https://w3c-ccg.github.io/did-method-web/
Other
32 stars 17 forks source link

update text to specify how ports should be handled #38

Closed kdenhartog closed 3 years ago

kdenhartog commented 3 years ago

fixes #7

mirceanis commented 3 years ago

Looks good, but I think percent encoding should happen regardless of port, to create valid DIDs from hostnames with unicode chars

kdenhartog commented 3 years ago

Looks good, but I think percent encoding should happen regardless of port, to create valid DIDs from hostnames with unicode chars

Do you have an example I should include around this? Are you thinking about percent encoding international Unicode characters?

mirceanis commented 3 years ago

I don't have an example, but I guess this spec should also cover internationalized domain names. Normally they are encoded using punycode, but I suspect that is not as readily available as percent encoding.

gribneau commented 3 years ago

If the domain contains a port percent decode the colon.

If we specify percent encoding as a part of the standard, I think it should be applied generally.

It's been a while, but I do recall some conversation on this topic, and I think views were positive overall.

mirceanis commented 3 years ago

My comment is also based on those discussions. Those discussions also mentioned percent encoding the path components.

kdenhartog commented 3 years ago

Ok, I think I understand. I'll update the text to reflect that the entire domain gets percent encoded. I'm less certain we want to do this with paths though because percent encoding can pick up / which could potentially lead to weird behaviors. How about we modify this text for domains only for now and we can discuss paths further to come up with a solid solution for how that might work.

mirceanis commented 3 years ago

I should have been more clear when i mentioned path components. I meant the sections of the path string that go between the slashes :) But I agree, this can be done later.

kdenhartog commented 3 years ago

3 months later and I still haven't finished updating this PR with the suggestions from @mirceanis

I'm aware of what he means now after our team implemented this further. I just need to update the text to make it more clear. I'll add this to my list of higher priority things to get finished up next week.

I'm going to open a second PR to add that additional text instead.

OR13 commented 3 years ago

When can we merge this? would the folks commenting here, please approve or request changes?

dmitrizagidulin commented 3 years ago

Merging this PR, as it's an important detail to have in the spec. We'll address percent-encoding path fragments in a separate PR.