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.

Sign up for GitHub

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

Percent-decode each path fragment. #51

Closed
wants to merge 2 commits into from
Closed

Percent-decode each path fragment. #51

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
280e17e
Percent-decode each path fragment.
dmitrizagidulin Dec 14, 2021
d696efb
Update index.html
dmitrizagidulin Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ https://w3c-ccg.github.io/

This specification uses [ReSpec](https://github.com/w3c/respec/) html. Just edit `index.html` and the draft will be rendered correctly.

When it's time to do a static snapshot, we use the ReSpec UI in the browser to export static HTML.
When it's time to do a static snapshot, we use the ReSpec UI in the browser to export static HTML.
17 changes: 15 additions & 2 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,18 @@ <h3>
-> https://w3c-ccg.github.io/user/alice/did.json
</pre>

<pre class="example nohighlight" title="Creating the DID with a path containing a colon (not recommended but allowed by URI rules)">
did:web:example.com/example%3Asubdirectory
dmitrizagidulin marked this conversation as resolved.
Show resolved Hide resolved
-> https://example.com/example:subdirectory/did.json
</pre>

<p>
If an optional port is specified on the domain, the port colon splitting the
host and the port MUST be percent encoded to prevent collision with the path.
</p>

<pre class="example nohighlight" title="Creating the DID with optional path and port">
did:web:example.com%3A3000:user:alice
did:web:example.com%3A3000:user:alice
-> https://example.com:3000/user/alice/did.json
</pre>
</section>
Expand All @@ -309,11 +314,19 @@ <h3>

<ol>
<li>
Remove the <code>did:web:</code> prefix.
</li>
<li>
Replace ":" with "/" in the method specific identifier to obtain the fully
qualified domain name and optional path.
</li>
<li>
If the domain contains a port percent decode the colon.
If the domain contains a port, percent decode the colon.
Copy link
Collaborator

Choose a reason for hiding this comment

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

wonder if we can keep the port part, and scratch the rest of this...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fwiw, the spec already mentions ports ("A port MAY be included and the colon MUST be percent encoded to prevent a conflict with paths.") So this PR is mainly about the path part.

</li>
<li>
Percent-decode each path fragment. (Note that although URI paths are technically
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 removing this change suggestion (move to an issue / tackle in a subsequent pr).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the main point of the PR tho..

allowed to contain <code>:</code> characters, doing so is not recommended, and
may hinder interoperability.)
Comment on lines +327 to +329
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't percent-decoding each path part mean that percent-encoded non-URL-safe characters in the DID would end up not being representable in the HTTPS URL (because they need to be percent-encoded)? These would then have to be double-percent-encoded in the DID.

e.g.
did:example:example.org:dir:space%20bar → https://example.org/dir/space bar.json
did:example:example.org:dir:space%25%20bar → https://example.org/dir/space%20bar.json
while I would think the desired case would be:
did:example:example.org:dir:space%20bar → https://example.org/dir/space%20bar.json

Would it make more sense to just say percent-decode the colon character in each path part, similar to how that is done for the domain part in the previous step? i.e. replace occurrences of %3A with : in each path part.

Maybe saying "path part" instead of "path fragment" could be more clear, so that the term fragment could be used more consistently?

</li>
<li>
Generate an HTTPS URL to the expected location of the DID document by
Expand Down