Added a flag to allow access of hidden files #2819

danpf · 2017-08-30T17:37:50Z

I thought this flag would be useful. However, looking back it looks like there was some discussion as to if this should ever be allowed. So not a big deal if you don't want it in, but since I've been using this for myself for a while I figured I'd at least give it a shot.

The flag '--allow-hidden' will allow Tornado to access hidden files
such as '.images/my_img.jpg'

relevant threads:
issue:#2659
jupyterlab/jupyterlab#2910
ipython/ipython#5157

Sorry for the multiple commits, didn't think to test something other than jupyter notebook originally

Fix how we aren't checking if a file is under the root directory.
~~Reduce option priority.~~ I don't think this makes much sense considering how much had to be changed.

The flag '--allow-hidden' will allow Tornado to access hidden files such as '.images/my_img.jpg'

Jupyterlab stores its options in a different location than the standard notebook. Added the ability to check there as well.

Previously I was accessing the settings dict based on the name of the app that was being used. ex 'NotebookApp', or 'LabApp'. Now the setting is passed directly into the Tornado settings, and can be accessed via a more general method.

Fixed broken unit tests by setting the default to allow_hidden=False then added unit test in FilesTest:test_hidden_files that checks for the accessibility of files with allow_hidden=True

takluyver · 2017-09-12T11:14:24Z

Thanks! I think this makes sense. We're just trying to release notebook 5.1, so we'll review this more closely once we've got that out.

takluyver · 2017-09-15T09:52:02Z

5.1 is now out, so we can get back to this. :-)

@minrk @gnestor @Carreau any thoughts on whether/how this should be done?

minrk

Makes sense to me. I just have minor questions inline about how/where it should be exposed, but in general I think it's a good idea.

minrk · 2017-09-15T14:27:21Z

notebook/notebookapp.py

@@ -445,6 +445,11 @@ def start(self):
    _("Allow the notebook to be run from root user.")
 )

+flags['allow-hidden']=(


I don't believe this argument is of sufficient priority that it should get this level of promotion. There's a cost to adding noise here. All options are always accessible via lower level config, e.g. --NotebookApp.allow_hidden=True.

That's a good option. The only thing that is weird about it is that it's kind of unintuitive to type jupyter lab --NotebookApp.allow_hidden=True when you're actually launching the LabApp. Although the back end is still notebook, it might seem weird to a user to have to type that when launching a different app (or they might not even think twice about it).

Alternatively, a user probably wouldn't have come across this option unless they knew what they were doing, so they might understand why it's a NotebookApp option.

minrk · 2017-09-15T14:28:39Z

notebook/notebookapp.py

@@ -550,6 +555,10 @@ def _default_log_format(self):
        help=_("Whether to allow the user to run the notebook as root.")
    )

