Skip to content

Commit b5b44eb

Browse files
scanontakluyver
andauthored
Fix for recursive symlink issue #4669 (#4670)
* This is a potential fix to issue #4669. The fix simply catches the recusrive symlink error and moves on. It is possibble that the try/except belongs in the utils but I wanted to limit the scope. * It looks like too many levels is error number 40 on linux. * Switch to using errno instead of hardcoding the number. * Fix spelling, smarter assert methods * Log unrecognised errors and continue listing directory * Skip recursive symlink test entirely on Windows * Fix which test is skipped on Windows Co-authored-by: Thomas Kluyver <[email protected]>
1 parent fb08a1b commit b5b44eb

File tree

2 files changed

+33
-5
lines changed

2 files changed

+33
-5
lines changed

notebook/services/contents/filemanager.py

+13-5
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,20 @@ def _dir_model(self, path, content=True):
330330
self.log.debug("%s not a regular file", os_path)
331331
continue
332332

333-
if self.should_list(name):
334-
if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
335-
contents.append(
336-
self.get(path='%s/%s' % (path, name), content=False)
333+
try:
334+
if self.should_list(name):
335+
if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
336+
contents.append(
337+
self.get(path='%s/%s' % (path, name), content=False)
338+
)
339+
except OSError as e:
340+
# ELOOP: recursive symlink
341+
if e.errno != errno.ELOOP:
342+
self.log.warning(
343+
"Unknown error checking if file %r is hidden",
344+
os_path,
345+
exc_info=True,
337346
)
338-
339347
model['format'] = 'json'
340348

341349
return model

notebook/services/contents/tests/test_manager.py

+20
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,26 @@ def test_bad_symlink(self):
129129
# broken symlinks should still be shown in the contents manager
130130
self.assertTrue('bad symlink' in contents)
131131

132+
@dec.skipif(sys.platform == 'win32')
133+
def test_recursive_symlink(self):
134+
with TemporaryDirectory() as td:
135+
cm = FileContentsManager(root_dir=td)
136+
path = 'test recursive symlink'
137+
_make_dir(cm, path)
138+
os_path = cm._get_os_path(path)
139+
os.symlink("recursive", os.path.join(os_path, "recursive"))
140+
file_model = cm.new_untitled(path=path, ext='.txt')
141+
142+
model = cm.get(path)
143+
144+
contents = {
145+
content['name']: content for content in model['content']
146+
}
147+
self.assertIn('untitled.txt', contents)
148+
self.assertEqual(contents['untitled.txt'], file_model)
149+
# recursive symlinks should not be shown in the contents manager
150+
self.assertNotIn('recursive', contents)
151+
132152
@dec.skipif(sys.platform == 'win32' and sys.version_info[0] < 3)
133153
def test_good_symlink(self):
134154
with TemporaryDirectory() as td:

0 commit comments

Comments
 (0)