diff --git a/jupyter_server/serverapp.py b/jupyter_server/serverapp.py index f3d2e430d9..0e7e4d0a8a 100644 --- a/jupyter_server/serverapp.py +++ b/jupyter_server/serverapp.py @@ -1629,22 +1629,11 @@ 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( @@ -1661,39 +1650,8 @@ def _preferred_dir_validate(self, proposal): 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")) @@ -1868,6 +1826,9 @@ def init_configurables(self): parent=self, log=self.log, ) + # Trigger a default/validation here explicitly while we still support the + # 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, @@ -2432,10 +2393,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 # Then, use extensions' config loading mechanism to 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 5187ee840f..d1b37b30eb 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -7,7 +7,9 @@ import shutil import stat import sys +import warnings from datetime import datetime +from pathlib import Path import nbformat from anyio.to_thread import run_sync @@ -19,6 +21,7 @@ from jupyter_server import _tz as tz from jupyter_server.base.handlers import AuthenticatedFileHandler from jupyter_server.transutils import _i18n +from jupyter_server.utils import to_api_path from .filecheckpoints import AsyncFileCheckpoints, FileCheckpoints from .fileio import AsyncFileManagerMixin, FileManagerMixin @@ -55,6 +58,34 @@ 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 + if value == self.parent.root_dir: + value = None + except AttributeError: + pass + else: + if value is not None: + warnings.warn( + "ServerApp.preferred_dir config is deprecated in jupyter-server 2.0. Use FileContentsManager.preferred_dir instead", + FutureWarning, + stacklevel=3, + ) + try: + path = Path(value) + return path.relative_to(self.root_dir).as_posix() + except ValueError: + raise TraitError("%s is outside root contents directory" % value) from None + return "" + + @validate("preferred_dir") + def _validate_preferred_dir(self, proposal): + # It should be safe to pass an API path through this method: + proposal["value"] = to_api_path(proposal["value"], self.root_dir) + return super()._validate_preferred_dir(proposal) + @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 7bd6450803..fc4847fc7c 100644 --- a/jupyter_server/services/contents/manager.py +++ b/jupyter_server/services/contents/manager.py @@ -3,10 +3,12 @@ # Distributed under the terms of the Modified BSD License. import itertools import json +import os import re import warnings from fnmatch import fnmatch +from jupyter_core.utils import run_sync from nbformat import ValidationError, sign from nbformat import validate as validate_nb from nbformat.v4 import new_notebook @@ -55,10 +57,40 @@ class ContentsManager(LoggingConfigurable): root_dir = Unicode("/", config=True) + preferred_dir = Unicode( + "", + config=True, + help=_i18n( + "Preferred starting directory to use for notebooks. This is an API path (`/` separated, relative to root dir)" + ), + ) + + @validate("preferred_dir") + def _validate_preferred_dir(self, proposal): + value = proposal["value"].strip("/") + try: + import inspect + + if inspect.iscoroutinefunction(self.dir_exists): + dir_exists = run_sync(self.dir_exists)(value) + else: + dir_exists = self.dir_exists(value) + except HTTPError as e: + raise TraitError(e.log_message) from e + if not dir_exists: + raise TraitError(_i18n("Preferred directory not found: %r") % value) + try: + if value != self.parent.preferred_dir: + self.parent.preferred_dir = os.path.join(self.root_dir, *value.split("/")) + except (AttributeError, TraitError): + pass + 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/pyproject.toml b/pyproject.toml index cdbc3165e3..1db471f43e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,3 +103,6 @@ module = [ "websocket" ] ignore_missing_imports = true + +[tool.check-wheel-contents] +ignore = ["W002"] diff --git a/setup.cfg b/setup.cfg index a96e944559..ae45ff684f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -42,7 +42,7 @@ install_requires = argon2-cffi jinja2 jupyter_client>=6.1.12 - jupyter_core>=4.7.0 + jupyter_core>=4.12,!=5.0.* nbconvert>=6.4.4 nbformat>=5.2.0 packaging diff --git a/tests/test_gateway.py b/tests/test_gateway.py index 1dc5d977ca..40d20f8d1b 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -270,7 +270,7 @@ async def test_gateway_class_mappings(init_gateway, jp_serverapp): assert jp_serverapp.kernel_spec_manager_class.__name__ == "GatewayKernelSpecManager" -async def test_gateway_get_kernelspecs(init_gateway, jp_fetch): +async def test_gateway_get_kernelspecs(init_gateway, jp_serverapp, jp_fetch): # Validate that kernelspecs come from gateway. with mocked_gateway: r = await jp_fetch("api", "kernelspecs", method="GET") @@ -297,11 +297,11 @@ async def test_gateway_get_named_kernelspec(init_gateway, jp_fetch): assert expected_http_error(e, 404) -async def test_gateway_session_lifecycle(init_gateway, jp_root_dir, jp_fetch): +async def test_gateway_session_lifecycle(init_gateway, 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 @@ -447,12 +447,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 1ab792d1e5..7cdcacdd7f 100644 --- a/tests/test_serverapp.py +++ b/tests/test_serverapp.py @@ -254,14 +254,18 @@ 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) assert app.root_dir == path assert app.preferred_dir == path assert app.root_dir == app.preferred_dir + assert app.contents_manager.root_dir == path + assert app.contents_manager.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") @@ -270,6 +274,7 @@ def test_valid_preferred_dir_is_root_subdir(tmp_path, jp_configurable_serverapp) assert app.root_dir == path assert app.preferred_dir == path_subdir assert app.preferred_dir.startswith(app.root_dir) + assert app.contents_manager.preferred_dir == "subdir" def test_valid_preferred_dir_does_not_exist(tmp_path, jp_configurable_serverapp): @@ -281,26 +286,46 @@ 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", + "root_dir_loc,preferred_dir_loc,config_target", [ - ("cli", "cli"), - ("cli", "config"), - ("cli", "default"), - ("config", "cli"), - ("config", "config"), - ("config", "default"), - ("default", "cli"), - ("default", "config"), - ("default", "default"), + ("cli", "cli", "ServerApp"), + ("cli", "cli", "FileContentsManager"), + ("cli", "config", "ServerApp"), + ("cli", "config", "FileContentsManager"), + ("cli", "default", "ServerApp"), + ("cli", "default", "FileContentsManager"), + ("config", "cli", "ServerApp"), + ("config", "cli", "FileContentsManager"), + ("config", "config", "ServerApp"), + ("config", "config", "FileContentsManager"), + ("config", "default", "ServerApp"), + ("config", "default", "FileContentsManager"), + ("default", "cli", "ServerApp"), + ("default", "cli", "FileContentsManager"), + ("default", "config", "ServerApp"), + ("default", "config", "FileContentsManager"), + ("default", "default", "ServerApp"), + ("default", "default", "FileContentsManager"), ], ) def test_preferred_dir_validation( - root_dir_loc, preferred_dir_loc, tmp_path, jp_config_dir, jp_configurable_serverapp + root_dir_loc, + preferred_dir_loc, + config_target, + tmp_path, + jp_config_dir, + jp_configurable_serverapp, ): expected_root_dir = str(tmp_path) - expected_preferred_dir = str(tmp_path / "subdir") - os.makedirs(expected_preferred_dir, exist_ok=True) + + os_preferred_dir = str(tmp_path / "subdir") + os.makedirs(os_preferred_dir, exist_ok=True) + config_preferred_dir = os_preferred_dir if config_target == "ServerApp" else "subdir" + config_preferred_dir = config_preferred_dir + "/" # add trailing slash to ensure it is removed + expected_preferred_dir = "subdir" argv = [] kwargs = {"root_dir": None} @@ -311,18 +336,18 @@ def test_preferred_dir_validation( config_file = jp_config_dir.joinpath("jupyter_server_config.py") if root_dir_loc == "cli": - argv.append(f"--ServerApp.root_dir={expected_root_dir}") + argv.append(f"--{config_target}.root_dir={expected_root_dir}") if root_dir_loc == "config": - config_lines.append(f'c.ServerApp.root_dir = r"{expected_root_dir}"') + config_lines.append(f'c.{config_target}.root_dir = r"{expected_root_dir}"') if root_dir_loc == "default": expected_root_dir = os.getcwd() if preferred_dir_loc == "cli": - argv.append(f"--ServerApp.preferred_dir={expected_preferred_dir}") + argv.append(f"--{config_target}.preferred_dir={config_preferred_dir}") if preferred_dir_loc == "config": - config_lines.append(f'c.ServerApp.preferred_dir = r"{expected_preferred_dir}"') + config_lines.append(f'c.{config_target}.preferred_dir = r"{config_preferred_dir}"') if preferred_dir_loc == "default": - expected_preferred_dir = expected_root_dir + expected_preferred_dir = "" if config_file is not None: config_file.write_text("\n".join(config_lines)) @@ -335,9 +360,9 @@ def test_preferred_dir_validation( jp_configurable_serverapp(**kwargs) 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) + assert app.contents_manager.root_dir == expected_root_dir + assert app.contents_manager.preferred_dir == expected_preferred_dir + assert ".." not in expected_preferred_dir def test_invalid_preferred_dir_does_not_exist(tmp_path, jp_configurable_serverapp): @@ -360,42 +385,39 @@ 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) +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, preferred_dir=path) - app.root_dir = new_path - assert app.preferred_dir == new_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 + if os.name == "nt": + assert "is not a relative API path" in str(error.value) + else: + assert "Preferred directory not found" in str(error.value)