Skip to content

Commit

Permalink
fix!: don't panic when unknown entry types are encountered.
Browse files Browse the repository at this point in the history
Related to helix-editor/helix#10660
which runs into object types that are unknown.

I have looked into this and [couldn't find evidence of a new pack-entry type](https://github.com/git/git/blob/0f3415f1f8478b05e64db11eb8aaa2915e48fef6/packfile.c#L1303-L1358)
in the Git codebase.

It also looks like that Git [will never write packs that aren't V2](https://github.com/git/git/blob/0f3415f1f8478b05e64db11eb8aaa2915e48fef6/pack-write.c#L352)
 - initially I thought it might write V3 based on some other criteria.

For now, the thesis is that one would have to be able to mark bad objects
to be able to handle this more gracefully, but all we can do is try to fail.
  • Loading branch information
Byron committed May 12, 2024
1 parent addf446 commit b32a847
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 35 deletions.
12 changes: 6 additions & 6 deletions gix-pack/src/bundle/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ impl crate::Bundle {
cache: &mut dyn crate::cache::DecodeEntry,
) -> Result<(gix_object::Data<'a>, crate::data::entry::Location), crate::data::decode::Error> {
let ofs = self.index.pack_offset_at_index(idx);
let pack_entry = self.pack.entry(ofs);
let pack_entry = self.pack.entry(ofs)?;
let header_size = pack_entry.header_size();
self.pack
.decode_entry(
pack_entry,
out,
inflate,
&|id, _out| {
self.index.lookup(id).map(|idx| {
crate::data::decode::entry::ResolvedBase::InPack(
self.pack.entry(self.index.pack_offset_at_index(idx)),
)
})
let idx = self.index.lookup(id)?;
self.pack
.entry(self.index.pack_offset_at_index(idx))
.ok()
.map(crate::data::decode::entry::ResolvedBase::InPack)
},
cache,
)
Expand Down
2 changes: 2 additions & 0 deletions gix-pack/src/cache/delta/traverse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub enum Error {
},
#[error("The resolver failed to obtain the pack entry bytes for the entry at {pack_offset}")]
ResolveFailed { pack_offset: u64 },
#[error(transparent)]
EntryType(#[from] crate::data::entry::decode::Error),
#[error("One of the object inspectors failed")]
Inspect(#[from] Box<dyn std::error::Error + Send + Sync>),
#[error("Interrupted")]
Expand Down
4 changes: 2 additions & 2 deletions gix-pack/src/cache/delta/traverse/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ where
let bytes = resolve(slice.clone(), resolve_data).ok_or(Error::ResolveFailed {
pack_offset: slice.start,
})?;
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len);
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len)?;
let compressed = &bytes[entry.header_size()..];
let decompressed_len = entry.decompressed_size as usize;
decompress_all_at_once_with(&mut inflate, compressed, decompressed_len, out)?;
Expand Down Expand Up @@ -304,7 +304,7 @@ where
let bytes = resolve(slice.clone(), resolve_data).ok_or(Error::ResolveFailed {
pack_offset: slice.start,
})?;
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len);
let entry = data::Entry::from_bytes(bytes, slice.start, hash_len)?;
let compressed = &bytes[entry.header_size()..];
let decompressed_len = entry.decompressed_size as usize;
decompress_all_at_once_with(&mut inflate, compressed, decompressed_len, out)?;
Expand Down
29 changes: 19 additions & 10 deletions gix-pack/src/data/entry/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@ use gix_features::decode::{leb64, leb64_from_read};
use super::{BLOB, COMMIT, OFS_DELTA, REF_DELTA, TAG, TREE};
use crate::data;

/// The error returned by [data::Entry::from_bytes()].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
#[error("Object type {type_id} is unsupported")]
pub struct Error {
pub type_id: u8,
}

/// Decoding
impl data::Entry {
/// Decode an entry from the given entry data `d`, providing the `pack_offset` to allow tracking the start of the entry data section.
///
/// # Panics
///
/// If we cannot understand the header, garbage data is likely to trigger this.
pub fn from_bytes(d: &[u8], pack_offset: data::Offset, hash_len: usize) -> data::Entry {
pub fn from_bytes(d: &[u8], pack_offset: data::Offset, hash_len: usize) -> Result<data::Entry, Error> {
let (type_id, size, mut consumed) = parse_header_info(d);

use crate::data::entry::Header::*;
Expand All @@ -36,21 +44,17 @@ impl data::Entry {
TREE => Tree,
COMMIT => Commit,
TAG => Tag,
_ => panic!("We currently don't support any V3 features or extensions"),
other => return Err(Error { type_id: other }),
};
data::Entry {
Ok(data::Entry {
header: object,
decompressed_size: size,
data_offset: pack_offset + consumed as u64,
}
})
}

/// Instantiate an `Entry` from the reader `r`, providing the `pack_offset` to allow tracking the start of the entry data section.
pub fn from_read(
r: &mut dyn io::Read,
pack_offset: data::Offset,
hash_len: usize,
) -> Result<data::Entry, io::Error> {
pub fn from_read(r: &mut dyn io::Read, pack_offset: data::Offset, hash_len: usize) -> io::Result<data::Entry> {
let (type_id, size, mut consumed) = streaming_parse_header_info(r)?;

use crate::data::entry::Header::*;
Expand Down Expand Up @@ -78,7 +82,12 @@ impl data::Entry {
TREE => Tree,
COMMIT => Commit,
TAG => Tag,
_ => panic!("We currently don't support any V3 features or extensions"),
other => {
return Err(io::Error::new(
io::ErrorKind::Other,
format!("Object type {other} is unsupported"),
))
}
};
Ok(data::Entry {
header: object,
Expand Down
4 changes: 3 additions & 1 deletion gix-pack/src/data/entry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ impl Entry {
}
}

mod decode;
///
#[allow(clippy::empty_docs)]
pub mod decode;

mod header;
pub use header::Header;
Expand Down
12 changes: 2 additions & 10 deletions gix-pack/src/data/file/decode/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,10 @@ impl File {
.map_err(Into::into)
}

fn assure_v2(&self) {
assert!(
matches!(self.version, crate::data::Version::V2),
"Only V2 is implemented"
);
}

/// Obtain the [`Entry`][crate::data::Entry] at the given `offset` into the pack.
///
/// The `offset` is typically obtained from the pack index file.
pub fn entry(&self, offset: data::Offset) -> data::Entry {
self.assure_v2();
pub fn entry(&self, offset: data::Offset) -> Result<data::Entry, data::entry::decode::Error> {
let pack_offset: usize = offset.try_into().expect("offset representable by machine");
assert!(pack_offset <= self.data.len(), "offset out of bounds");

Expand Down Expand Up @@ -246,7 +238,7 @@ impl File {
});
use crate::data::entry::Header;
cursor = match cursor.header {
Header::OfsDelta { base_distance } => self.entry(cursor.base_pack_offset(base_distance)),
Header::OfsDelta { base_distance } => self.entry(cursor.base_pack_offset(base_distance))?,
Header::RefDelta { base_id } => match resolve(base_id.as_ref(), out) {
Some(ResolvedBase::InPack(entry)) => entry,
Some(ResolvedBase::OutOfPack { end, kind }) => {
Expand Down
2 changes: 1 addition & 1 deletion gix-pack/src/data/file/decode/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl File {
if first_delta_decompressed_size.is_none() {
first_delta_decompressed_size = Some(self.decode_delta_object_size(inflate, &entry)?);
}
entry = self.entry(entry.base_pack_offset(base_distance))
entry = self.entry(entry.base_pack_offset(base_distance))?
}
RefDelta { base_id } => {
num_deltas += 1;
Expand Down
2 changes: 2 additions & 0 deletions gix-pack/src/data/file/decode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub enum Error {
ZlibInflate(#[from] gix_features::zlib::inflate::Error),
#[error("A delta chain could not be followed as the ref base with id {0} could not be found")]
DeltaBaseUnresolved(gix_hash::ObjectId),
#[error(transparent)]
EntryType(#[from] crate::data::entry::decode::Error),
#[error("Entry too large to fit in memory")]
OutOfMemory,
}
Expand Down
8 changes: 7 additions & 1 deletion gix-pack/src/data/output/entry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub enum Kind {
pub enum Error {
#[error("{0}")]
ZlibDeflate(#[from] std::io::Error),
#[error(transparent)]
EntryType(#[from] crate::data::entry::decode::Error),
}

impl output::Entry {
Expand Down Expand Up @@ -74,7 +76,11 @@ impl output::Entry {
};

let pack_offset_must_be_zero = 0;
let pack_entry = data::Entry::from_bytes(&entry.data, pack_offset_must_be_zero, count.id.as_slice().len());
let pack_entry = match data::Entry::from_bytes(&entry.data, pack_offset_must_be_zero, count.id.as_slice().len())
{
Ok(e) => e,
Err(err) => return Some(Err(err.into())),
};

use crate::data::entry::Header::*;
match pack_entry.header {
Expand Down
2 changes: 2 additions & 0 deletions gix-pack/src/index/traverse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ pub enum Error<E: std::error::Error + Send + Sync + 'static> {
Tree(#[from] crate::cache::delta::from_offsets::Error),
#[error("The tree traversal failed")]
TreeTraversal(#[from] crate::cache::delta::traverse::Error),
#[error(transparent)]
EntryType(#[from] crate::data::entry::decode::Error),
#[error("Object {id} at offset {offset} could not be decoded")]
PackDecode {
id: gix_hash::ObjectId,
Expand Down
9 changes: 5 additions & 4 deletions gix-pack/src/index/traverse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,18 @@ impl index::File {
C: crate::cache::DecodeEntry,
E: std::error::Error + Send + Sync + 'static,
{
let pack_entry = pack.entry(index_entry.pack_offset);
let pack_entry = pack.entry(index_entry.pack_offset)?;
let pack_entry_data_offset = pack_entry.data_offset;
let entry_stats = pack
.decode_entry(
pack_entry,
buf,
inflate,
&|id, _| {
self.lookup(id).map(|index| {
crate::data::decode::entry::ResolvedBase::InPack(pack.entry(self.pack_offset_at_index(index)))
})
let index = self.lookup(id)?;
pack.entry(self.pack_offset_at_index(index))
.ok()
.map(crate::data::decode::entry::ResolvedBase::InPack)
},
cache,
)
Expand Down
1 change: 1 addition & 0 deletions gix-pack/src/multi_index/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ impl File {
TreeTraversal(err) => TreeTraversal(err),
PackDecode { id, offset, source } => PackDecode { id, offset, source },
PackMismatch { expected, actual } => PackMismatch { expected, actual },
EntryType(err) => EntryType(err),
PackObjectMismatch {
expected,
actual,
Expand Down

0 comments on commit b32a847

Please sign in to comment.