Skip to content
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

feat: scope mfs calls to an application path #431

Merged
merged 9 commits into from
Mar 27, 2018

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Mar 23, 2018

https://youtu.be/KvgxAafphPg

See #330 (comment) for discussion.

To avoid conflicts, each app gets it's own MFS directory where it can store files. When using MFS functions (see docs) this directory will be automatically added to paths you pass. Your app's MFS directory is based on the origin and path where your application is running.

e.g.

  • files.write to /myfile.txt on https://domain.com/
    • writes to /dapps/https/domain.com/myfile.txt
  • files.write to /path/to/myfile.txt on https://domain.com/feature
    • writes to /dapps/https/domain.com/feature/path/to/myfile.txt
  • files.read from /feature/path/to/myfile.txt on https://domain.com/
    • reads from /dapps/https/domain.com/feature/path/to/myfile.txt
  • files.stat to / on https://domain.com/feature
    • stats /dapps/https/domain.com/feature
  • files.read from /../myfile.txt on https://domain.com/feature
    • reads from /dapps/https/domain.com/feature/myfile.txt (no traverse above your app's root)

@alanshaw alanshaw changed the title [WIP] feat: adds function to scope mfs calls to an application path [WIP] feat: scope mfs calls to an application path Mar 23, 2018
@alanshaw alanshaw changed the title [WIP] feat: scope mfs calls to an application path feat: scope mfs calls to an application path Mar 23, 2018
@alanshaw alanshaw requested review from olizilla and lidel March 23, 2018 17:22
@alanshaw alanshaw mentioned this pull request Mar 23, 2018
7 tasks
@olizilla
Copy link
Member

This looks great, I love the video. Can we special case the https / http level to remove the colon before url-encoding? We're going to end up presenting these to the user in the new UIs and having the url-encoded version is gonna look surprising.

}

// Turn http://ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn
// into /http%3A/ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn
Copy link
Member

Choose a reason for hiding this comment

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

I agree with #431 (comment), we should add .replace(':\/\/','/') at the beginning of the scopeToPath pipeline to change first (and only first) occurrence of :// into /


module.exports = createPreMfsScope

const MfsPre = {
Copy link
Member

@lidel lidel Mar 25, 2018

Choose a reason for hiding this comment

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

Question; how will it behave when a new API ipfs.files.foo is added ?
Will it not work, or will it work with no path scoping?
If the latter is the case, we should explicitly block executions of new methods in scoped ipfs.files API to avoid surprises in future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I mean, you'd need to update the ipfs-postmsg-proxy dependency to allow the new method to be used in the first place, but no it wouldn't be scoped automatically unless you did some work here.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, being dependent on ipfs-postmsg-proxy makes this safe enough for now.
Unless its trivial to add, I propose we add additional safeguards in separate PR.

@alanshaw
Copy link
Member Author

@lidel good now?

@hacdias
Copy link
Member

hacdias commented Mar 27, 2018

@alanshaw will we have a special case for WebUI? WebUI should be able to access everything. How will we do that?

@alanshaw
Copy link
Member Author

@hacdias yes, if accessed through companion we can privilege the window.ipfs object to not scope MFS or ask permission. We can do the same with desktop if we load WebUI into it.

If you're just loading (our new, yet to be built) WebUI from the gateway in the browser with ipfs companion installed (i.e. https://ipfs.io/ipfs/QmPhnvn747LqwPYMJmQVorMaGbMSgA7mRRoyyZYz3DoZRQ) then MFS stuff will be scoped. You might want to load from /ipns/webui.ipfs.io to get around this, and also the problem of different versions of the app being scoped to different folders.

Check out this comment (and the backstory) for a bit more context: #330 (comment)

@hacdias
Copy link
Member

hacdias commented Mar 27, 2018

@alanshaw gotcha. It makes sense. I like the /ipns/webui.ipfs.io idea. Always the same link and we could easily update the version without waiting for go-ipfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants