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
daviddias merged 1 commit into
masterfrom
fix/multiaddr-validation
Feb 18, 2018
Merged

Better validation of Multiaddrs in bootstrap command#1227
daviddias merged 1 commit into
masterfrom
fix/multiaddr-validation

Conversation

@victorb

@victorb victorb commented Feb 18, 2018

Copy link
Copy Markdown
Member

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

@daviddias daviddias left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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