-
-
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
fix(p2p): issue #5523 #5529
fix(p2p): issue #5523 #5529
Conversation
8fe7465
to
c654cb0
Compare
@Stebalien @Kubuxu Could u help me review it? |
core/commands/p2p.go
Outdated
func checkPort(target ma.Multiaddr) error { | ||
addr := target.String() | ||
var sport string | ||
if strings.Contains(addr, "tcp") { |
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.
Instead of doing this I would use ValueForProtocol
from Multiaddr
https://github.com/multiformats/go-multiaddr/blob/master/interface.go#L44
directly.
It will return ErrProtocolNotFound
if there is no such protocol in multiaddr.
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.
@Kubuxu I had modified it , could you help me review it again?
c654cb0
to
fbae26e
Compare
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.
Thanks! Can you try adding a sharenss case for this - https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0180-p2p.sh#L60-L80 + ipfs p2p ls
?
if sport != "" { | ||
return sport, nil | ||
} | ||
return "", fmt.Errorf("address does not contain tcp or udp protocol") |
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.
We'll want to support unix sockets in the future, but until that happens this can probably stay here
9278867
to
1518873
Compare
@magik6k I had added test, could u help me review it? |
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
;wq Signed-off-by: Overbool <overbool.xu@gmail.com> License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
1518873
to
717c0c8
Compare
@magik6k @Stebalien I had resolved the conflict, could u help me review it? |
Fixes: #5523
License: MIT
Signed-off-by: Overbool overbool.xu@gmail.com