From e5d3921a51729085ac0ec2f2999672470c178fae Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Sat, 12 Mar 2022 12:42:43 -0500 Subject: [PATCH 01/11] Remove redundant calls in new_untitled method and added dir_exists call in PUT to maintain behavior --- jupyter_server/services/contents/filemanager.py | 11 +++++++++++ jupyter_server/services/contents/handlers.py | 3 +++ jupyter_server/services/contents/manager.py | 3 --- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 554087579a..221b916c16 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -906,3 +906,14 @@ async def rename_file(self, old_path, new_path): raise except Exception as e: raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e + + async def dir_exists(self, path): + path = path.strip("/") + os_path = self._get_os_path(path=path) + return os.path.isdir(os_path) + + async def file_exists(self, path): + pass + + async def is_hidden(self, path): + pass \ No newline at end of file diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index 83db1f99ee..e20b829d93 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -234,6 +234,9 @@ async def put(self, path=""): else: await self._upload(model, path) else: + dir_exists = await ensure_async(self.contents_manager.dir_exists(path)) + if not dir_exists: + raise web.HTTPError(404, "No such directory: %s" % path) await self._new_untitled(path) @web.authenticated diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 3e47a637af..71234b01f3 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -752,9 +752,6 @@ async def new_untitled(self, path="", type="", ext=""): Use `new` to create files with a fully specified path (including filename). """ path = path.strip("/") - dir_exists = await ensure_async(self.dir_exists(path)) - if not dir_exists: - raise HTTPError(404, "No such directory: %s" % path) model = {} if type: From 42b87df1de6c6ef07e4dc0187f20b12fbbf4c246 Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Mon, 14 Mar 2022 11:17:56 -0400 Subject: [PATCH 02/11] Remove dir_exists from Contents Manager handlers --- jupyter_server/services/contents/filemanager.py | 13 +------------ jupyter_server/services/contents/handlers.py | 7 ------- jupyter_server/services/contents/manager.py | 11 +++++++++++ 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 221b916c16..2b5f900be3 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -905,15 +905,4 @@ async def rename_file(self, old_path, new_path): except web.HTTPError: raise except Exception as e: - raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e - - async def dir_exists(self, path): - path = path.strip("/") - os_path = self._get_os_path(path=path) - return os.path.isdir(os_path) - - async def file_exists(self, path): - pass - - async def is_hidden(self, path): - pass \ No newline at end of file + raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e \ No newline at end of file diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index e20b829d93..bdd0afbbd0 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -194,10 +194,6 @@ async def post(self, path=""): if file_exists: raise web.HTTPError(400, "Cannot POST to files, use PUT instead.") - dir_exists = await ensure_async(cm.dir_exists(path)) - if not dir_exists: - raise web.HTTPError(404, "No such directory: %s" % path) - model = self.get_json_body() if model is not None: @@ -234,9 +230,6 @@ async def put(self, path=""): else: await self._upload(model, path) else: - dir_exists = await ensure_async(self.contents_manager.dir_exists(path)) - if not dir_exists: - raise web.HTTPError(404, "No such directory: %s" % path) await self._new_untitled(path) @web.authenticated diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 71234b01f3..1084d9b887 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -459,6 +459,10 @@ def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") + dir_exists = self.dir_exists(path) + if not dir_exists: + raise HTTPError(404, "No such directory: %s" % path) + if to_path is not None: to_path = to_path.strip("/") @@ -752,6 +756,9 @@ async def new_untitled(self, path="", type="", ext=""): Use `new` to create files with a fully specified path (including filename). """ path = path.strip("/") + dir_exists = await ensure_async(self.dir_exists(path)) + if not dir_exists: + raise HTTPError(404, "No such directory: %s" % path) model = {} if type: @@ -816,6 +823,10 @@ async def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") + dir_exists = await self.dir_exists(path) + if not dir_exists: + raise HTTPError(404, "No such directory: %s" % path) + if to_path is not None: to_path = to_path.strip("/") From 2c866823307a1c4310f1d087e2c4d6e4018fe85c Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Mon, 14 Mar 2022 11:31:09 -0400 Subject: [PATCH 03/11] fix copy implementation --- jupyter_server/services/contents/manager.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 1084d9b887..198def5e43 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -459,12 +459,12 @@ def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") - dir_exists = self.dir_exists(path) - if not dir_exists: - raise HTTPError(404, "No such directory: %s" % path) if to_path is not None: to_path = to_path.strip("/") + dir_exists = self.dir_exists(path) + if not dir_exists: + raise HTTPError(404, "No such directory: %s" % path) if "/" in path: from_dir, from_name = path.rsplit("/", 1) @@ -823,13 +823,14 @@ async def copy(self, from_path, to_path=None): from_path must be a full path to a file. """ path = from_path.strip("/") - dir_exists = await self.dir_exists(path) - if not dir_exists: - raise HTTPError(404, "No such directory: %s" % path) if to_path is not None: to_path = to_path.strip("/") + dir_exists = await self.dir_exists(path) + if not dir_exists: + raise HTTPError(404, "No such directory: %s" % path) + if "/" in path: from_dir, from_name = path.rsplit("/", 1) else: From c8b4ea14346a069883bf8f8c0ea3a739ca5b8773 Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Mon, 14 Mar 2022 11:31:44 -0400 Subject: [PATCH 04/11] fix copy implementation --- jupyter_server/services/contents/manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 198def5e43..91995572d1 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -462,9 +462,9 @@ def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - dir_exists = self.dir_exists(path) + dir_exists = self.dir_exists(to_path) if not dir_exists: - raise HTTPError(404, "No such directory: %s" % path) + raise HTTPError(404, "No such directory: %s" % to_path) if "/" in path: from_dir, from_name = path.rsplit("/", 1) @@ -827,9 +827,9 @@ async def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - dir_exists = await self.dir_exists(path) + dir_exists = await self.dir_exists(to_path) if not dir_exists: - raise HTTPError(404, "No such directory: %s" % path) + raise HTTPError(404, "No such directory: %s" % to_path) if "/" in path: from_dir, from_name = path.rsplit("/", 1) From c9ba8692e96f1f1eeb59f95a45c792ec9b86e3bb Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Mon, 14 Mar 2022 19:41:00 -0400 Subject: [PATCH 05/11] add back new line --- jupyter_server/services/contents/filemanager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 2b5f900be3..a61dc47db8 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -905,4 +905,5 @@ async def rename_file(self, old_path, new_path): except web.HTTPError: raise except Exception as e: - raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e \ No newline at end of file + raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e + From a76c7f7010388001bef8f6a1e2a88be077d524f2 Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Mon, 14 Mar 2022 19:43:30 -0400 Subject: [PATCH 06/11] fix merge conflict --- jupyter_server/services/contents/filemanager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index a61dc47db8..554087579a 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -906,4 +906,3 @@ async def rename_file(self, old_path, new_path): raise except Exception as e: raise web.HTTPError(500, "Unknown error renaming file: %s %s" % (old_path, e)) from e - From 53992fb149a5e3b7c1281e3a4ca5fe83cef79c4d Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Fri, 25 Mar 2022 20:27:38 -0400 Subject: [PATCH 07/11] remove duplication --- jupyter_server/services/contents/manager.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 91995572d1..cb5cd9a25d 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -462,10 +462,7 @@ def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - dir_exists = self.dir_exists(to_path) - if not dir_exists: - raise HTTPError(404, "No such directory: %s" % to_path) - + if "/" in path: from_dir, from_name = path.rsplit("/", 1) else: @@ -484,6 +481,8 @@ def copy(self, from_path, to_path=None): name = copy_pat.sub(".", from_name) to_name = self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) + else: + raise HTTPError(404, "No such directory: %s" % to_path) model = self.save(model, to_path) return model From f6a300b9bf678464ea3bdfd10d40bccbc2089920 Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Fri, 25 Mar 2022 21:28:35 -0400 Subject: [PATCH 08/11] address feedback --- jupyter_server/services/contents/manager.py | 25 ++++++++++++++------- tests/services/contents/test_manager.py | 9 ++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index e1a57f6c87..d8a7eabfb2 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -602,14 +602,19 @@ def copy(self, from_path, to_path=None): if model["type"] == "directory": raise HTTPError(400, "Can't copy directories") - if to_path is None: + is_destination_specified = to_path is not None + if not is_destination_specified: to_path = from_dir if self.dir_exists(to_path): name = copy_pat.sub(".", from_name) to_name = self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) + elif is_destination_specified and "/" in to_path: + to_dir, to_name = path.rsplit("/", 1) + if not self.dir_exists(to_dir): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) else: - raise HTTPError(404, "No such directory: %s" % to_path) + raise HTTPError(404, "No such directory: %s" % to_path) model = self.save(model, to_path) return model @@ -952,11 +957,7 @@ async def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - - dir_exists = await self.dir_exists(to_path) - if not dir_exists: - raise HTTPError(404, "No such directory: %s" % to_path) - + if "/" in path: from_dir, from_name = path.rsplit("/", 1) else: @@ -968,12 +969,20 @@ async def copy(self, from_path, to_path=None): model.pop("name", None) if model["type"] == "directory": raise HTTPError(400, "Can't copy directories") - if to_path is None: + + is_destination_specified = to_path is not None + if not is_destination_specified: to_path = from_dir if await ensure_async(self.dir_exists(to_path)): name = copy_pat.sub(".", from_name) to_name = await self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) + elif is_destination_specified and "/" in to_path: + to_dir, to_name = path.rsplit("/", 1) + if not await ensure_async(self.dir_exists(to_dir)): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) + else: + raise HTTPError(404, "No such directory: %s" % to_path) model = await self.save(model, to_path) return model diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index 04608dc6e1..c4f45360db 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -657,11 +657,20 @@ async def test_copy(jp_contents_manager): copy2 = await ensure_async(cm.copy(path, "å b/copy 2.ipynb")) assert copy2["name"] == "copy 2.ipynb" assert copy2["path"] == "å b/copy 2.ipynb" + # copy with specified path copy2 = await ensure_async(cm.copy(path, "/")) assert copy2["name"] == name assert copy2["path"] == name + # copy to destination that does not exist + with pytest.raises(HTTPError) as e: + await ensure_async(cm.copy(path, "å x")) + + # copy to destination whose parent dir does not exist + with pytest.raises(HTTPError) as e: + await ensure_async(cm.copy(path, "å x/copy 2.ipynb")) + async def test_mark_trusted_cells(jp_contents_manager): cm = jp_contents_manager From cd96a4eda4b1cbf8524e1b8bda8034213e3addcc Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Fri, 25 Mar 2022 21:30:45 -0400 Subject: [PATCH 09/11] address feedback --- tests/services/contents/test_manager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index c4f45360db..9e99a77119 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -671,6 +671,10 @@ async def test_copy(jp_contents_manager): with pytest.raises(HTTPError) as e: await ensure_async(cm.copy(path, "å x/copy 2.ipynb")) + copy3 = await ensure_async(cm.copy(path, "/copy 3.ipynb")) + assert copy3["name"] == "copy 3.ipynb" + assert copy3["path"] == "/copy 3.ipynb" + async def test_mark_trusted_cells(jp_contents_manager): cm = jp_contents_manager From 073b46e4669086b091621e33a1f9e58c150689ea Mon Sep 17 00:00:00 2001 From: Josh Hamet Date: Fri, 25 Mar 2022 21:52:57 -0400 Subject: [PATCH 10/11] feedback --- jupyter_server/services/contents/manager.py | 18 ++++++++++-------- tests/services/contents/test_manager.py | 6 +----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index d8a7eabfb2..8cc9e3d89b 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -609,10 +609,11 @@ def copy(self, from_path, to_path=None): name = copy_pat.sub(".", from_name) to_name = self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) - elif is_destination_specified and "/" in to_path: - to_dir, to_name = path.rsplit("/", 1) - if not self.dir_exists(to_dir): - raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) + elif is_destination_specified: + if "/" in to_path: + to_dir, to_name = to_path.rsplit("/", 1) + if not self.dir_exists(to_dir): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) else: raise HTTPError(404, "No such directory: %s" % to_path) @@ -977,10 +978,11 @@ async def copy(self, from_path, to_path=None): name = copy_pat.sub(".", from_name) to_name = await self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) - elif is_destination_specified and "/" in to_path: - to_dir, to_name = path.rsplit("/", 1) - if not await ensure_async(self.dir_exists(to_dir)): - raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) + elif is_destination_specified: + if "/" in to_path: + to_dir, to_name = to_path.rsplit("/", 1) + if not await ensure_async(self.dir_exists(to_dir)): + raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) else: raise HTTPError(404, "No such directory: %s" % to_path) diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index 9e99a77119..b8b6fd5ca6 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -663,17 +663,13 @@ async def test_copy(jp_contents_manager): assert copy2["name"] == name assert copy2["path"] == name - # copy to destination that does not exist - with pytest.raises(HTTPError) as e: - await ensure_async(cm.copy(path, "å x")) - # copy to destination whose parent dir does not exist with pytest.raises(HTTPError) as e: await ensure_async(cm.copy(path, "å x/copy 2.ipynb")) copy3 = await ensure_async(cm.copy(path, "/copy 3.ipynb")) assert copy3["name"] == "copy 3.ipynb" - assert copy3["path"] == "/copy 3.ipynb" + assert copy3["path"] == "copy 3.ipynb" async def test_mark_trusted_cells(jp_contents_manager): From 1e3b4b7d60b29d75faa59da1afc448533f060c4a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 29 Mar 2022 20:27:25 +0000 Subject: [PATCH 11/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_server/services/contents/manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 8cc9e3d89b..654fdba4e5 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -589,7 +589,7 @@ def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - + if "/" in path: from_dir, from_name = path.rsplit("/", 1) else: @@ -615,7 +615,7 @@ def copy(self, from_path, to_path=None): if not self.dir_exists(to_dir): raise HTTPError(404, "No such parent directory: %s to copy file in" % to_dir) else: - raise HTTPError(404, "No such directory: %s" % to_path) + raise HTTPError(404, "No such directory: %s" % to_path) model = self.save(model, to_path) return model @@ -958,7 +958,7 @@ async def copy(self, from_path, to_path=None): if to_path is not None: to_path = to_path.strip("/") - + if "/" in path: from_dir, from_name = path.rsplit("/", 1) else: @@ -978,7 +978,7 @@ async def copy(self, from_path, to_path=None): name = copy_pat.sub(".", from_name) to_name = await self.increment_filename(name, to_path, insert="-Copy") to_path = "{0}/{1}".format(to_path, to_name) - elif is_destination_specified: + elif is_destination_specified: if "/" in to_path: to_dir, to_name = to_path.rsplit("/", 1) if not await ensure_async(self.dir_exists(to_dir)):