git: Prevent crashes when looking index text for empty path (#50487)

Cole Miller created 1 month ago

We were trying to mitigate these by passing `.` instead of ``, but it
turns out that git2 also panics internally for that. It also just
doesn't make sense to look up the index text (or committed text) for an
empty path, because a file should always have a nonempty repo path.

Closes ZED-560

Release Notes:

- N/A

Change summary

crates/git/src/repository.rs | 57 ++++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 23 deletions(-)

Detailed changes

crates/git/src/repository.rs

@@ -1318,33 +1318,31 @@ impl GitRepository for RealGitRepository {
         self.executor
             .spawn(async move {
                 fn logic(repo: &git2::Repository, path: &RepoPath) -> Result<Option<String>> {
-                    // This check is required because index.get_path() unwraps internally :(
                     let mut index = repo.index()?;
                     index.read(false)?;
 
                     const STAGE_NORMAL: i32 = 0;
-                    let path = path.as_std_path();
-                    // `RepoPath` contains a `RelPath` which normalizes `.` into an empty path
-                    // `get_path` unwraps on empty paths though, so undo that normalization here
-                    let path = if path.components().next().is_none() {
-                        ".".as_ref()
+                    // git2 unwraps internally on empty paths or `.`
+                    if path.is_empty() {
+                        bail!("empty path has no index text");
+                    }
+                    let entry = index
+                        .get_path(path.as_std_path(), STAGE_NORMAL)
+                        .with_context(|| format!("looking up {path:?} in index"))?;
+                    let oid = if entry.mode != GIT_MODE_SYMLINK {
+                        entry.id
                     } else {
-                        path
-                    };
-                    let oid = match index.get_path(path, STAGE_NORMAL) {
-                        Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id,
-                        _ => return Ok(None),
+                        return Ok(None);
                     };
 
                     let content = repo.find_blob(oid)?.content().to_owned();
                     Ok(String::from_utf8(content).ok())
                 }
 
-                match logic(&repo.lock(), &path) {
-                    Ok(value) => return value,
-                    Err(err) => log::error!("Error loading index text: {:?}", err),
-                }
-                None
+                logic(&repo.lock(), &path)
+                    .context("loading index text")
+                    .log_err()
+                    .flatten()
             })
             .boxed()
     }
@@ -1353,14 +1351,27 @@ impl GitRepository for RealGitRepository {
         let repo = self.repository.clone();
         self.executor
             .spawn(async move {
-                let repo = repo.lock();
-                let head = repo.head().ok()?.peel_to_tree().log_err()?;
-                let entry = head.get_path(path.as_std_path()).ok()?;
-                if entry.filemode() == i32::from(git2::FileMode::Link) {
-                    return None;
+                fn logic(repo: &git2::Repository, path: &RepoPath) -> Result<Option<String>> {
+                    let head = repo.head()?.peel_to_tree()?;
+                    if path.is_empty() {
+                        return Err(anyhow!("empty path has no committed text"));
+                    }
+                    // git2 unwraps internally on empty paths or `.`
+                    let entry = head.get_path(path.as_std_path())?;
+                    if entry.filemode() == i32::from(git2::FileMode::Link) {
+                        bail!(
+                            "symlink has no
+                committed text"
+                        );
+                    }
+                    let content = repo.find_blob(entry.id())?.content().to_owned();
+                    Ok(String::from_utf8(content).ok())
                 }
-                let content = repo.find_blob(entry.id()).log_err()?.content().to_owned();
-                String::from_utf8(content).ok()
+
+                logic(&repo.lock(), &path)
+                    .context("loading committed text")
+                    .log_err()
+                    .flatten()
             })
             .boxed()
     }