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.

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 the non-writable path deletion error #670

Merged
merged 1 commit into from
Jan 25, 2022
Merged
Changes from all commits
Commits
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
40 changes: 31 additions & 9 deletions jupyter_server/services/contents/filemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,28 @@ def is_hidden(self, path):
os_path = self._get_os_path(path=path)
return is_hidden(os_path, self.root_dir)

def is_writable(self, path):
"""Does the API style path correspond to a writable directory or file?

Parameters
----------
path : string
The path to check. This is an API path (`/` separated,
relative to root_dir).

Returns
-------
hidden : bool
Whether the path exists and is writable.
"""
path = path.strip("/")
os_path = self._get_os_path(path=path)
try:
return os.access(os_path, os.W_OK)
except OSError:
self.log.error("Failed to check write permissions on %s", os_path)
return False

def file_exists(self, path):
"""Returns True if the file exists, else returns False.

Expand Down Expand Up @@ -251,12 +273,8 @@ def _base_model(self, path):
model["format"] = None
model["mimetype"] = None
model["size"] = size
model["writable"] = self.is_writable(path)

try:
model["writable"] = os.access(os_path, os.W_OK)
except OSError:
self.log.error("Failed to check write permissions on %s", os_path)
model["writable"] = False
return model

def _dir_model(self, path, content=True):
Expand Down Expand Up @@ -514,10 +532,12 @@ def is_non_empty_dir(os_path):
# deleting non-empty files. See Github issue 3631.
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
if _check_trash(os_path):
self.log.debug("Sending %s to trash", os_path)
# Looking at the code in send2trash, I don't think the errors it
# raises let us distinguish permission errors from other errors in
# code. So for now, just let them all get logged as server errors.
# code. So for now, the "look before you leap" approach is used.
if not self.is_writable(path):
raise web.HTTPError(403, u"Permission denied: %s" % path)
self.log.debug("Sending %s to trash", os_path)
send2trash(os_path)
return
else:
Expand Down Expand Up @@ -842,10 +862,12 @@ async def is_non_empty_dir(os_path):
# deleting non-empty files. See Github issue 3631.
raise web.HTTPError(400, u"Directory %s not empty" % os_path)
if await _check_trash(os_path):
self.log.debug("Sending %s to trash", os_path)
# Looking at the code in send2trash, I don't think the errors it
# raises let us distinguish permission errors from other errors in
# code. So for now, just let them all get logged as server errors.
# code. So for now, the "look before you leap" approach is used.
if not self.is_writable(path):
raise web.HTTPError(403, u"Permission denied: %s" % path)
self.log.debug("Sending %s to trash", os_path)
send2trash(os_path)
return
else:
Expand Down