-
Notifications
You must be signed in to change notification settings - Fork 14
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
Check ipfs path with is-ipfs #31
Changes from all commits
b177708
354ef96
c2d0ec1
7af1081
89fec70
51463b9
770b695
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
var DAGNode = require('./dag-node').DAGNode | ||
var Block = require('ipfs-blocks').Block | ||
const DAGNode = require('./dag-node').DAGNode | ||
const Block = require('ipfs-blocks').Block | ||
const isIPFS = require('is-ipfs') | ||
const base58 = require('bs58') | ||
|
||
exports = module.exports = DAGService | ||
|
||
|
@@ -23,10 +25,27 @@ function DAGService (blockService) { | |
// this.addRecursive | ||
|
||
// get retrieves a DAGNode, using the Block Service | ||
this.get = (multihash, callback) => { | ||
if (!multihash) { return callback(new Error('Invalid Key')) } | ||
this.get = function (multihash, callback) { | ||
const isMhash = isIPFS.multihash(multihash) | ||
const isPath = isIPFS.path(multihash) | ||
|
||
if (!isMhash && !isPath) { | ||
return callback(new Error('Invalid Key')) | ||
} | ||
|
||
if (isMhash) { | ||
this.getWith(multihash, callback) | ||
} | ||
|
||
if (isPath) { | ||
var ipfsKey = multihash.replace('/ipfs/', '') | ||
this.getWith(ipfsKey, callback) | ||
} | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems that there is already the formatting happening on line 37 and 41, is this necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood. Nevertheless there are two transformation steps, one before calling the func and inside the func. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took out the buffer encoding on the path to key since you are right, the check in getWith() function will catch it. |
||
this.bs.getBlock(formatted, (err, block) => { | ||
if (err) { return callback(err) } | ||
var node = new DAGNode() | ||
node.unMarshal(block.data) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 @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 :)