diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index 646b2b36e0..08e590e64f 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -526,14 +526,14 @@ def delete_file(self, path): os_path = self._get_os_path(path) rm = os.unlink + if is_hidden(os_path, self.root_dir) and not self.allow_hidden: + raise web.HTTPError(400, f'Cannot delete file or directory {os_path!r}') + four_o_four = "file or directory does not exist: %r" % path if not self.exists(path): raise web.HTTPError(404, four_o_four) - if is_hidden(os_path, self.root_dir) and not self.allow_hidden: - raise web.HTTPError(400, f'Cannot delete file or directory {os_path!r}') - def is_non_empty_dir(os_path): if os.path.isdir(os_path): # A directory containing only leftover checkpoints is @@ -575,9 +575,6 @@ def rename_file(self, old_path, new_path): if new_path == old_path: return - if (is_hidden(old_path, self.root_dir) or is_hidden(new_path, self.root_dir)) and not self.allow_hidden: - raise web.HTTPError(400, f'Cannot rename file or directory {old_path!r}') - # Perform path validation prior to converting to os-specific value since this # is still relative to root_dir. self._validate_path(new_path) @@ -585,6 +582,9 @@ def rename_file(self, old_path, new_path): new_os_path = self._get_os_path(new_path) old_os_path = self._get_os_path(old_path) + if (is_hidden(old_os_path, self.root_dir) or is_hidden(new_os_path, self.root_dir)) and not self.allow_hidden: + raise web.HTTPError(400, f'Cannot rename file or directory {old_path!r}') + # Should we proceed with the move? if os.path.exists(new_os_path) and not samefile(old_os_path, new_os_path): raise web.HTTPError(409, f'File already exists: {new_path}') diff --git a/notebook/services/contents/tests/test_manager.py b/notebook/services/contents/tests/test_manager.py index 05043d6888..efb3d1ae95 100644 --- a/notebook/services/contents/tests/test_manager.py +++ b/notebook/services/contents/tests/test_manager.py @@ -185,37 +185,26 @@ def test_403(self): def test_400(self): #Test Delete behavior #Test delete of file in hidden directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = '.hidden' - file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - os_path = cm._get_os_path(model['path']) + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + + with self.assertRaises(HTTPError) as excinfo: + cm.delete_file(file_in_hidden_path) + self.assertEqual(excinfo.exception.status_code, 400) - try: - result = cm.delete_file(os_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") #Test delete hidden file in visible directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = 'visible' - file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - os_path = cm._get_os_path(model['path']) + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) - try: - result = cm.delete_file(os_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") + with self.assertRaises(HTTPError) as excinfo: + cm.delete_file(file_in_hidden_path) + self.assertEqual(excinfo.exception.status_code, 400) #Test Save behavior #Test save of file in hidden directory @@ -253,76 +242,56 @@ def test_400(self): #Test rename behavior #Test rename with source file in hidden directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = '.hidden' - file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - old_path = cm._get_os_path(model['path']) - new_path = "new.txt" + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + old_path = file_in_hidden_path + new_path = "new.txt" - try: - result = cm.rename_file(old_path, new_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") + with self.assertRaises(HTTPError) as excinfo: + cm.rename_file(old_path, new_path) + self.assertEqual(excinfo.exception.status_code, 400) #Test rename of dest file in hidden directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = '.hidden' - file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - new_path = cm._get_os_path(model['path']) - old_path = "old.txt" + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + new_path = file_in_hidden_path + old_path = "old.txt" - try: - result = cm.rename_file(old_path, new_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") + with self.assertRaises(HTTPError) as excinfo: + cm.rename_file(old_path, new_path) + self.assertEqual(excinfo.exception.status_code, 400) #Test rename with hidden source file in visible directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = 'visible' - file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - old_path = cm._get_os_path(model['path']) - new_path = "new.txt" + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + old_path = file_in_hidden_path + new_path = "new.txt" - try: - result = cm.rename_file(old_path, new_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") + with self.assertRaises(HTTPError) as excinfo: + cm.rename_file(old_path, new_path) + self.assertEqual(excinfo.exception.status_code, 400) #Test rename with hidden dest file in visible directory - with self.assertRaises(HTTPError) as excinfo: - with TemporaryDirectory() as td: - cm = FileContentsManager(root_dir=td) - hidden_dir = 'visible' - file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') - _make_dir(cm, hidden_dir) - model = cm.new(path=file_in_hidden_path) - new_path = cm._get_os_path(model['path']) - old_path = "old.txt" - - try: - result = cm.rename_file(old_path, new_path) - except HTTPError as e: - self.assertEqual(e.status_code, 400) - else: - self.fail("Should have raised HTTPError(400)") + with TemporaryDirectory() as td: + cm = FileContentsManager(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + new_path = file_in_hidden_path + old_path = "old.txt" + + with self.assertRaises(HTTPError) as excinfo: + cm.rename_file(old_path, new_path) + self.assertEqual(excinfo.exception.status_code, 400) @skipIf(sys.platform.startswith('win'), "Can't test hidden files on Windows") def test_404(self):