diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index 865f797b17..b786d31548 100644 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1617,71 +1617,29 @@ def _normalize_dir(self, value): value = os.path.abspath(value) return value - # Because the validation of preferred_dir depends on root_dir and validation - # occurs when the trait is loaded, there are times when we should defer the - # validation of preferred_dir (e.g., when preferred_dir is defined via CLI - # and root_dir is defined via a config file). - _defer_preferred_dir_validation = False - @validate("root_dir") def _root_dir_validate(self, proposal): value = self._normalize_dir(proposal["value"]) if not os.path.isdir(value): raise TraitError(trans.gettext("No such directory: '%r'") % value) - - if self._defer_preferred_dir_validation: - # If we're here, then preferred_dir is configured on the CLI and - # root_dir is configured in a file - self._preferred_dir_validation(self.preferred_dir, value) return value preferred_dir = Unicode( + None, + allow_none=True, config=True, help=trans.gettext("Preferred starting directory to use for notebooks and kernels."), ) - @default("preferred_dir") - def _default_prefered_dir(self): - return self.root_dir - @validate("preferred_dir") def _preferred_dir_validate(self, proposal): + if proposal["value"] is None: + return None value = self._normalize_dir(proposal["value"]) if not os.path.isdir(value): raise TraitError(trans.gettext("No such preferred dir: '%r'") % value) - - # Before we validate against root_dir, check if this trait is defined on the CLI - # and root_dir is not. If that's the case, we'll defer it's further validation - # until root_dir is validated or the server is starting (the latter occurs when - # the default root_dir (cwd) is used). - cli_config = self.cli_config.get("ServerApp", {}) - if "preferred_dir" in cli_config and "root_dir" not in cli_config: - self._defer_preferred_dir_validation = True - - if not self._defer_preferred_dir_validation: # Validate now - self._preferred_dir_validation(value, self.root_dir) return value - def _preferred_dir_validation(self, preferred_dir: str, root_dir: str) -> None: - """Validate preferred dir relative to root_dir - preferred_dir must be equal or a subdir of root_dir""" - if not preferred_dir.startswith(root_dir): - raise TraitError( - trans.gettext( - "preferred_dir must be equal or a subdir of root_dir. preferred_dir: '%r' root_dir: '%r'" - ) - % (preferred_dir, root_dir) - ) - self._defer_preferred_dir_validation = False - - @observe("root_dir") - def _root_dir_changed(self, change): - self._root_dir_set = True - if not self.preferred_dir.startswith(change["new"]): - self.log.warning( - trans.gettext("Value of preferred_dir updated to use value of root_dir") - ) - self.preferred_dir = change["new"] - @observe("server_extensions") def _update_server_extensions(self, change): self.log.warning(_i18n("server_extensions is deprecated, use jpserver_extensions")) @@ -1865,6 +1823,9 @@ def init_configurables(self): parent=self, log=self.log, ) + # Trigger a default/validation here explicitly while we still supporte + # deprecated trait on ServerApp (FIXME remove when deprecation finalized) + self.contents_manager.preferred_dir self.session_manager = self.session_manager_class( parent=self, log=self.log, @@ -2482,10 +2443,6 @@ def initialize( # Parse command line, load ServerApp config files, # and update ServerApp config. super().initialize(argv=argv) - if self._defer_preferred_dir_validation: - # If we're here, then preferred_dir is configured on the CLI and - # root_dir has the default value (cwd) - self._preferred_dir_validation(self.preferred_dir, self.root_dir) if self._dispatching: return # initialize io loop as early as possible, diff --git a/jupyter_server/services/contents/fileio.py b/jupyter_server/services/contents/fileio.py index e1d6ae66dc..57314ebbc5 100644 --- a/jupyter_server/services/contents/fileio.py +++ b/jupyter_server/services/contents/fileio.py @@ -254,6 +254,9 @@ def _get_os_path(self, path): 404: if path is outside root """ root = os.path.abspath(self.root_dir) + # to_os_path is not safe if path starts with a drive, since os.path.join discards first part + if os.path.splitdrive(path)[0]: + raise HTTPError(404, "%s is not a relative API path" % path) os_path = to_os_path(path, root) if not (os.path.abspath(os_path) + os.path.sep).startswith(root): raise HTTPError(404, "%s is outside root contents directory" % path) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 9b5d876966..c8dbac882a 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -7,6 +7,7 @@ import shutil import stat import sys +import warnings from datetime import datetime import nbformat @@ -55,6 +56,24 @@ def _validate_root_dir(self, proposal): raise TraitError("%r is not a directory" % value) return value + @default("preferred_dir") + def _default_preferred_dir(self): + try: + value = self.parent.preferred_dir + except AttributeError: + pass + else: + if value is not None: + warnings.warn( + "ServerApp.preferred_dir config is deprecated in jupyter-server 2.0. Use ContentsManager.preferred_dir with a relative path instead", + FutureWarning, + stacklevel=3, + ) + if not (value + os.path.sep).startswith(self.root_dir): + raise TraitError("%s is outside root contents directory" % value) + return os.path.relpath(value, self.root_dir).replace(os.path.sep, "/") + return "/" + @default("checkpoints_class") def _checkpoints_class_default(self): return FileCheckpoints diff --git a/jupyter_server/services/contents/manager.py b/jupyter_server/services/contents/manager.py index 089a71fc65..a94946b8c1 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -75,10 +75,27 @@ def emit(self, data): root_dir = Unicode("/", config=True) + preferred_dir = Unicode( + "/", + config=True, + help=_i18n( + "Preferred starting directory to use for notebooks as a path local to `root_dir`." + ), + ) + + @validate("preferred_dir") + def _validate_preferred_dir(self, proposal): + value = proposal["value"] + + if not value.startswith(self.root_dir): + raise TraitError(_i18n(f"{value} is outside root contents directory")) + return value + allow_hidden = Bool(False, config=True, help="Allow access to hidden files") notary = Instance(sign.NotebookNotary) + @default("notary") def _notary_default(self): return sign.NotebookNotary(parent=self) diff --git a/tests/test_gateway.py b/tests/test_gateway.py index c1f8ce2f29..a07d2f121c 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -421,7 +421,7 @@ async def test_gateway_session_lifecycle(init_gateway, jp_root_dir, jp_fetch): # Validate session lifecycle functions; create and delete. # create - session_id, kernel_id = await create_session(jp_root_dir, jp_fetch, "kspec_foo") + session_id, kernel_id = await create_session(jp_fetch, "kspec_foo") # ensure kernel still considered running assert await is_kernel_running(jp_fetch, kernel_id) is True @@ -567,12 +567,12 @@ async def test_channel_queue_get_msg_when_response_router_had_finished(): # # Test methods below... # -async def create_session(root_dir, jp_fetch, kernel_name): +async def create_session(jp_fetch, kernel_name): """Creates a session for a kernel. The session is created against the server which then uses the gateway for kernel management. """ with mocked_gateway: - nb_path = root_dir / "testgw.ipynb" + nb_path = "/testgw.ipynb" body = json.dumps( {"path": str(nb_path), "type": "notebook", "kernel": {"name": kernel_name}} ) diff --git a/tests/test_serverapp.py b/tests/test_serverapp.py index 0da94cffad..5be17f5624 100644 --- a/tests/test_serverapp.py +++ b/tests/test_serverapp.py @@ -6,6 +6,7 @@ import pytest from jupyter_core.application import NoStart +from tornado.web import HTTPError from traitlets import TraitError from traitlets.tests.utils import check_help_all_output @@ -256,6 +257,7 @@ def test_urls(config, public_url, local_url, connection_url): # Preferred dir tests # ---------------------------------------------------------------------------- +@pytest.mark.filterwarnings("ignore::FutureWarning") def test_valid_preferred_dir(tmp_path, jp_configurable_serverapp): path = str(tmp_path) app = jp_configurable_serverapp(root_dir=path, preferred_dir=path) @@ -264,6 +266,7 @@ def test_valid_preferred_dir(tmp_path, jp_configurable_serverapp): assert app.root_dir == app.preferred_dir +@pytest.mark.filterwarnings("ignore::FutureWarning") def test_valid_preferred_dir_is_root_subdir(tmp_path, jp_configurable_serverapp): path = str(tmp_path) path_subdir = str(tmp_path / "subdir") @@ -283,6 +286,8 @@ def test_valid_preferred_dir_does_not_exist(tmp_path, jp_configurable_serverapp) assert "No such preferred dir:" in str(error) +# This tests some deprecated behavior as well +@pytest.mark.filterwarnings("ignore::FutureWarning") @pytest.mark.parametrize( "root_dir_loc,preferred_dir_loc", [ @@ -338,8 +343,11 @@ def test_preferred_dir_validation( else: app = jp_configurable_serverapp(**kwargs) assert app.root_dir == expected_root_dir - assert app.preferred_dir == expected_preferred_dir - assert app.preferred_dir.startswith(app.root_dir) + rel = os.path.relpath(expected_preferred_dir, expected_root_dir) + if rel == ".": + rel = "/" + assert app.contents_manager.preferred_dir == rel + assert ".." not in rel def test_invalid_preferred_dir_does_not_exist(tmp_path, jp_configurable_serverapp): @@ -362,42 +370,35 @@ def test_invalid_preferred_dir_does_not_exist_set(tmp_path, jp_configurable_serv assert "No such preferred dir:" in str(error) +@pytest.mark.filterwarnings("ignore::FutureWarning") def test_invalid_preferred_dir_not_root_subdir(tmp_path, jp_configurable_serverapp): path = str(tmp_path / "subdir") os.makedirs(path, exist_ok=True) not_subdir_path = str(tmp_path) - with pytest.raises(TraitError) as error: - app = jp_configurable_serverapp(root_dir=path, preferred_dir=not_subdir_path) - - assert "preferred_dir must be equal or a subdir of root_dir. " in str(error) + with pytest.raises(SystemExit): + jp_configurable_serverapp(root_dir=path, preferred_dir=not_subdir_path) -def test_invalid_preferred_dir_not_root_subdir_set(tmp_path, jp_configurable_serverapp): +async def test_invalid_preferred_dir_not_root_subdir_set(tmp_path, jp_configurable_serverapp): path = str(tmp_path / "subdir") os.makedirs(path, exist_ok=True) - not_subdir_path = str(tmp_path) + not_subdir_path = os.path.relpath(tmp_path, path) app = jp_configurable_serverapp(root_dir=path) with pytest.raises(TraitError) as error: - app.preferred_dir = not_subdir_path + app.contents_manager.preferred_dir = not_subdir_path - assert "preferred_dir must be equal or a subdir of root_dir. " in str(error) + assert "is outside root contents directory" in str(error.value) -def test_observed_root_dir_updates_preferred_dir(tmp_path, jp_configurable_serverapp): - path = str(tmp_path) - new_path = str(tmp_path / "subdir") - os.makedirs(new_path, exist_ok=True) - - app = jp_configurable_serverapp(root_dir=path, preferred_dir=path) - app.root_dir = new_path - assert app.preferred_dir == new_path +async def test_absolute_preferred_dir_not_root_subdir_set(tmp_path, jp_configurable_serverapp): + path = str(tmp_path / "subdir") + os.makedirs(path, exist_ok=True) + not_subdir_path = str(tmp_path) + app = jp_configurable_serverapp(root_dir=path) + with pytest.raises(TraitError) as error: + app.contents_manager.preferred_dir = not_subdir_path -def test_observed_root_dir_does_not_update_preferred_dir(tmp_path, jp_configurable_serverapp): - path = str(tmp_path) - new_path = str(tmp_path.parent) - app = jp_configurable_serverapp(root_dir=path, preferred_dir=path) - app.root_dir = new_path - assert app.preferred_dir == path + assert "is outside root contents directory" in str(error.value) diff --git a/tests/test_terminal.py b/tests/test_terminal.py index f011f2abef..3260548f3e 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -276,4 +276,10 @@ def test_shell_command_override( pytest.importorskip("traitlets", minversion=min_traitlets) argv = shlex.split(f"--ServerApp.terminado_settings={terminado_settings}") app = jp_configurable_serverapp(argv=argv) - assert app.web_app.settings["terminal_manager"].shell_command == expected_shell + if os.name == "nt": + assert app.web_app.settings["terminal_manager"].shell_command in ( + expected_shell, + " ".join(expected_shell), + ) + else: + assert app.web_app.settings["terminal_manager"].shell_command == expected_shell