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

Reorganize, streamline, and extend did:web. #42 #43

Closed
wants to merge 0 commits into from

Conversation

gribneau
Copy link
Contributor

I've reorganized the document, extended the resolution logic, added server
configuration considerations, and updated RFC references.

There are still comments in this document referencing various issues, but I have
moved them to sections dedicated to discussing the issues raised. We should
reach a consensus on paths forward for these. I think most of them have been
resolved by simply recognizing (and calling out) the reality of resolution over
https and can be removed.

index.html Outdated
Comment on lines 206 to 208
slashes. Colons in directories and subdirectories MUST be percent encoded, if
present, to avoid being translated into slashes. A filename MAY optionally be
included as the final section of the colon delimited method specific identifier.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
slashes. Colons in directories and subdirectories MUST be percent encoded, if
present, to avoid being translated into slashes. A filename MAY optionally be
included as the final section of the colon delimited method specific identifier.
slashes. A filename MAY optionally be included as the final section of the colon
delimited method specific identifier. All path fragments (directories and
filenames) MUST be percent encoded.

I think we should emphasize that all characters in path fragments are percent-encoded, not just colons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Colons are a special case. If a filename or path contains a literal :, that would be translated into / if not percent encoded.

If the colon was not our delimiter, it would not need percent encoding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gribneau

If the colon was not our delimiter, it would not need percent encoding.

Ah, we still would, though, because directory and file names can contain characters that are not allowed by the DID spec. So it's not just the colon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The : that appears in the domain segment of the method specific identifier in the case that a port is specified must be percent-encoded to avoid being rewritten as /.

e.g.

did:web:example.com:1443
---> https://example.com/1443/

vs.

did:web:example.com%3A1443
---> https://example.com:1443/.well-known/did/

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

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

👍
Great work, @gribneau! Thank you.

@gribneau
Copy link
Contributor Author

Within the context of this PR, I think it would be beneficial to address a number of issues that have been noted in the text of the specification with an eye to ensuring that our explicit handling of those issues is adequate to remove them from the text and close them.

These appear on lines 462, 466, 487, and 582.

That informational section is intended to address those issues (and others), and avoid making did:web something it cannot be without a dramatic increase in complexity. This method should support DID delivery by static webservers in the simplest possible fashion.

There is also an ongoing discussion about the language describing potential DNS vulnerabilities and mitigation in #45, and we could pick up #46 and #44 as well.

Ideally, I'd like this PR to clean up the text and clear out as many outstanding issues as possible.

index.html Outdated Show resolved Hide resolved
index.html Outdated
did:web:w3c-ccg.github.io
-> https://w3c-ccg.github.io/.well-known/did.json
-> https://w3c-ccg.github.io/.well-known/did/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest we not co-mingle these changes with the other cleanups, also this is a breaking change that would make existing implementations non conformant... cc @kdenhartog

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 we need to break this apart into multiple PRs

index.html Outdated
MIME Types
</h3>
<p>
Webservers MUST be configured to associate proper content types with registered
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to see application/json as the only defined format first... and then attempt to address these other representations separately... too many changes in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In #47, I reduced to application/json and application/did+json to preserve the logic of shifting to index files instead of hardcoded /did.json.

I also removed references to JSON-LD.

This presents us with a path forward to comfortably support other representations in the future without embracing the complexity today.

Copy link
Collaborator

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Let's split out the file system and content type changes from the rest of this PR.

I'm generally in favor of the changes but concerned we are doing too much at once, and especially worried about the interop and breaking changes associated with the proposed representation changes.

Copy link
Contributor

@mprorock mprorock left a comment

Choose a reason for hiding this comment

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

We should break this apart, at least by grammatical cleanup, followed by any PRs that may introduce breaking changes

index.html Outdated
did:web:w3c-ccg.github.io
-> https://w3c-ccg.github.io/.well-known/did.json
-> https://w3c-ccg.github.io/.well-known/did/
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 we need to break this apart into multiple PRs

@gribneau
Copy link
Contributor Author

There is a clean PR at #47 reflecting feedback above.

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.

4 participants