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

Multiaddress validation by IR #557

Closed
carpawell opened this issue May 25, 2021 · 3 comments · Fixed by nspcc-dev/neofs-spec#37
Closed

Multiaddress validation by IR #557

carpawell opened this issue May 25, 2021 · 3 comments · Fixed by nspcc-dev/neofs-spec#37
Assignees
Labels
neofs-ir Inner Ring node application issues

Comments

@carpawell
Copy link
Member

carpawell commented May 25, 2021

In #549 there was a chain of actions: TLS was added => changed the argument of client cache's Get() method from string to network.Address to define what exactly connection should be created => changed client cache's key's format from "HostString" to multiaddr => "/dns4/s01.neofs.devenv/tcp/8080/tls" and "/tls/dns4/s01.neofs.devenv/tcp/8080" are logically the same keys(and both are correct for go-multiaddr library), but there will be two clients created for them

Therefore, it should be discussed, if IR should validate(to be in the same format) multiaddress on bootstrap or not.

@carpawell carpawell added triage neofs-ir Inner Ring node application issues labels May 25, 2021
@alexvanin
Copy link
Contributor

Another option is to define canonical multiaddress field order and convert all address to it locally in client cache.

@fyrchik
Copy link
Contributor

fyrchik commented May 27, 2021

I think the former is correct and we can (should) disallow the latter. I haven't found an explicit description in a doc, but
Encapsulate method appends wrapped protocol to the end https://github.com/multiformats/go-multiaddr/blob/master/multiaddr.go#L140
Here is example in doc https://github.com/multiformats/go-multiaddr/blob/master/doc.go#L29

Second example (/tls/dns4/...) means something like "ip4 over TLS" which is certainly not what we want. It is not difficult to validate address on a client especially when most of the parsing will be hidden in a SDK.

@alexvanin
Copy link
Contributor

alexvanin commented Jun 11, 2021

  • Define expected format of multi address in neofs-spec (@realloc will find best place where to put it).
  • Implement sanity check of multiaddress according to this format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-ir Inner Ring node application issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants