Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

add more error handling for malformed bootstrap multiaddr #74

Merged
merged 5 commits into from
Apr 12, 2018

Conversation

zcstarr
Copy link
Contributor

@zcstarr zcstarr commented Mar 1, 2018

The intent here is to allow downstream consumers to just log bad multiaddrs. issue: #44

@coveralls
Copy link

coveralls commented Mar 1, 2018

Coverage Status

Coverage increased (+2.9%) to 87.5% when pulling aa631ad on zcstarr:master into 92035a4 on libp2p:master.

@richardschneider
Copy link

A unit test or two would help.

@zcstarr
Copy link
Contributor Author

zcstarr commented Mar 1, 2018

@richardschneider good point, went for extending the existing paradigm and added a sample peer list with malformed addresses, PTAL if you don't mind :)

@richardschneider
Copy link

LGTM

@victorb
Copy link
Member

victorb commented Mar 1, 2018

We should be able to use https://github.com/multiformats/js-mafmt here. I think the IPFS format in js-mafmt is the thin-waist format, but /ipfs/QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSoooo4 being valid makes me believe it's not.

@diasdavid do we have any functions anywhere for validating thin-waist formats?


r.start(() => {})

r.once('peer', (peer) => done())
Copy link
Member

Choose a reason for hiding this comment

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

This should check that only valid peers come out.

src/index.js Outdated
const peerIdB58Str = ma.getPeerId()
if (!peerIdB58Str) { return log.error('Unable to resolve bootstrap peer id') }

const peerId = PeerId.createFromB58String(peerIdB58Str)
Copy link
Member

Choose a reason for hiding this comment

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

You can probably simplify this by using mafmt with a isIPFS

@daviddias
Copy link
Member

@zcstarr still on top of this one?

@zcstarr
Copy link
Contributor Author

zcstarr commented Apr 10, 2018

@diasdavid just a heads up, fixed the test timeout.

@daviddias daviddias merged commit f65e1ba into libp2p:master Apr 12, 2018
@ghost ghost removed the status/in-progress In progress label Apr 12, 2018
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.

5 participants