+    allow_hidden = Bool(False, config=True,


It seems like this would more logically be on FileContentsManager, rather than NotebookApp. What do you think?

Interestingly, the abstract ContentsManager (which doesn't know about files) defines an is_hidden() method, but doesn't appear to use it for anything. It's FileContentsManager which is deciding not to show hidden files.

Hrm I think, for my original intentions, it would be mildly confusing to put it in FileContentsManager.

FileContentsManager controls the ability to travel to hidden directories.

AuthenticatedFileHandler controls the ability to access hidden files in markdown (maybe more?)

TreeHandler controls the ability to access hidden files in the filebrowser

For my personal use, this PR was to allow access to hidden files through markdown, not to allow travel to hidden directories/open hidden files. But issue #2659 wanted that ability, so I'm not sure what the best course of action is.

If you think I should include the ability to travel to hidden directories and access hidden files, I can though.

Carreau · 2017-09-15T15:34:05Z

I'm happy to have that as an option. We have to be careful, I believe some of the "is_hidden" logic does (used to ?) do two things; a file is hidden if either:

somewhere in its path the name starts with .
its abspath (after resolving links) is outside the server root.

I think we still want the files outside of the root to be unaccessible. In which case this implementation is not totally correct.

danpf · 2017-09-15T16:57:31Z

I think we still want the files outside of the root to be unaccessible. In which case this implementation is not totally correct.

That's a good point, I will think of a way to change that.

edit:
I just tested it, and it doesn't look like I can access files outside of the root directory with this flag on. Tornado.web is unable to find something outside of the mount locations. So I think we're okay on that front.

minrk · 2017-09-18T08:15:06Z

its abspath (after resolving links) is outside the server root.

I don't think this has ever been true. I believe we've always allowed the use of symlinks to serve files outside root (my default notebooks dir is just a folder full of symlinks to elsewhere).

gnestor · 2017-10-09T18:58:09Z

Is this ready to merge? Or should we mark it as 5.3?

danpf · 2017-10-09T19:17:20Z

I don't know exactly how to proceed. I mentioned this above, but I'll summarize:

For my personal use, this PR was to allow access to hidden files through markdown, not to allow travel to hidden directories/open hidden files in the filebrowser.

In order to add that ability you would have to add an option to the filebrowser to view these, you pretty much only have to edit the function is_hidden in utils.

Editing that is relatively easy and straightforward, but i feel like the filebrowser would be super clogged with tons of dotfiles, and could be annoying unless there's a gui option on the filebrowser for showing hidden files, I think it would have to be a toggle specific to TreeHandler, could be wrong, once I start I'll double check.

Do you think I should go ahead with that instead of my original proposal?

takluyver · 2017-10-10T15:40:34Z

Yeah, there's really two things to control: allowing access to hidden files, and showing them in the list. I think it's fine to add an option to allow access but leave them hidden. Showing them is more complex, because you probably want that to be switchable at runtime, not just from a config file.

It sounds like @gnestor is getting ready to release 5.2 in the next couple of days, so I think this PR may need to wait for 5.3.

gnestor · 2017-11-03T15:40:33Z

Proposal: Add a "Toggle hidden files" item to the View menu?

Previously allow-hidden flag only allowed hidden files to be accessed via tornado. Now you can use the allow-hidden flag to access hidden directories and files via the filebrowser.

danpf · 2017-11-09T00:28:47Z

I don't know javascript at all. I tried for a little bit, but I couldn't figure it out. Maybe someone else can help on that part?

In any case, the --allow-hidden flag now should let you see hidden files in the filebrowser, and travel to hidden directories. You also still cannot create hidden files via the filemanager. I don't know how to move variables between JS and python, but it looks relatively simple if you could.

let me know if the implementation is poor!

…ter test

takluyver · 2018-01-05T12:41:27Z

I've had a go at simplifying this:

Condensed to one attribute ContentsManager.allow_hidden. Configurable, default False.
For now, setting it to True allows access to hidden files but doesn't show them in the listing (so they're still hidden). We can think about showing them in the list as a separate step after this.
At @minrk's suggestion, I've removed the command line flag. It's still possible to specify it at the command line by using the full name: --ContentsManager.allow_hidden=True .

gnestor · 2018-01-05T23:28:36Z

Thanks @takluyver! Should we try to include this in 5.3?

takluyver · 2018-01-06T13:35:01Z

I'd say go for it - it should be low risk.

danpf added 4 commits August 30, 2017 10:02

Added a flag to allow access of hidden files

8355477

The flag '--allow-hidden' will allow Tornado to access hidden files such as '.images/my_img.jpg'

Fixed jupyterlab not following allow-hidden

a0dc62c

Jupyterlab stores its options in a different location than the standard notebook. Added the ability to check there as well.

Updated implementation for any app

426ce31

Previously I was accessing the settings dict based on the name of the app that was being used. ex 'NotebookApp', or 'LabApp'. Now the setting is passed directly into the Tornado settings, and can be accessed via a more general method.

Added/fixed unit tests for test_hidden_files

3426610

Fixed broken unit tests by setting the default to allow_hidden=False then added unit test in FilesTest:test_hidden_files that checks for the accessibility of files with allow_hidden=True

takluyver modified the milestone: 5.2 Sep 12, 2017

minrk reviewed Sep 15, 2017

View reviewed changes

blink1073 mentioned this pull request Oct 3, 2017

Option to display hidden files in the file browser side panel jupyterlab/jupyterlab#2049

Closed

gnestor modified the milestones: 5.2, 5.3 Oct 9, 2017

danpf added 2 commits November 8, 2017 15:55

allow-hidden now works everywhere

d4c9f86

Previously allow-hidden flag only allowed hidden files to be accessed via tornado. Now you can use the allow-hidden flag to access hidden directories and files via the filebrowser.

merge with newer master

dfbdf6c

takluyver added 6 commits January 5, 2018 11:57

Remove --allow-hidden alias

8f8dab6

Move allow_hidden option onto ContentsManager

48ab999

Use try/finally to ensure allow_hidden option is set back to False af…

c4625e5

…ter test

Allow access to hidden files, but don't list them for now

55f67cf

Simplify hidden check for listing again

f833015

Fix indentation

0725830

minrk approved these changes Jan 5, 2018

View reviewed changes

gnestor modified the milestones: 5.3, 5.4 Jan 5, 2018

gnestor modified the milestones: 5.4, 5.3 Jan 7, 2018

gnestor merged commit 605eaa7 into jupyter:master Jan 7, 2018

takluyver mentioned this pull request Jan 30, 2018

Enhancement: Option to allow serving from hidden directory #2659

Closed

github-actions bot added the status:resolved-locked label Apr 3, 2021

github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a flag to allow access of hidden files #2819

Added a flag to allow access of hidden files #2819

danpf commented Aug 30, 2017 •

edited

Loading

takluyver commented Sep 12, 2017

takluyver commented Sep 15, 2017

minrk left a comment

minrk Sep 15, 2017

danpf Sep 15, 2017 •

edited

Loading

minrk Sep 15, 2017

takluyver Sep 15, 2017

danpf Sep 15, 2017 •

edited

Loading

Carreau commented Sep 15, 2017

danpf commented Sep 15, 2017 •

edited

Loading

minrk commented Sep 18, 2017

gnestor commented Oct 9, 2017

danpf commented Oct 9, 2017

takluyver commented Oct 10, 2017

gnestor commented Nov 3, 2017

danpf commented Nov 9, 2017 •

edited

Loading

takluyver commented Jan 5, 2018

gnestor commented Jan 5, 2018

takluyver commented Jan 6, 2018

Added a flag to allow access of hidden files #2819

Added a flag to allow access of hidden files #2819

Conversation

danpf commented Aug 30, 2017 • edited Loading

takluyver commented Sep 12, 2017

takluyver commented Sep 15, 2017

minrk left a comment

Choose a reason for hiding this comment

minrk Sep 15, 2017

Choose a reason for hiding this comment

danpf Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

minrk Sep 15, 2017

Choose a reason for hiding this comment

takluyver Sep 15, 2017

Choose a reason for hiding this comment

danpf Sep 15, 2017 • edited Loading

Choose a reason for hiding this comment

Carreau commented Sep 15, 2017

danpf commented Sep 15, 2017 • edited Loading

minrk commented Sep 18, 2017

gnestor commented Oct 9, 2017

danpf commented Oct 9, 2017

takluyver commented Oct 10, 2017

gnestor commented Nov 3, 2017

danpf commented Nov 9, 2017 • edited Loading

takluyver commented Jan 5, 2018

gnestor commented Jan 5, 2018

takluyver commented Jan 6, 2018

danpf commented Aug 30, 2017 •

edited

Loading

danpf Sep 15, 2017 •

edited

Loading

danpf Sep 15, 2017 •

edited

Loading

danpf commented Sep 15, 2017 •

edited

Loading

danpf commented Nov 9, 2017 •

edited

Loading