Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

Better validation of Multiaddrs in bootstrap command #1227

Merged
merged 1 commit into from
Feb 18, 2018

Conversation

victorb
Copy link
Member

@victorb victorb commented Feb 18, 2018

mafmt handles both instances of multiaddr and multiaddrs in strings.

isMultiaddr only handles instances of multiaddr.

Demonstration:

> const ma = require('multiaddr')

> const mafmt = require('mafmt').IPFS.matches

> mafmt('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
true

> ma.isMultiaddr('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
false

> m = ma('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
<Multiaddr 0400000000061389a503020562 - /ip4/0.0.0.0/tcp/5001/ipfs/Qm>

> mafmt(m)
true

> ma.isMultiaddr(m)
true

mafmt handles both instances of multiaddr and multiaddrs in strings.

isMultiaddr only handles instances of multiaddr.

Demonstration:

```
> const ma = require('multiaddr')

> const mafmt = require('mafmt').IPFS.matches

> mafmt('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
true

> ma.isMultiaddr('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
false

> m = ma('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
<Multiaddr 0400000000061389a503020562 - /ip4/0.0.0.0/tcp/5001/ipfs/Qm>

> mafmt(m)
true

> ma.isMultiaddr(m)
true
```
@ghost ghost assigned victorb Feb 18, 2018
@ghost ghost added the status/in-progress In progress label Feb 18, 2018
@victorb victorb requested a review from daviddias February 18, 2018 17:43
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

LGTM once CI is green

@daviddias daviddias merged commit d9744a1 into master Feb 18, 2018
@daviddias daviddias deleted the fix/multiaddr-validation branch February 18, 2018 20:08
@ghost ghost removed the status/in-progress In progress label Feb 18, 2018
@daviddias
Copy link
Member

Thanks @victorbjelkholm :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants