Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions notebook/services/contents/filemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os
import shutil
import stat
import sys
import warnings
import mimetypes
import nbformat
Expand All @@ -18,6 +19,7 @@
from .filecheckpoints import FileCheckpoints
from .fileio import FileManagerMixin
from .manager import ContentsManager
from ...utils import exists

from ipython_genutils.importstring import import_item
from traitlets import Any, Unicode, Bool, TraitError, observe, default, validate
Expand Down Expand Up @@ -220,12 +222,12 @@ def exists(self, path):
"""
path = path.strip('/')
os_path = self._get_os_path(path=path)
return os.path.exists(os_path)
return exists(os_path)

def _base_model(self, path):
"""Build the common base of a contents model"""
os_path = self._get_os_path(path)
info = os.stat(os_path)
info = os.lstat(os_path)
last_modified = tz.utcfromtimestamp(info.st_mtime)
created = tz.utcfromtimestamp(info.st_ctime)
# Create the base model.
Expand Down Expand Up @@ -275,7 +277,7 @@ def _dir_model(self, path, content=True):
continue

try:
st = os.stat(os_path)
st = os.lstat(os_path)
except OSError as e:
# skip over broken symlinks in listing
if e.errno == errno.ENOENT:
Expand All @@ -284,7 +286,9 @@ def _dir_model(self, path, content=True):
self.log.warning("Error stat-ing %s: %s", os_path, e)
continue

if not stat.S_ISREG(st.st_mode) and not stat.S_ISDIR(st.st_mode):
if (not stat.S_ISLNK(st.st_mode)
and not stat.S_ISREG(st.st_mode)
and not stat.S_ISDIR(st.st_mode)):
self.log.debug("%s not a regular file", os_path)
continue

Expand Down
15 changes: 11 additions & 4 deletions notebook/services/contents/tests/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def test_checkpoint_subdir(self):
self.assertEqual(cp_dir, os.path.join(root, cpm.checkpoint_dir, cp_name))
self.assertEqual(cp_subdir, os.path.join(root, subd, cpm.checkpoint_dir, cp_name))

@dec.skip_win32
@dec.skipif(sys.platform == 'win32' and sys.version_info[0] < 3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prior to version 3.2 os.symlink which is required for this test only existed for unix. This enables testing on Python 3 for Windows as all supported versions have this functionality now.

Copy link
Member

Choose a reason for hiding this comment

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

os.symlink exists now, but I think that in most situations it still raises a permission error, because normal users don't have the right to create symlinks. So I suspect that when we next do a build on Windows, this might fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered if that might be the case but I figured if the tests passed on appveyor it would be fine.

That was based on the assumption that most normal Windows users probably wouldn't be running the tests. In that case it would only affect a Windows developer trying to run the tests without admin privileges where it would fail with a permission error. I think that having some testing on Windows/appveyor is probably worth any hassle that might cause?

OT: In the future it should be possible for developers without admin privileges to create symlinks on Windows 10 - https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/#oWkhfiQvGP6wte4V.97

Copy link
Member

Choose a reason for hiding this comment

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

Ah, nice. I guess our build servers have got the 'creators update' and therefore are able to create symlinks. :-)

def test_bad_symlink(self):
with TemporaryDirectory() as td:
cm = FileContentsManager(root_dir=td)
Expand All @@ -120,9 +120,16 @@ def test_bad_symlink(self):
# create a broken symlink
self.symlink(cm, "target", '%s/%s' % (path, 'bad symlink'))
model = cm.get(path)
self.assertEqual(model['content'], [file_model])

@dec.skip_win32
contents = {
content['name']: content for content in model['content']
}
self.assertTrue('untitled.txt' in contents)
self.assertEqual(contents['untitled.txt'], file_model)
# broken symlinks should still be shown in the contents manager
self.assertTrue('bad symlink' in contents)

@dec.skipif(sys.platform == 'win32' and sys.version_info[0] < 3)
def test_good_symlink(self):
with TemporaryDirectory() as td:
cm = FileContentsManager(root_dir=td)
Expand Down Expand Up @@ -224,7 +231,7 @@ def check_populated_dir_files(self, api_path):
self.assertEqual(entry['name'], "nb.ipynb")
complete_path = "/".join([api_path, "nb.ipynb"])
self.assertEqual(entry["path"], complete_path)

def setUp(self):
self._temp_dir = TemporaryDirectory()
self.td = self._temp_dir.name
Expand Down
4 changes: 2 additions & 2 deletions notebook/services/kernels/kernelmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from jupyter_client.multikernelmanager import MultiKernelManager
from traitlets import Bool, Dict, List, Unicode, TraitError, Integer, default, validate

from notebook.utils import to_os_path
from notebook.utils import to_os_path, exists
from notebook._tz import utcnow, isoformat
from ipython_genutils.py3compat import getcwd

Expand Down Expand Up @@ -55,7 +55,7 @@ def _update_root_dir(self, proposal):
if not os.path.isabs(value):
# If we receive a non-absolute path, make it absolute.
value = os.path.abspath(value)
if not os.path.exists(value) or not os.path.isdir(value):
if not exists(value) or not os.path.isdir(value):
raise TraitError("kernel root dir %r is not a directory" % value)
return value

Expand Down
45 changes: 24 additions & 21 deletions notebook/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@
UF_HIDDEN = getattr(stat, 'UF_HIDDEN', 32768)


def exists(path):
"""Replacement for `os.path.exists` which works for host mapped volumes
on Windows containers
"""
try:
os.lstat(path)
except OSError:
return False
return True


def url_path_join(*pieces):
"""Join components of url into a relative url

