diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 5396027ef3d..ee841b95894 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -586,20 +586,23 @@ fn gix_open_opts_from_settings(settings: &UserSettings) -> gix::open::Options { } /// Parses the `jj:conflict-labels` header value if present. -fn extract_conflict_labels_from_commit(commit: &gix::objs::CommitRef) -> Merge { +fn extract_conflict_labels_from_commit(commit: &gix::objs::CommitRef) -> Result, ()> { let Some(value) = commit .extra_headers() .find(JJ_CONFLICT_LABELS_COMMIT_HEADER) else { - return Merge::resolved(String::new()); + return Ok(Merge::resolved(String::new())); }; - str::from_utf8(value) - .expect("labels should be valid utf8") + let labels = str::from_utf8(value) + .map_err(|_| ())? .split_terminator('\n') .map(str::to_owned) - .collect::>() - .build() + .collect_vec(); + if labels.len() == 1 || labels.len() % 2 == 0 { + return Err(()); + } + Ok(Merge::from_vec(labels)) } /// Parses the `jj:trees` header value if present, otherwise returns the @@ -656,7 +659,8 @@ fn commit_from_git_without_root_parent( }; // If the commit is a conflict, the conflict labels are stored in a commit // header separately from the trees. - let conflict_labels = extract_conflict_labels_from_commit(&commit); + let conflict_labels = extract_conflict_labels_from_commit(&commit) + .map_err(|()| to_read_object_err("Invalid jj:conflict-labels header", id))?; // Conflicted commits written before we started using the `jj:trees` header // (~March 2024) may have the root trees stored in the extra metadata table // instead. For such commits, we'll update the root tree later when we read the diff --git a/lib/tests/test_git_backend.rs b/lib/tests/test_git_backend.rs index aedadcd1e1b..e33bb343f76 100644 --- a/lib/tests/test_git_backend.rs +++ b/lib/tests/test_git_backend.rs @@ -19,13 +19,17 @@ use std::sync::Arc; use std::time::Duration; use std::time::SystemTime; +use assert_matches::assert_matches; use futures::executor::block_on_stream; use itertools::Itertools as _; +use jj_lib::backend::Backend as _; +use jj_lib::backend::BackendError; use jj_lib::backend::CommitId; use jj_lib::backend::CopyRecord; use jj_lib::commit::Commit; use jj_lib::conflict_labels::ConflictLabels; use jj_lib::git_backend::GitBackend; +use jj_lib::git_backend::JJ_CONFLICT_LABELS_COMMIT_HEADER; use jj_lib::git_backend::JJ_TREES_COMMIT_HEADER; use jj_lib::merge::Merge; use jj_lib::merged_tree::MergedTree; @@ -452,6 +456,52 @@ fn test_jj_trees_header_with_one_tree() -> TestResult { Ok(()) } +#[test] +fn test_invalid_conflict_labels_header() -> TestResult { + let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git); + let repo = test_repo.repo; + let git_backend = get_git_backend(&repo); + let git_repo = git_backend.git_repo(); + + let tree = create_single_tree(&repo, &[(repo_path("file"), "aaa")]); + let commit = commit_with_tree( + repo.store(), + MergedTree::resolved(repo.store().clone(), tree.id().clone()), + ); + let git_commit_id = gix::ObjectId::from_bytes_or_panic(commit.id().as_bytes()); + let git_commit = git_repo.find_commit(git_commit_id)?; + + // Invalid UTF-8 and an even number of labels cannot represent a merge. + for labels in [b"\xff".as_slice(), b"left\nright\n".as_slice()] { + let mut new_commit: gix::objs::Commit = git_commit.decode()?.try_into()?; + new_commit.extra_headers = vec![(JJ_CONFLICT_LABELS_COMMIT_HEADER.into(), labels.into())]; + let new_commit_id = git_repo.write_object(&new_commit)?; + let new_commit_id = CommitId::from_bytes(new_commit_id.as_bytes()); + + assert_matches!( + git_backend.import_head_commits(std::slice::from_ref(&new_commit_id)), + Err(BackendError::ReadObject { source, .. }) + if source.to_string() == "Invalid jj:conflict-labels header" + ); + } + + // The direct-read path rejects an even number of labels, too. + let mut new_commit: gix::objs::Commit = git_commit.decode()?.try_into()?; + new_commit.extra_headers = vec![( + JJ_CONFLICT_LABELS_COMMIT_HEADER.into(), + "left\nright\n".into(), + )]; + let new_commit_id = git_repo.write_object(&new_commit)?; + let new_commit_id = CommitId::from_bytes(new_commit_id.as_bytes()); + + assert_matches!( + git_backend.read_commit(&new_commit_id).block_on(), + Err(BackendError::ReadObject { source, .. }) + if source.to_string() == "Invalid jj:conflict-labels header" + ); + Ok(()) +} + #[test] fn test_conflict_headers_roundtrip() -> TestResult { let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);