This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10748

dirstate-v2: Add a zero-size error type for dirstate v2 parse errors
ClosedPublic

Authored by SimonSapin on May 19 2021, 12:37 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG18b3060fe598: dirstate-v2: Add a zero-size error type for dirstate v2 parse errors
Summary

This error should only happen if Mercurial is buggy or the file is corrupted.
It indicates for example that:

  • A part of the file refers to another part, and the byte offset or item count would cause reading out of bounds, beyond the end of the file.
  • The byte for an entry state has an invalid value

When parsing becomes lazy, many more functions will return a Result with
this error. Making it zero-size reduces the work that the ? operator needs
to do to pass around the error value.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.May 19 2021, 12:37 PM
baymax updated this revision to Diff 28172.May 21 2021, 4:37 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare requested changes to this revision.May 21 2021, 5:41 AM
Alphare added a subscriber: Alphare.

The commit message passes a threshold of typos that makes it very hard to read. The change looks ok though

This revision now requires changes to proceed.May 21 2021, 5:41 AM
SimonSapin added a comment.May 21 2021, 6:44 AM

Oof sorry about that. I’ll reword it.

SimonSapin edited the summary of this revision. (Show Details)May 21 2021, 6:54 AM
baymax updated this revision to Diff 28271.May 30 2021, 3:27 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.May 31 2021, 3:41 AM
This revision is now accepted and ready to land.May 31 2021, 3:41 AM

Revision Contents
Changeset List

Diff 28283

rust/hg-core/src/dirstate_tree/dirstate_map.rs

