Skip to content

Commit

Permalink
Fix rename_file to handle relative paths properly (related to GHSA-v7…
Browse files Browse the repository at this point in the history 
…vq-3x77-87vg?) (#6609)

* 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>
  • Loading branch information
@yacchin1205
yacchin1205 and yacchin1205 authored Dec 9, 2022
1 parent 9affa19 commit 64e7c06
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 95 deletions.
12 changes: 6 additions & 6 deletions notebook/services/contents/filemanager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -575,16 +575,16 @@ 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)

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}')
Expand Down
147 changes: 58 additions & 89 deletions notebook/services/contents/tests/test_manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 64e7c06

Please sign in to comment.