Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rename_file to handle relative paths properly (related to GHSA-v7vq-3x77-87vg?) #6609

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Fix rename_file to handle relative paths properly (related to GHSA-v7vq-3x77-87vg?) #6609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d610672
Fix the path form for rename_file
yacchin1205 Nov 1, 2022
f198622
Fix tests for rename_file to give values in relative paths
yacchin1205 Nov 1, 2022
bd41193
Fix tests to be in line with jupyter-server
yacchin1205 Nov 18, 2022
b5244b4
Fix for determining whether a file is hidden and tests for delete_file
yacchin1205 Dec 7, 2022
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: 6 additions & 6 deletions notebook/services/contents/filemanager.py
View file
Open in desktop
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
Open in desktop
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