Forbid relative paths in rust-project.json

Author: Veykril

crates/paths/src/lib.rs

@@ -166,9 +166,8 @@ impl AbsPath {
         AbsPathBuf::try_from(self.0.to_path_buf()).unwrap()
     }
 
-    /// Equivalent of [`Path::canonicalize`] for `AbsPath`.
-    pub fn canonicalize(&self) -> Result<AbsPathBuf, std::io::Error> {
-        Ok(self.as_ref().canonicalize()?.try_into().unwrap())
+    pub fn canonicalize(&self) -> ! {
+        panic!("We explicitly do not provide canonicalization API, as that is almost always a wrong solution, see #14430")
     }
 
     /// Equivalent of [`Path::strip_prefix`] for `AbsPath`.

crates/project-model/src/manifest_path.rs

@@ -35,9 +35,8 @@ impl ManifestPath {
         self.file.parent().unwrap()
     }
 
-    /// Equivalent of [`Path::canonicalize`] for `ManifestPath`.
-    pub fn canonicalize(&self) -> Result<ManifestPath, std::io::Error> {
-        Ok((&**self).canonicalize()?.try_into().unwrap())
+    pub fn canonicalize(&self) -> ! {
+        (&**self).canonicalize()
     }
 }
 

crates/project-model/src/project_json.rs

@@ -49,13 +49,13 @@
 //! user explores them belongs to that extension (it's totally valid to change
 //! rust-project.json over time via configuration request!)
 
-use std::path::PathBuf;
-
+use anyhow::Result;
 use base_db::{CrateDisplayName, CrateId, CrateName, Dependency, Edition};
 use la_arena::RawIdx;
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
 use serde::{de, Deserialize};
+use std::path::PathBuf;
 
 use crate::cfg_flag::CfgFlag;
 
@@ -98,34 +98,62 @@ impl ProjectJson {
     /// * `base` - The path to the workspace root (i.e. the folder containing `rust-project.json`)
     /// * `data` - The parsed contents of `rust-project.json`, or project json that's passed via
     ///            configuration.
-    pub fn new(base: &AbsPath, data: ProjectJsonData) -> ProjectJson {
-        ProjectJson {
-            sysroot: data.sysroot.map(|it| base.join(it)),
-            sysroot_src: data.sysroot_src.map(|it| base.join(it)),
+    pub fn new(base: &AbsPath, data: ProjectJsonData) -> Result<ProjectJson> {
+        let absolutize = |p| AbsPathBuf::try_from(p).map(|it| it.normalize());
+        Ok(ProjectJson {
+            sysroot: data
+                .sysroot
+                .map(absolutize)
+                .transpose()
+                .map_err(|path| {
+                    anyhow::format_err!(
+                        "sysroot path is required to be absolute: {}",
+                        path.display()
+                    )
+                })?,
+            sysroot_src: data
+                .sysroot_src
+                .map(absolutize)
+                .transpose()
+                .map_err(|path| {
+                    anyhow::format_err!(
+                        "sysroot-src path is required to be absolute: {}",
+                        path.display()
+                    )
+                })?,
             project_root: base.to_path_buf(),
             crates: data
                 .crates
                 .into_iter()
                 .map(|crate_data| {
-                    let is_workspace_member = crate_data.is_workspace_member.unwrap_or_else(|| {
-                        crate_data.root_module.is_relative()
-                            && !crate_data.root_module.starts_with("..")
-                            || crate_data.root_module.starts_with(base)
-                    });
-                    let root_module = base.join(crate_data.root_module).normalize();
+                    let root_module = absolutize(crate_data.root_module).map_err(|path| {
+                        anyhow::format_err!(
+                            "crate root module path is required to be absolute: {}",
+                            path.display()
+                        )
+                    })?;
+                    let is_workspace_member = crate_data
+                        .is_workspace_member
+                        .unwrap_or_else(|| root_module.starts_with(base));
                     let (include, exclude) = match crate_data.source {
                         Some(src) => {
                             let absolutize = |dirs: Vec<PathBuf>| {
                                 dirs.into_iter()
-                                    .map(|it| base.join(it).normalize())
-                                    .collect::<Vec<_>>()
+                                    .map(absolutize)
+                                    .collect::<Result<Vec<_>, _>>()
+                                    .map_err(|path| {
+                                        anyhow::format_err!(
+                                            "include and exclude paths are required to be absolute: {}",
+                                            path.display()
+                                        )
+                                    })
                             };
-                            (absolutize(src.include_dirs), absolutize(src.exclude_dirs))
+                            (absolutize(src.include_dirs)?, absolutize(src.exclude_dirs)?)
                         }
                         None => (vec![root_module.parent().unwrap().to_path_buf()], Vec::new()),
                     };
 
-                    Crate {
+                    Ok(Crate {
                         display_name: crate_data
                             .display_name
                             .map(CrateDisplayName::from_canonical_name),
@@ -147,16 +175,23 @@ impl ProjectJson {
                         env: crate_data.env,
                         proc_macro_dylib_path: crate_data
                             .proc_macro_dylib_path
-                            .map(|it| base.join(it)),
+                            .map(absolutize)
+                            .transpose()
+                            .map_err(|path| {
+                                anyhow::format_err!(
+                                    "crate proc macro dylib path is required to be absolute: {}",
+                                    path.display()
+                                )
+                            })?,
                         is_workspace_member,
                         include,
                         exclude,
                         is_proc_macro: crate_data.is_proc_macro,
                         repository: crate_data.repository,
-                    }
+                    })
                 })
-                .collect::<Vec<_>>(),
-        }
+                .collect::<Result<Vec<_>>>()?,
+        })
     }
 
     /// Returns the number of crates in the project.
@@ -243,7 +278,7 @@ struct CrateSource {
     exclude_dirs: Vec<PathBuf>,
 }
 
-fn deserialize_crate_name<'de, D>(de: D) -> Result<CrateName, D::Error>
+fn deserialize_crate_name<'de, D>(de: D) -> std::result::Result<CrateName, D::Error>
 where
     D: de::Deserializer<'de>,
 {

crates/project-model/src/tests.rs

@@ -133,7 +133,7 @@ fn rooted_project_json(data: ProjectJsonData) -> ProjectJson {
     replace_root(&mut root, true);
     let path = Path::new(&root);
     let base = AbsPath::assert(path);
-    ProjectJson::new(base, data)
+    ProjectJson::new(base, data).unwrap()
 }
 
 fn to_crate_graph(project_workspace: ProjectWorkspace) -> (CrateGraph, ProcMacroPaths) {

crates/project-model/src/workspace.rs

@@ -179,7 +179,6 @@ impl ProjectWorkspace {
         };
         let res = match manifest {
             ProjectManifest::ProjectJson(project_json) => {
-                let project_json = project_json.canonicalize()?;
                 let file = fs::read_to_string(&project_json).with_context(|| {
                     format!("Failed to read json file {}", project_json.display())
                 })?;
@@ -188,7 +187,7 @@ impl ProjectWorkspace {
                 })?;
                 let project_location = project_json.parent().to_path_buf();
                 let toolchain = version(&*project_location, toolchain::rustc(), "rustc ")?;
-                let project_json = ProjectJson::new(&project_location, data);
+                let project_json = ProjectJson::new(&project_location, data)?;
                 ProjectWorkspace::load_inline(
                     project_json,
                     config.target.as_deref(),

crates/rust-analyzer/src/config.rs

@@ -898,7 +898,7 @@ impl Config {
                             .map(Into::into)
                     }
                     ManifestOrProjectJson::ProjectJson(it) => {
-                        Some(ProjectJson::new(&self.root_path, it.clone()).into())
+                        ProjectJson::new(&self.root_path, it.clone()).ok().map(Into::into)
                     }
                 })
                 .collect(),