Make preferred_dir content manager trait

Move the preferred_dir trait to content manager, but keep the old one for backwards compatibility. The new location should also cause the value to be read later in the init, allowing us to avoid the deferred validation. Also fixes an issue with escaping the root dir on Windows if an absolute path is passed.

Author: vidartf

jupyter_server/serverapp.py

@@ -1644,71 +1644,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"))
@@ -2495,10 +2453,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,

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)

jupyter_server/services/contents/filemanager.py

@@ -7,7 +7,9 @@
 import shutil
 import stat
 import sys
+import warnings
 from datetime import datetime
+from posixpath import isabs
 
 import nbformat
 from anyio.to_thread import run_sync
@@ -17,7 +19,7 @@
 from traitlets import Bool, TraitError, Unicode, default, validate
 
 from jupyter_server import _tz as tz
-from jupyter_server.base.handlers import AuthenticatedFileHandler
+from jupyter_server.base.handlers import AuthenticatedFileHandler, HTTPError
 from jupyter_server.transutils import _i18n
 
 from .filecheckpoints import AsyncFileCheckpoints, FileCheckpoints
@@ -55,6 +57,40 @@ 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",
+                    DeprecationWarning,
+                    stacklevel=3,
+                )
+                # For transitioning to relative path, we check if it is a valid relative path:
+                try:
+                    if not os.path.isabs(value) and self.dir_exists(value):
+                        return value
+                except HTTPError:
+                    pass
+                value = self.parent._normalize_dir(value)
+                if not os.path.isdir(value):
+                    raise TraitError(_i18n("No such directory: '%r'") % value)
+                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 "/"
+
+    @validate("preferred_dir")
+    def _validate_preferred_dir(self, proposal):
+        try:
+            super()._validate_preferred_dir(proposal)
+        except HTTPError as e:
+            raise TraitError(e.log_message) from e
+
     @default("checkpoints_class")
     def _checkpoints_class_default(self):
         return FileCheckpoints

jupyter_server/services/contents/manager.py

@@ -75,10 +75,26 @@ 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 self.dir_exists(value):
+            raise TraitError(_i18n("No such directory: '%r'") % value)
+        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)
 

tests/test_serverapp.py

@@ -283,6 +283,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::DeprecationWarning")
 @pytest.mark.parametrize(
     "root_dir_loc,preferred_dir_loc",
     [
@@ -333,13 +335,17 @@ def test_preferred_dir_validation(
         kwargs["argv"] = argv  # type:ignore
 
     if root_dir_loc == "default" and preferred_dir_loc != "default":  # error expected
-        with pytest.raises(SystemExit):
-            jp_configurable_serverapp(**kwargs)
+        app = jp_configurable_serverapp(**kwargs)
+        with pytest.raises(TraitError):
+            app.contents_manager.preferred_dir
     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 +368,42 @@ 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::DeprecationWarning")
 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)
+        # reading the value triggers default generator:
+        app.contents_manager.preferred_dir
 
-    assert "preferred_dir must be equal or a subdir of root_dir. " in str(error)
+    assert "is outside root contents directory" in str(error)
 
 
 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
-
-    assert "preferred_dir must be equal or a subdir of root_dir. " in str(error)
+        app.contents_manager.preferred_dir = not_subdir_path
 
+    assert "is outside root contents directory" in str(error)
 
-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
+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
+    if os.name == "nt":
+        assert "is not a relative API path" in str(error)
+    else:
+        assert "is outside root contents directory" in str(error)