nodes_with_copy_source_count: 0, nodes_with_copy_source_count: 0,
} }
} }
#[timed] #[timed]
pub fn new_v2( pub fn new_v2(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<(Self, Option<DirstateParents>), DirstateError> { ) -> Result<(Self, Option<DirstateParents>), DirstateError> {
on_disk::read(on_disk) Ok(on_disk::read(on_disk)?)
} }
#[timed] #[timed]
pub fn new_v1( pub fn new_v1(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<(Self, Option<DirstateParents>), DirstateError> { ) -> Result<(Self, Option<DirstateParents>), DirstateError> {
let mut map = Self::empty(on_disk); let mut map = Self::empty(on_disk);
if map.on_disk.is_empty() { if map.on_disk.is_empty() {

rust/hg-core/src/dirstate_tree/on_disk.rs

type OptPathSlice = Slice; type OptPathSlice = Slice;
/// Make sure that size-affecting changes are made knowingly /// Make sure that size-affecting changes are made knowingly
fn _static_assert_size_of() { fn _static_assert_size_of() {
let _ = std::mem::transmute::<Header, [u8; 72]>; let _ = std::mem::transmute::<Header, [u8; 72]>;
let _ = std::mem::transmute::<Node, [u8; 57]>; let _ = std::mem::transmute::<Node, [u8; 57]>;
} }
/// Unexpected file format found in `.hg/dirstate` with the "v2" format.
pub(crate) struct DirstateV2ParseError;
impl From<DirstateV2ParseError> for HgError {
fn from(_: DirstateV2ParseError) -> Self {
HgError::corrupted("dirstate-v2 parse error")
}
}
impl From<DirstateV2ParseError> for crate::DirstateError {
fn from(error: DirstateV2ParseError) -> Self {
HgError::from(error).into()
}
}
pub(super) fn read<'on_disk>( pub(super) fn read<'on_disk>(
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<(DirstateMap<'on_disk>, Option<DirstateParents>), DirstateError> { ) -> Result<
(DirstateMap<'on_disk>, Option<DirstateParents>),
DirstateV2ParseError,
> {
if on_disk.is_empty() { if on_disk.is_empty() {
return Ok((DirstateMap::empty(on_disk), None)); return Ok((DirstateMap::empty(on_disk), None));
} }
let (header, _) = Header::from_bytes(on_disk) let (header, _) =
.map_err(|_| HgError::corrupted("truncated dirstate-v2"))?; Header::from_bytes(on_disk).map_err(|_| DirstateV2ParseError)?;
let Header { let Header {
marker, marker,
parents, parents,
root, root,
nodes_with_entry_count, nodes_with_entry_count,
nodes_with_copy_source_count, nodes_with_copy_source_count,
} = header; } = header;
if marker != V2_FORMAT_MARKER { if marker != V2_FORMAT_MARKER {
return Err(HgError::corrupted("missing dirstated-v2 marker").into()); return Err(DirstateV2ParseError);
} }
let dirstate_map = DirstateMap { let dirstate_map = DirstateMap {
on_disk, on_disk,
root: read_nodes(on_disk, *root)?, root: read_nodes(on_disk, *root)?,
nodes_with_entry_count: nodes_with_entry_count.get(), nodes_with_entry_count: nodes_with_entry_count.get(),
nodes_with_copy_source_count: nodes_with_copy_source_count.get(), nodes_with_copy_source_count: nodes_with_copy_source_count.get(),
}; };
let parents = Some(parents.clone()); let parents = Some(parents.clone());
Ok((dirstate_map, parents)) Ok((dirstate_map, parents))
} }
impl Node { impl Node {
pub(super) fn path<'on_disk>( pub(super) fn path<'on_disk>(
&self, &self,
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<dirstate_map::NodeKey<'on_disk>, HgError> { ) -> Result<dirstate_map::NodeKey<'on_disk>, DirstateV2ParseError> {
let full_path = read_hg_path(on_disk, self.full_path)?; let full_path = read_hg_path(on_disk, self.full_path)?;
let base_name_start = usize::try_from(self.base_name_start.get()) let base_name_start = usize::try_from(self.base_name_start.get())
// u32 -> usize, could only panic on a 16-bit CPU // u32 -> usize, could only panic on a 16-bit CPU
.expect("dirstate-v2 base_name_start out of bounds"); .expect("dirstate-v2 base_name_start out of bounds");
if base_name_start < full_path.len() { if base_name_start < full_path.len() {
Ok(WithBasename::from_raw_parts(full_path, base_name_start)) Ok(WithBasename::from_raw_parts(full_path, base_name_start))
} else { } else {
Err(HgError::corrupted( Err(DirstateV2ParseError)
"dirstate-v2 base_name_start out of bounds",
))
} }
} }
pub(super) fn copy_source<'on_disk>( pub(super) fn copy_source<'on_disk>(
&self, &self,
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<Option<Cow<'on_disk, HgPath>>, HgError> { ) -> Result<Option<Cow<'on_disk, HgPath>>, DirstateV2ParseError> {
Ok(if self.copy_source.start.get() != 0 { Ok(if self.copy_source.start.get() != 0 {
Some(read_hg_path(on_disk, self.copy_source)?) Some(read_hg_path(on_disk, self.copy_source)?)
} else { } else {
None None
}) })
} }
pub(super) fn entry(&self) -> Result<Option<DirstateEntry>, HgError> { pub(super) fn entry(
&self,
) -> Result<Option<DirstateEntry>, DirstateV2ParseError> {
Ok(if self.entry.state != b'\0' { Ok(if self.entry.state != b'\0' {
Some(DirstateEntry { Some(DirstateEntry {
state: self.entry.state.try_into()?, state: self
.entry
.state
.try_into()
.map_err(|_| DirstateV2ParseError)?,
mode: self.entry.mode.get(), mode: self.entry.mode.get(),
mtime: self.entry.mtime.get(), mtime: self.entry.mtime.get(),
size: self.entry.size.get(), size: self.entry.size.get(),
}) })
} else { } else {
None None
}) })
} }
pub(super) fn to_in_memory_node<'on_disk>( pub(super) fn to_in_memory_node<'on_disk>(
&self, &self,
on_disk: &'on_disk [u8], on_disk: &'on_disk [u8],
) -> Result<dirstate_map::Node<'on_disk>, HgError> { ) -> Result<dirstate_map::Node<'on_disk>, DirstateV2ParseError> {
Ok(dirstate_map::Node { Ok(dirstate_map::Node {
children: read_nodes(on_disk, self.children)?, children: read_nodes(on_disk, self.children)?,
copy_source: self.copy_source(on_disk)?, copy_source: self.copy_source(on_disk)?,
entry: self.entry()?, entry: self.entry()?,
tracked_descendants_count: self.tracked_descendants_count.get(), tracked_descendants_count: self.tracked_descendants_count.get(),
}) })
} }
} }
fn read_nodes( fn read_nodes(
on_disk: &[u8], on_disk: &[u8],
slice: ChildNodes, slice: ChildNodes,
) -> Result<dirstate_map::ChildNodes, HgError> { ) -> Result<dirstate_map::ChildNodes, DirstateV2ParseError> {
read_slice::<Node>(on_disk, slice)? read_slice::<Node>(on_disk, slice)?
.iter() .iter()
.map(|node| { .map(|node| {
Ok((node.path(on_disk)?, node.to_in_memory_node(on_disk)?)) Ok((node.path(on_disk)?, node.to_in_memory_node(on_disk)?))
}) })
.collect::<Result<_, _>>() .collect::<Result<_, _>>()
.map(dirstate_map::ChildNodes::InMemory) .map(dirstate_map::ChildNodes::InMemory)
} }
fn read_hg_path(on_disk: &[u8], slice: Slice) -> Result<Cow<HgPath>, HgError> { fn read_hg_path(
on_disk: &[u8],
slice: Slice,
) -> Result<Cow<HgPath>, DirstateV2ParseError> {
let bytes = read_slice::<u8>(on_disk, slice)?; let bytes = read_slice::<u8>(on_disk, slice)?;
Ok(Cow::Borrowed(HgPath::new(bytes))) Ok(Cow::Borrowed(HgPath::new(bytes)))
} }
fn read_slice<T>(on_disk: &[u8], slice: Slice) -> Result<&[T], HgError> fn read_slice<T>(
on_disk: &[u8],
slice: Slice,
) -> Result<&[T], DirstateV2ParseError>
where where
T: BytesCast, T: BytesCast,
{ {
// Either `usize::MAX` would result in "out of bounds" error since a single // Either `usize::MAX` would result in "out of bounds" error since a single
// `&[u8]` cannot occupy the entire addess space. // `&[u8]` cannot occupy the entire addess space.
let start = usize::try_from(slice.start.get()).unwrap_or(std::usize::MAX); let start = usize::try_from(slice.start.get()).unwrap_or(std::usize::MAX);
let len = usize::try_from(slice.len.get()).unwrap_or(std::usize::MAX); let len = usize::try_from(slice.len.get()).unwrap_or(std::usize::MAX);
on_disk on_disk
.get(start..) .get(start..)
.and_then(|bytes| T::slice_from_bytes(bytes, len).ok()) .and_then(|bytes| T::slice_from_bytes(bytes, len).ok())
.map(|(slice, _rest)| slice) .map(|(slice, _rest)| slice)
.ok_or_else(|| { .ok_or_else(|| DirstateV2ParseError)
HgError::corrupted("dirstate v2 slice is out of bounds")
})
} }
pub(super) fn write( pub(super) fn write(
dirstate_map: &mut DirstateMap, dirstate_map: &mut DirstateMap,
parents: DirstateParents, parents: DirstateParents,
) -> Result<Vec<u8>, DirstateError> { ) -> Result<Vec<u8>, DirstateError> {
let header_len = std::mem::size_of::<Header>(); let header_len = std::mem::size_of::<Header>();