window.ipfs.files.cp and files.mv scoping bypass

[lidel]

(Below A and B are reproducible with ipfs-companion v2.4.2)

---

Bugs related to createSrcDestPre hook in Companion

Bug A: passing source and destination without wrapping them in array bypasses scoping

window.ipfs.files.cp('/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX', '/destination.jpg')
// → No error, but file is created outside the sandbox, at real /destination.jpg

createSrcDestPre.args.before: ["/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX","/destination.jpg"] 
createSrcDestPre.appPath:     '/dapps/https/www.wikipedia.org/'
// no destination prefix:
createSrcDestPre.args.after:  ["/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX","/destination.jpg"]

How to fix?

• A.a) Always require sources and destination wrapped in array (throw error if !Array.isArray(args[0]))
• A.b) Add support for both versions?
  • Fixed in 109565f

Bug B: copying from /ipfs/ is broken due to unnecessary prefixing (passing source and destination in array)

window.ipfs.files.cp(['/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX', '/destination.jpg'])
// → Error: file does not exist

createSrcDestPre.args.before: [["/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX","/destination.jpg"]] 
createSrcDestPre.appPath:     '/dapps/https/www.wikipedia.org/'
// the /ipfs/ source got (unnecessarily) prefixed, which produced error above:
createSrcDestPre.args.after:  [["/dapps/https/www.wikipedia.org/ipfs/QmWfVY9y3xjsixTgbd9AorQxH7VtMpzfx2HaWtsoUYecaX","/dapps/https/www.wikipedia.org/destination.jpg"]]

How to fix?

• Do not prefix if source is a valid /ipfs/ path (validate CID, if invalid, then prefix)?
  • Fixed in e16709c

Problem with tests/docs for files.cp and files.mv

I noticed we test different API calls to files.cp and files.mv than ones in docs.

Right now only array-wrapped version is being tested:

• interface-ipfs-core/js/src/files/cp.js
• interface-ipfs-core/js/src/files/mv.js
• js-ipfs-api/test/files.spec.js
• ipfs-companion/test/functional/lib/ipfs-proxy/pre-mfs-scope.test.js

.. but we advertise unwrapped version it in the SPEC docs:

• interface-ipfs-core/SPEC/FILES.md#filescp
• interface-ipfs-core/SPEC/FILES.md#filesmv

This means users probably run version that we don't have real tests for (at least that is the case for MFS exposed via window.ipfs). In most cases it does not matter, but there are edge cases such as window.ipfs where it makes a difference (see bug A vs B above)

@alanshaw I would appreciate some feedback and sanity check on how to proceed. Should we add tests to interface-ipfs-core for both versions (implies A.b), refuse non-wrapped version and change SPEC docs (A.a), or maybe I misunderstood a deeper problem entirely?

[alanshaw]

@lidel

A) The syntax recently changed to remove passing an array. Array syntax is deprecated but currently will work in js-ipfs-api - we need to update the code in companion that does the prefixing to either allow for both versions, or just the non array syntax

B) Yes that's a bug

[lidel]

@alanshaw ack, added fixes and deprecation warning in #531, please take a look in spare moment

[lidel]

Fixed in #531