-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
namesys: Make paths with multiple segemnts work. Fixes #2059 #2116
Conversation
1ba22bb
to
309c70a
Compare
Added tests, fixed one more case and rebased. |
ead7be6
to
6dd56b5
Compare
Should I recreate the PR for dev0.4.0 branch? |
@@ -82,12 +82,15 @@ Resolve the value of an IPFS DAG path: | |||
// the case when ipns is resolved step by step | |||
if strings.HasPrefix(name, "/ipns/") && !recursive { | |||
p, err := n.Namesys.ResolveN(req.Context(), name, 1) | |||
|
|||
if p != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont like treating this as a special case... If we call the path resolved, the error should be nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like it too but in other case there is no reason to have resolve without recursion.
You can't use it to do step by step resolving as you just get error of recursion exceeded.
I will remove this from the PR as this is something that might require some refactoring to get right.
I've removed the change to the command. |
Also fixes non-recursive resolve erring instead showing one step. The patch of core/commands/resolve.go could be done better but I don't know how to get access to ErrResolveRecursion. It allows for dnslinks into sub-segments. So for example hosting multiple blogs on just domains from one pubkey. Fixes ipfs#2059 Add tests and fix case when dnslinks references dnslink License: MIT Signed-off-by: Jakub (Kubuxu) Sztandera <kubuxu@gmail.com>
@Kubuxu we don't seem to test for |
Fixed some issues with trailing slashes. License: MIT Signed-off-by: Jakub (Kubuxu) Sztandera <kubuxu@gmail.com>
I've added more tests and resolved issues with trailing slashes. Thanks for pointing this out. I can squash commits if you want. |
no worries about squashing |
@Kubuxu just as a sanity check, you've tested this with real world dns resultion once or twice, right? |
this LGTM |
I did. http://hastebin.com/inorefagip.txt
|
awesome, this LGTM |
namesys: Make paths with multiple segemnts work. Fixes #2059
Also fixes non-recursive resolve erring instead showing one step.
The patch of
core/commands/resolve.go
could be done better but I don't know how to get access toErrResolveRecursion
.It allows for dnslinks into sub-segments. So ie. hosting multiple blogs on just domains from one pubkey.
Fixes #2059