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

serve local files in notebook-dir #1211

Merged
merged 5 commits into from Jan 6, 2012
Merged

serve local files in notebook-dir #1211

merged 5 commits into from Jan 6, 2012

Conversation

minrk
Copy link
Member

@minrk minrk commented Dec 28, 2011

Files relative to the notebook-dir are available as http://server:port/files/<file>, and can thus be used in Markdown/HTML, etc. When multiple projects are supported, this url will be http://server:port/<project>/files/<file>.

Update: As the project will be prepended to the url when multiple projects are supported, it is recommended that relative urls (files/img.png) are used.

This grants total read access to the contents of the notebook-dir (assuming you already know what file you are looking for). One should be extra careful after this change, because if you run ipython notebook in your home dir, you can visit http://localhost:8888/files/.ssh/id_dsa to view the default location of your ssh private key, and various other potentially sensitive things.

Local file access is protected just like execution, so if you have a password set, you must login before you are able to view local files.

@minrk
Copy link
Member Author

minrk commented Dec 28, 2011

Since we want to divorce the notebook directory from the server directory, it makes sense for this to take into account the notebook id, so the urls should probably be {base_url}/{notebook_id}/files/relative/file/path instead of just {base_url}/local, as different notebooks will soon be able to have different root directories.

@takluyver
Copy link
Member

As a small security measure, I wonder if we should disallow access to hidden files by default, perhaps with a config option to allow it?

@minrk
Copy link
Member Author

minrk commented Dec 28, 2011

Doesn't seem worth it to me, especially since there's no listing mechanism. Running the notebook server from anywhere other than home would make hidden files a complete non-issue. Also remember, if you have access to the files, you have access to a complete running shell.

@minrk
Copy link
Member Author

minrk commented Dec 29, 2011

URLs are per-notebook. This required that notebooks be identifiable as a relative location, which changed the relative path from the notebook page to the static dir. Absolute paths are now used for static files.

@ellisonbg
Copy link
Member

@minrk but aren't notebook IDs transient and reset each time the notebook server restarts? Won't that mean that the URLs have to be updated each time the notebook server is started? Unless I am missing something I think for the single user notebook, we should not use per-notebook URLs for this reason. In my mind the server directory in single user mode is like a "project directory" and that should be used for serving all file for all notebooks at that location.

Another question - is there checks to make sure a user doesn't break out of the directory using ../../..? That would be a serious security risk.

@minrk
Copy link
Member Author

minrk commented Jan 3, 2012

@minrk but aren't notebook IDs transient and reset each time the notebook server restarts?

Yes, though I believe that is only temporary until we redo the persistent URL scheme more carefully.

Won't that mean that the URLs have to be updated each time the notebook server is started?

Yes, if you use absolute URLs. That's why I adjusted the url scheme (unchanged other than the notebook url ends with '/') so that relative URLs can be used. The url files/foo will always work, no matter what you change about the notebook name, location, server session, etc. It will still work if you send it to another machine, etc.

Unless I am missing something I think for the single user notebook, we should not use per-notebook URLs for this reason. In my mind the server directory in single user mode is like a "project directory" and that should be used for serving all file for all notebooks at that location.

You are right, per-project does make more sense. I think what that means is I should revert my adjustment to the url scheme. Do we know what the url-scheme will look like in a multi-project environment? It seems like the project name must simply be prepended to our existing urls, so they become /<project>/notebook, in which case it should be /<project>/files. Is this correct?

Another question - is there checks to make sure a user doesn't break out of the directory using ../../..? That would be a serious security risk.

I am just using the tornado StaticFileHandler, modified only to add authentication, so unless our current '/static' url grants access outside itself (which would be astonishing), it cannot, but I will check to be sure.

adds AuthenticatedFileHandler, which extends StaticFileHandler with
a simple authentication check before providing access to files local
to the notebook dir.
@minrk
Copy link
Member Author

minrk commented Jan 3, 2012

Updated such that urls are now per-project, and the per-notebook logic and relevant changes are removed.

Re: stepping out of the static-dir:

.. is evaluated in the url, prior to handler dispatch, so http://host/files/../foo is evaluated to http://host/foo, which does not exist, and never reaches the file handler. So it should be perfectly safe.

@fperez
Copy link
Member

fperez commented Jan 4, 2012

This PR should probably have also a small change to one of the intro notebooks where we show the Image object for display, illustrating how to access the same file via a URL. Eventually we'll figure out how to render our notebooks as part of our documentation, but in the meantime they're probably the best way for users to get acquainted with the capabilities of the system, so we should illustrate these improvements in the example nbs.

@ellisonbg
Copy link
Member

OK

On Tue, Jan 3, 2012 at 11:46 AM, Min RK
reply@reply.github.com
wrote:

Updated such that urls are now

Re: stepping out of the static-dir:

.. is evaluated in the url, prior to handler dispatch, so http://host/files/../foo is evaluated to http://host/foo, which does not exist, and never reaches the file handler.  So it should be perfectly safe.


Reply to this email directly or view it on GitHub:
#1211 (comment)

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@minrk
Copy link
Member Author

minrk commented Jan 5, 2012

@fperez I made PR #1224 when I was trying to add notes to the tour, because we can't actually put any interactive elements (other than Flash, which unpleasantly swallows all events) until we stop making single-click immediately trigger edit on rendered text cells.

@fperez
Copy link
Member

fperez commented Jan 6, 2012

I've tested it and it looks great to me. So I'd say the only thing left to do is to add a markdown cell along the lines of

Cells can also display local images... (a bit more explanation here, including the security implications)

![logo](files/logo.png)

to one of the introductory notebooks in the examples dir, and we're good to go. This is an excellent improvement, thanks!

@minrk
Copy link
Member Author

minrk commented Jan 6, 2012

note added to tour, including image and video examples, as well as security comment. I should note that the video part of the example will not be playable until PR #1224 is merged.

@fperez
Copy link
Member

fperez commented Jan 6, 2012

Great, thanks! @ellisonbg, unless you have any other concerns, I'm happy with this going in (as well as #1224).

@ellisonbg
Copy link
Member

Merge away!

minrk added a commit that referenced this pull request Jan 6, 2012
Files in notebook-dir are served by the nbserver as `files/<relativepath>`.
@minrk minrk merged commit a50ac36 into ipython:master Jan 6, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Files in notebook-dir are served by the nbserver as `files/<relativepath>`.
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.

None yet

4 participants