From 193d7e0cb6a3fc820f89a7f51c74f8e2084ab972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 09:42:54 +0100 Subject: [PATCH 1/6] Ensure terminal cwd exists --- jupyter_server/terminal/api_handlers.py | 18 +++-- tests/test_terminal.py | 91 ++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 9 deletions(-) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index bc2d4cd3f7..1e6df50e9f 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -1,5 +1,5 @@ import json -import os +from pathlib import Path from tornado import web @@ -30,11 +30,17 @@ def post(self): # if cwd is a relative path, it should be relative to the root_dir, # but if we pass it as relative, it will we be considered as relative to # the path jupyter_server was started in - if "cwd" in data.keys(): - if not os.path.isabs(data["cwd"]): - cwd = data["cwd"] - cwd = os.path.join(self.settings["server_root_dir"], cwd) - data["cwd"] = cwd + if "cwd" in data: + cwd = Path(data["cwd"]) + if not cwd.resolve().exists(): + cwd = Path(self.settings["server_root_dir"]) / cwd + if not cwd.resolve().exists(): + cwd = None + + if cwd is None: + del data["cwd"] + else: + data["cwd"] = str(cwd.resolve()) model = self.terminal_manager.create(**data) self.finish(json.dumps(model)) diff --git a/tests/test_terminal.py b/tests/test_terminal.py index a12ad2ef52..d1deb67c33 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -8,9 +8,8 @@ from traitlets.config import Config -@pytest.fixture -def terminal_path(tmp_path): - subdir = tmp_path.joinpath("terminal_path") +def create_terminal_fixture(path: pathlib.Path): + subdir = path.joinpath("terminal_path") subdir.mkdir() yield subdir @@ -18,6 +17,16 @@ def terminal_path(tmp_path): shutil.rmtree(str(subdir), ignore_errors=True) +@pytest.fixture +def terminal_path(tmp_path): + return create_terminal_fixture(tmp_path) + + +@pytest.fixture +def terminal_root_dir(jp_root_dir): + return create_terminal_fixture(jp_root_dir) + + CULL_TIMEOUT = 10 CULL_INTERVAL = 3 @@ -137,6 +146,82 @@ async def test_terminal_create_with_cwd( await jp_cleanup_subprocesses() +async def test_terminal_create_with_relative_cwd( + jp_fetch, jp_ws_fetch, jp_root_dir, terminal_root_dir, jp_cleanup_subprocesses +): + resp = await jp_fetch( + "api", + "terminals", + method="POST", + body=json.dumps( + {"cwd": str(terminal_root_dir.relative_to(jp_root_dir))} + ), + allow_nonstandard_methods=True, + ) + + data = json.loads(resp.body.decode()) + term_name = data["name"] + + ws = await jp_ws_fetch("terminals", "websocket", term_name) + + ws.write_message(json.dumps(["stdin", "pwd\r\n"])) + + message_stdout = "" + while True: + try: + message = await asyncio.wait_for(ws.read_message(), timeout=5.0) + except asyncio.TimeoutError: + break + + message = json.loads(message) + + if message[0] == "stdout": + message_stdout += message[1] + + ws.close() + + assert str(terminal_root_dir) in message_stdout + await jp_cleanup_subprocesses() + + +async def test_terminal_create_with_bad_cwd( + jp_fetch, jp_ws_fetch, jp_root_dir, jp_cleanup_subprocesses +): + resp = await jp_fetch( + "api", + "terminals", + method="POST", + body=json.dumps( + {"cwd": "/tmp/path/to/nowhere"} + ), + allow_nonstandard_methods=True, + ) + + data = json.loads(resp.body.decode()) + term_name = data["name"] + + ws = await jp_ws_fetch("terminals", "websocket", term_name) + + ws.write_message(json.dumps(["stdin", "pwd\r\n"])) + + message_stdout = "" + while True: + try: + message = await asyncio.wait_for(ws.read_message(), timeout=5.0) + except asyncio.TimeoutError: + break + + message = json.loads(message) + + if message[0] == "stdout": + message_stdout += message[1] + + ws.close() + + assert str(jp_root_dir) in message_stdout + await jp_cleanup_subprocesses() + + async def test_culling_config(jp_server_config, jp_configurable_serverapp): terminal_mgr_config = jp_configurable_serverapp().config.ServerApp.TerminalManager assert terminal_mgr_config.cull_inactive_timeout == CULL_TIMEOUT From 8d01f764b39510295651eca3d6d5c2c5b52b9f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 09:47:54 +0100 Subject: [PATCH 2/6] Fix CI --- jupyter_server/terminal/api_handlers.py | 2 +- tests/test_terminal.py | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 1e6df50e9f..9a4f09c2ec 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -36,7 +36,7 @@ def post(self): cwd = Path(self.settings["server_root_dir"]) / cwd if not cwd.resolve().exists(): cwd = None - + if cwd is None: del data["cwd"] else: diff --git a/tests/test_terminal.py b/tests/test_terminal.py index d1deb67c33..18ffa90977 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -8,7 +8,7 @@ from traitlets.config import Config -def create_terminal_fixture(path: pathlib.Path): +def create_terminal_fixture(path: "pathlib.Path"): subdir = path.joinpath("terminal_path") subdir.mkdir() @@ -153,9 +153,7 @@ async def test_terminal_create_with_relative_cwd( "api", "terminals", method="POST", - body=json.dumps( - {"cwd": str(terminal_root_dir.relative_to(jp_root_dir))} - ), + body=json.dumps({"cwd": str(terminal_root_dir.relative_to(jp_root_dir))}), allow_nonstandard_methods=True, ) @@ -191,9 +189,7 @@ async def test_terminal_create_with_bad_cwd( "api", "terminals", method="POST", - body=json.dumps( - {"cwd": "/tmp/path/to/nowhere"} - ), + body=json.dumps({"cwd": "/tmp/path/to/nowhere"}), allow_nonstandard_methods=True, ) From 75bb5aa918fc06938531aa4ce3d0d1662d8dbf8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 10:30:20 +0100 Subject: [PATCH 3/6] Fix unit tests --- tests/test_terminal.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/test_terminal.py b/tests/test_terminal.py index 18ffa90977..842db0fb89 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -8,8 +8,10 @@ from traitlets.config import Config -def create_terminal_fixture(path: "pathlib.Path"): - subdir = path.joinpath("terminal_path") +@pytest.fixture +def terminal_path(tmp_path): + # return create_terminal_fixture(tmp_path) + subdir = tmp_path.joinpath("terminal_path") subdir.mkdir() yield subdir @@ -18,13 +20,13 @@ def create_terminal_fixture(path: "pathlib.Path"): @pytest.fixture -def terminal_path(tmp_path): - return create_terminal_fixture(tmp_path) +def terminal_root_dir(jp_root_dir): + subdir = jp_root_dir.joinpath("terminal_path") + subdir.mkdir() + yield subdir -@pytest.fixture -def terminal_root_dir(jp_root_dir): - return create_terminal_fixture(jp_root_dir) + shutil.rmtree(str(subdir), ignore_errors=True) CULL_TIMEOUT = 10 @@ -183,13 +185,14 @@ async def test_terminal_create_with_relative_cwd( async def test_terminal_create_with_bad_cwd( - jp_fetch, jp_ws_fetch, jp_root_dir, jp_cleanup_subprocesses + jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses ): + non_existing_path = "/tmp/path/to/nowhere" resp = await jp_fetch( "api", "terminals", method="POST", - body=json.dumps({"cwd": "/tmp/path/to/nowhere"}), + body=json.dumps({"cwd": non_existing_path}), allow_nonstandard_methods=True, ) @@ -214,7 +217,7 @@ async def test_terminal_create_with_bad_cwd( ws.close() - assert str(jp_root_dir) in message_stdout + assert non_existing_path not in message_stdout await jp_cleanup_subprocesses() From c562ee888954a20dd1cad95dac28e0b79a79b5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 11:47:36 +0100 Subject: [PATCH 4/6] Add debug message and try to fix windows tests --- jupyter_server/terminal/api_handlers.py | 2 ++ tests/test_terminal.py | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 9a4f09c2ec..8cdb3d491e 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -38,8 +38,10 @@ def post(self): cwd = None if cwd is None: + self.log.debug(f"Failed to find requested terminal cwd: {data.get('cwd')}") del data["cwd"] else: + self.log.debug(f"Opening terminal in: {cwd.resolve()!s}") data["cwd"] = str(cwd.resolve()) model = self.terminal_manager.create(**data) diff --git a/tests/test_terminal.py b/tests/test_terminal.py index 842db0fb89..d1eef0b1d4 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -2,6 +2,7 @@ import json import os import shutil +import sys import pytest from tornado.httpclient import HTTPClientError @@ -10,7 +11,6 @@ @pytest.fixture def terminal_path(tmp_path): - # return create_terminal_fixture(tmp_path) subdir = tmp_path.joinpath("terminal_path") subdir.mkdir() @@ -180,13 +180,12 @@ async def test_terminal_create_with_relative_cwd( ws.close() - assert str(terminal_root_dir) in message_stdout + expected = terminal_root_dir.name if sys.platform == "win32" else str(terminal_root_dir) + assert expected in message_stdout await jp_cleanup_subprocesses() -async def test_terminal_create_with_bad_cwd( - jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses -): +async def test_terminal_create_with_bad_cwd(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses): non_existing_path = "/tmp/path/to/nowhere" resp = await jp_fetch( "api", From d340c801a987b2896d4c61c3f0dbfe1bbe297c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 14:39:55 +0100 Subject: [PATCH 5/6] Expand user folder it is removed in ServerApp from the root directory --- jupyter_server/terminal/api_handlers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index 8cdb3d491e..b1e3bf4326 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -33,12 +33,14 @@ def post(self): if "cwd" in data: cwd = Path(data["cwd"]) if not cwd.resolve().exists(): - cwd = Path(self.settings["server_root_dir"]) / cwd + cwd = Path(self.settings["server_root_dir"]).expanduser() / cwd if not cwd.resolve().exists(): cwd = None if cwd is None: - self.log.debug(f"Failed to find requested terminal cwd: {data.get('cwd')}") + self.log.debug( + f"Failed to find requested terminal cwd: {data.get('cwd')}\n It was not found within the server root neither: {cwd.resolve()!s}." + ) del data["cwd"] else: self.log.debug(f"Opening terminal in: {cwd.resolve()!s}") From b339cb10806ba52ca9ef4da53eb3071a1c4a3390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Thu, 24 Mar 2022 15:34:58 +0100 Subject: [PATCH 6/6] Correct debug msg --- jupyter_server/terminal/api_handlers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyter_server/terminal/api_handlers.py b/jupyter_server/terminal/api_handlers.py index b1e3bf4326..307b0d9852 100644 --- a/jupyter_server/terminal/api_handlers.py +++ b/jupyter_server/terminal/api_handlers.py @@ -38,8 +38,10 @@ def post(self): cwd = None if cwd is None: + server_root_dir = self.settings["server_root_dir"] self.log.debug( - f"Failed to find requested terminal cwd: {data.get('cwd')}\n It was not found within the server root neither: {cwd.resolve()!s}." + f"Failed to find requested terminal cwd: {data.get('cwd')}\n" + f" It was not found within the server root neither: {server_root_dir}." ) del data["cwd"] else: