Merge pull request #531 from ipfs-shipyard/fix/scope-files-cp

window.ipfs.files.cp|mv fixes

add-on/src/lib/ipfs-proxy/pre-mfs-scope.js

@@ -1,5 +1,6 @@
 // Use path-browserify for consistent behavior between browser and tests on Windows
 const Path = require('path-browserify')
+const IsIpfs = require('is-ipfs')
 const DEFAULT_ROOT_PATH = '/dapps'
 
 // Creates a "pre" function that is called prior to calling a real function
@@ -42,13 +43,60 @@ const MfsPre = {
 // Scope a src/dest tuple to the app path
 function createSrcDestPre (getScope, getIpfs, rootPath) {
   return async (...args) => {
+    // console.log('createSrcDestPre.args.before: ' + JSON.stringify(args))
     const appPath = await getAppPath(getScope, getIpfs, rootPath)
-    args[0][0] = Path.join(appPath, safePath(args[0][0]))
-    args[0][1] = Path.join(appPath, safePath(args[0][1]))
+    // console.log('createSrcDestPre.appPath:     ', appPath)
+    args = prefixSrcDestArgs(appPath, args)
+    // console.log('createSrcDestPre.args.after:  ' + JSON.stringify(args))
     return args
   }
 }
 
+// Prefix src and dest args where applicable
+function prefixSrcDestArgs (prefix, args) {
+  const prefixedArgs = []
+  const destPosition = destinationPosition(args)
+  for (let i = 0; i < args.length; i++) {
+    const item = args[i]
+    if (typeof item === 'string') {
+      const isDestination = (i === destPosition)
+      prefixedArgs[i] = safePathPrefix(prefix, item, isDestination)
+    } else if (Array.isArray(item)) {
+      // The syntax recently changed to remove passing an array,
+      // but we allow for both versions until js-ipfs-api is updated to remove
+      // support for it
+      console.warn('[ipfs-companion] use of array in ipfs.files.cp|mv is deprecated, see https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/FILES.md#filescp')
+      prefixedArgs[i] = prefixSrcDestArgs(prefix, item)
+    } else {
+      // {options} or callback, passing as-is
+      prefixedArgs[i] = item
+    }
+  }
+  return prefixedArgs
+}
+
+// Find the last string argument and save as the position of destination path
+function destinationPosition (args) {
+  let destPosition
+  for (let i = 0; i < args.length; i++) {
+    if (typeof args[i] === 'string') {
+      destPosition = i
+    }
+  }
+  return destPosition
+}
+
+// Add a prefix to a path in a safe way
+function safePathPrefix (prefix, path, isDestination) {
+  const realPath = safePath(path)
+  if (!isDestination && IsIpfs.ipfsPath(realPath)) {
+    // we don't prefix valid /ipfs/ paths in source paths
+    // (those are public and immutable, so safe as-is)
+    return realPath
+  }
+  return Path.join(prefix, realPath)
+}
+
 // Scope a src path to the app path
 function createSrcPre (getScope, getIpfs, rootPath) {
   return async (...args) => {

test/functional/lib/ipfs-proxy/pre-mfs-scope.test.js

@@ -12,7 +12,51 @@ describe('lib/ipfs-proxy/pre-mfs-scope', () => {
     expect(pre).to.equal(null)
   })
 
-  it('should scope src/dest paths for files.cp', async () => {
+  it('should scope (src, dest) paths for files.cp', async () => {
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre('/source.txt', '/destination.txt')
+    expect(args[0]).to.equal('/test-dapps/https/ipfs.io/source.txt')
+    expect(args[1]).to.equal('/test-dapps/https/ipfs.io/destination.txt')
+  })
+
+  it('should scope (src1, src2, destDir) paths for files.cp', async () => {
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre('/source1.txt', '/source2.txt', '/destinationDir/')
+    expect(args[0]).to.equal('/test-dapps/https/ipfs.io/source1.txt')
+    expect(args[1]).to.equal('/test-dapps/https/ipfs.io/source2.txt')
+    expect(args[2]).to.equal('/test-dapps/https/ipfs.io/destinationDir')
+  })
+
+  it('should not scope src path if it is a valid /ipfs/ path for files.cp', async () => {
+    // Bug B from https://github.com/ipfs-shipyard/ipfs-companion/issues/530#issue-341352979
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy', '/destination.jpg')
+    expect(args[0]).to.equal('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+    expect(args[1]).to.equal('/test-dapps/https/ipfs.io/destination.jpg')
+  })
+
+  it('should scope dest path even if it is a valid /ipfs/ path for files.cp', async () => {
+    // https://github.com/ipfs-shipyard/ipfs-companion/pull/531#pullrequestreview-139053285
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy', '/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+    expect(args[0]).to.equal('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+    expect(args[1]).to.equal('/test-dapps/https/ipfs.io/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+  })
+
+  // array-wrapping is deprecated, but we support it until removed from js-ipfs-api
+  it('should scope ([src, dest]) paths for files.cp', async () => {
     const fnName = 'files.cp'
     const getScope = () => 'https://ipfs.io/'
     const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
@@ -22,6 +66,30 @@ describe('lib/ipfs-proxy/pre-mfs-scope', () => {
     expect(args[0][1]).to.equal('/test-dapps/https/ipfs.io/destination.txt')
   })
 
+  // array-wrapping is deprecated, but we support it until removed from js-ipfs-api
+  it('should not scope array-wrapped src path if it is valid /ipfs/ path for files.cp', async () => {
+    // Bug B from https://github.com/ipfs-shipyard/ipfs-companion/issues/530#issue-341352979
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre(['/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy', '/destination.jpg'])
+    expect(args[0][0]).to.equal('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+    expect(args[0][1]).to.equal('/test-dapps/https/ipfs.io/destination.jpg')
+  })
+
+  // array-wrapping is deprecated, but we support it until removed from js-ipfs-api
+  it('should scope array-wrapped dest path even if it is a valid /ipfs/ path for files.cp', async () => {
+    // https://github.com/ipfs-shipyard/ipfs-companion/pull/531#pullrequestreview-139053285
+    const fnName = 'files.cp'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre(['/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy', '/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy'])
+    expect(args[0][0]).to.equal('/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+    expect(args[0][1]).to.equal('/test-dapps/https/ipfs.io/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy')
+  })
+
   it('should scope src path for files.mkdir', async () => {
     const fnName = 'files.mkdir'
     const getScope = () => 'https://ipfs.io/'
@@ -67,7 +135,18 @@ describe('lib/ipfs-proxy/pre-mfs-scope', () => {
     expect(args[0]).to.equal('/test-dapps/https/ipfs.io/path/to/file.md')
   })
 
-  it('should scope src/dest paths for files.mv', async () => {
+  // array-wrapping is deprecated, but we support it until removed from js-ipfs-api
+  it('should scope (src, dest) paths for files.mv', async () => {
+    const fnName = 'files.mv'
+    const getScope = () => 'https://ipfs.io/'
+    const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })
+    const pre = createPreMfsScope(fnName, getScope, getIpfs, '/test-dapps')
+    const args = await pre('/source.txt', '/destination.txt')
+    expect(args[0]).to.equal('/test-dapps/https/ipfs.io/source.txt')
+    expect(args[1]).to.equal('/test-dapps/https/ipfs.io/destination.txt')
+  })
+
+  it('should scope ([src, dest]) paths for files.mv', async () => {
     const fnName = 'files.mv'
     const getScope = () => 'https://ipfs.io/'
     const getIpfs = () => ({ files: { mkdir: () => Promise.resolve() } })