Apply PR #6609 to 6.5.x (Fix rename_file and delete_file to handle hidden files properly) (#6660)

* Fix the path form for rename_file

* Fix tests for rename_file to give values in relative paths

* Fix tests to be in line with jupyter-server

* Fix for determining whether a file is hidden and tests for delete_file

Co-authored-by: yacchin1205 <968739+yacchin1205@users.noreply.github.com>
This commit is contained in:
Satoshi Yazawa 2022-12-13 03:46:45 +09:00 committed by GitHub
parent 047f69f328
commit e9e5a93dc8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 63 additions and 94 deletions

View File

@ -526,14 +526,14 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
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 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
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 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
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}')

View File

@ -185,37 +185,26 @@ class TestFileContentsManager(TestCase):
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 @@ class TestFileContentsManager(TestCase):
#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"
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"
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)
@skipIf(sys.platform.startswith('win'), "Can't test hidden files on Windows")
def test_404(self):