consistently add context with file path when parsing fails

src/config.rs

@@ -641,11 +641,15 @@ impl Cfg {
             };
 
             if let Ok(contents) = contents {
-                let add_file_context = || format!("in {}", toolchain_file.to_string_lossy());
                 // XXX Should not return the unvalidated contents; but a new
                 // internal only safe struct
-                let override_file = Cfg::parse_override_file(contents, parse_mode)
-                    .with_context(add_file_context)?;
+                let override_file =
+                    Cfg::parse_override_file(contents, parse_mode).with_context(|| {
+                        RustupError::ParsingFile {
+                            name: "override",
+                            path: toolchain_file.clone(),
+                        }
+                    })?;
                 if let Some(toolchain_name_str) = &override_file.toolchain.channel {
                     let toolchain_name = ResolvableToolchainName::try_from(toolchain_name_str)?;
                     let default_host_triple = get_default_host_triple(settings);

src/dist/config.rs

@@ -43,7 +43,7 @@ impl Config {
     }
 
     pub(crate) fn parse(data: &str) -> Result<Self> {
-        let value = toml::from_str(data).context("error parsing manifest")?;
+        let value = toml::from_str(data).context("error parsing config")?;
         Self::from_toml(value, "")
     }
 

src/dist/dist.rs

@@ -1096,7 +1096,11 @@ pub(crate) async fn dl_v2_manifest(
                 return Ok(None);
             };
             let manifest_str = utils::read_file("manifest", &manifest_file)?;
-            let manifest = ManifestV2::parse(&manifest_str)?;
+            let manifest =
+                ManifestV2::parse(&manifest_str).with_context(|| RustupError::ParsingFile {
+                    name: "manifest",
+                    path: manifest_file.to_path_buf(),
+                })?;
 
             Ok(Some((manifest, manifest_hash)))
         }

src/dist/manifestation.rs

@@ -313,8 +313,12 @@ impl Manifestation {
 
         // Read configuration and delete it
         let rel_config_path = prefix.rel_manifest_file(CONFIG_FILE);
-        let config_str = utils::read_file("dist config", &prefix.path().join(&rel_config_path))?;
-        let config = Config::parse(&config_str)?;
+        let abs_config_path = prefix.path().join(&rel_config_path);
+        let config_str = utils::read_file("dist config", &abs_config_path)?;
+        let config = Config::parse(&config_str).with_context(|| RustupError::ParsingFile {
+            name: "config",
+            path: abs_config_path,
+        })?;
         tx.remove_file("dist config", rel_config_path)?;
 
         for component in config.components {
@@ -359,7 +363,12 @@ impl Manifestation {
         let config_path = prefix.path().join(rel_config_path);
         if utils::path_exists(&config_path) {
             let config_str = utils::read_file("dist config", &config_path)?;
-            Ok(Some(Config::parse(&config_str)?))
+            Ok(Some(Config::parse(&config_str).with_context(|| {
+                RustupError::ParsingFile {
+                    name: "Config",
+                    path: config_path,
+                }
+            })?))
         } else {
             Ok(None)
         }
@@ -371,7 +380,12 @@ impl Manifestation {
         let old_manifest_path = prefix.manifest_file(DIST_MANIFEST);
         if utils::path_exists(&old_manifest_path) {
             let manifest_str = utils::read_file("installed manifest", &old_manifest_path)?;
-            Ok(Some(Manifest::parse(&manifest_str)?))
+            Ok(Some(Manifest::parse(&manifest_str).with_context(|| {
+                RustupError::ParsingFile {
+                    name: "manifest",
+                    path: old_manifest_path,
+                }
+            })?))
         } else {
             Ok(None)
         }

src/errors.rs

@@ -71,6 +71,8 @@ pub(crate) enum RustupError {
     ReadingDirectory { name: &'static str, path: PathBuf },
     #[error("could not read {name} file: '{}'", .path.display())]
     ReadingFile { name: &'static str, path: PathBuf },
+    #[error("could not parse {name} file: '{}'", .path.display())]
+    ParsingFile { name: &'static str, path: PathBuf },
     #[error("could not remove '{}' directory: '{}'", .name, .path.display())]
     RemovingDirectory { name: &'static str, path: PathBuf },
     #[error("could not remove '{name}' file: '{}'", .path.display())]

src/settings.rs

@@ -43,7 +43,10 @@ impl SettingsFile {
                 drop(b);
                 *self.cache.borrow_mut() = Some(if utils::is_file(&self.path) {
                     let content = utils::read_file("settings", &self.path)?;
-                    Settings::parse(&content)?
+                    Settings::parse(&content).with_context(|| RustupError::ParsingFile {
+                        name: "settings",
+                        path: self.path.clone(),
+                    })?
                 } else {
                     needs_save = true;
                     Default::default()

tests/suite/cli_v2.rs

@@ -299,6 +299,48 @@ fn bad_sha_on_manifest() {
     });
 }
 
+#[test]
+fn bad_manifest() {
+    // issue #3851
+    setup(&|config| {
+        // install some toolchain
+        config.expect_ok(&["rustup", "update", "nightly"]);
+
+        #[cfg(not(target_os = "windows"))]
+        let path = format!(
+            "toolchains/nightly-{}/lib/rustlib/multirust-channel-manifest.toml",
+            this_host_triple(),
+        );
+
+        #[cfg(target_os = "windows")]
+        let path = format!(
+            r"toolchains\nightly-{}\lib/rustlib\multirust-channel-manifest.toml",
+            this_host_triple(),
+        );
+
+        assert!(config.rustupdir.has(&path));
+        let path = config.rustupdir.join(&path);
+
+        // corrupt the manifest file by inserting a NUL byte at some position
+        let old = fs::read_to_string(&path).unwrap();
+        let pattern = "[[pkg.rust.targ";
+        let (prefix, suffix) = old.split_once(pattern).unwrap();
+        let new = format!("{prefix}{pattern}\u{0}{suffix}");
+        fs::write(&path, new).unwrap();
+
+        // run some commands that try to load the manifest and
+        // check that the manifest parsing error includes the manifest file path
+        config.expect_err(
+            &["rustup", "check"],
+            &format!("error: could not parse manifest file: '{}'", path.display()),
+        );
+        config.expect_err(
+            &["cargo", "--help"],
+            &format!("error: could not parse manifest file: '{}'", path.display()),
+        );
+    });
+}
+
 #[test]
 fn bad_sha_on_installer() {
     setup(&|config| {