Check ipfs path with is-ipfs #31
Conversation
| dagService.add(node, (err) => { | ||
| expect(err).to.not.exist | ||
| var mh = node.multihash() | ||
| var encodedMh = bs58.encode(mh) |
There was a problem hiding this comment.
Could you use a static string whose value is what you're expecting? This makes the test more explicit and doesn't rely on generated values to test assumptions.
|
Woohoo! I've been wanting this for a while. 💯 |
| this.getWith(ipfsKey, callback) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
👍 @nginnever :D
Can we get all of this part of 'is-ipfs' logic? I'm sure @xicombd would be happy to include it there and it will be useful for a ton of other places.
btw, it would be stellar if mafmt from @whyrusleeping used this to check multiaddrs that are also ipfs multiaddrs with valid multihashes :)
|
Requires ipfs-shipyard/is-ipfs#5 |
|
ipfs-shipyard/is-ipfs#5 has now been merged and published as |
|
|
||
| this.bs.getBlock(multihash, (err, block) => { | ||
| this.getWith = function (key, callback) { | ||
| const formatted = typeof key === 'string' ? new Buffer(base58.decode(key)) : key |
There was a problem hiding this comment.
seems that there is already the formatting happening on line 37 and 41, is this necessary?
There was a problem hiding this comment.
This format check is needed in this implementation. While the 'is-ipfs' module can handle any input, it only returns true or false and does not return a formatted key enforced to be a base58 decoded buffer.
'is-ipfs' could return a formatted key (buffer instead of string) if the key/path are valid multihashes, but then the name of the module wouldn't make much sense any more. And would probably need to be something like 'ipfs-format-key'
There was a problem hiding this comment.
Understood. Nevertheless there are two transformation steps, one before calling the func and inside the func.
There was a problem hiding this comment.
I took out the buffer encoding on the path to key since you are right, the check in getWith() function will catch it.
This adds is-ipfs tests for correct paths and multihashes with is-ipfs.
@diasdavid had discussed adding buffer condition to is-ipfs. This checks the supplied path to the dag-service to see if it is a buffer first, if it is then it base58 encodes the buffer and then tests with is-ipfs.
If the path supplied to dag-service is a string then it checks to see if it is a path or a correct multihash.