Expand Down Expand Up @@ -58,26 +69,25 @@ def url2path(url):
pieces = [ unquote(p) for p in url.split('/') ]
path = os.path.join(*pieces)
return path

def url_escape(path):
"""Escape special characters in a URL path

Turns '/foo bar/' into '/foo%20bar/'
"""
parts = py3compat.unicode_to_str(path, encoding='utf8').split('/')
return u'/'.join([quote(p) for p in parts])

def url_unescape(path):
"""Unescape special characters in a URL path

Turns '/foo%20bar/' into '/foo bar/'
"""
return u'/'.join([
py3compat.str_to_unicode(unquote(p), encoding='utf8')
for p in py3compat.unicode_to_str(path, encoding='utf8').split('/')
])

_win32_FILE_ATTRIBUTE_HIDDEN = 0x02

def is_file_hidden_win(abs_path, stat_res=None):
"""Is a file hidden?
Expand All @@ -98,22 +108,15 @@ def is_file_hidden_win(abs_path, stat_res=None):
if os.path.basename(abs_path).startswith('.'):
return True

# check that dirs can be listed
if os.path.isdir(abs_path):
# can't trust os.access on Windows because it seems to always return True
try:
os.stat(abs_path)
except OSError:
# stat may fail on Windows junctions or non-user-readable dirs
return True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed checking for a broken symlink as I think it's better to let the user know and have them deal with it rather than pretend it doesn't exist


win32_FILE_ATTRIBUTE_HIDDEN = 0x02
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the module level global local to this function as it isn't used elsewhere

try:
attrs = ctypes.windll.kernel32.GetFileAttributesW(
py3compat.cast_unicode(abs_path))
py3compat.cast_unicode(abs_path)
)
except AttributeError:
pass
else:
if attrs > 0 and attrs & _win32_FILE_ATTRIBUTE_HIDDEN:
if attrs > 0 and attrs & win32_FILE_ATTRIBUTE_HIDDEN:
return True

return False
Expand Down Expand Up @@ -164,12 +167,12 @@ def is_file_hidden_posix(abs_path, stat_res=None):

def is_hidden(abs_path, abs_root=''):
"""Is a file hidden or contained in a hidden directory?

This will start with the rightmost path element and work backwards to the
given root to see if a path is hidden or in a hidden directory. Hidden is
determined by either name starting with '.' or the UF_HIDDEN flag as
determined by either name starting with '.' or the UF_HIDDEN flag as
reported by stat.

Parameters
----------
abs_path : unicode
Expand All @@ -191,12 +194,12 @@ def is_hidden(abs_path, abs_root=''):
# is_file_hidden() already checked the file, so start from its parent dir
path = os.path.dirname(abs_path)
while path and path.startswith(abs_root) and path != abs_root:
if not os.path.exists(path):
if not exists(path):
path = os.path.dirname(path)
continue
try:
# may fail on Windows junctions
st = os.stat(path)
st = os.lstat(path)
except OSError:
return True
if getattr(st, 'st_flags', 0) & UF_HIDDEN:
Expand Down Expand Up @@ -244,7 +247,7 @@ def to_os_path(path, root=''):

def to_api_path(os_path, root=''):
"""Convert a filesystem path to an API path

If given, root will be removed from the path.
root must be a filesystem path already.
"""
Expand Down