This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11962

rhg: RevlogEntry::uncompressed_len is signed
ClosedPublic

Authored by SimonSapin on Jan 6 2022, 1:56 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG0a4ac916673e: rhg: RevlogEntry::uncompressed_len is signed
Summary

The corresponding Python code appears to explicitly check for non-negative values.

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.Jan 6 2022, 1:56 PM
Alphare accepted this revision.Jan 7 2022, 4:37 AM
Alphare added a subscriber: Alphare.

Probably vestigial, but I agree it's safer to do this

This revision is now accepted and ready to land.Jan 7 2022, 4:37 AM
SimonSapin added a commit: rHG0a4ac916673e: rhg: RevlogEntry::uncompressed_len is signed.Jan 7 2022, 4:52 AM
Closed by commit rHG0a4ac916673e: rhg: RevlogEntry::uncompressed_len is signed (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 31614

rust/hg-core/src/revlog/index.rs

while offset + INDEX_ENTRY_SIZE <= bytes.len() { while offset + INDEX_ENTRY_SIZE <= bytes.len() {
offsets.push(offset); offsets.push(offset);
let end = offset + INDEX_ENTRY_SIZE; let end = offset + INDEX_ENTRY_SIZE;
let entry = IndexEntry { let entry = IndexEntry {
bytes: &bytes[offset..end], bytes: &bytes[offset..end],
offset_override: None, offset_override: None,
}; };
offset += INDEX_ENTRY_SIZE + entry.compressed_len(); offset += INDEX_ENTRY_SIZE + entry.compressed_len() as usize;
} }
if offset == bytes.len() { if offset == bytes.len() {
Ok(Self { Ok(Self {
bytes, bytes,
offsets: Some(offsets), offsets: Some(offsets),
uses_generaldelta, uses_generaldelta,
}) })
} else { } else {
let mut bytes = [0; 8]; let mut bytes = [0; 8];
bytes[2..8].copy_from_slice(&self.bytes[0..=5]); bytes[2..8].copy_from_slice(&self.bytes[0..=5]);
BigEndian::read_u64(&bytes[..]) as usize BigEndian::read_u64(&bytes[..]) as usize
} }
} }
/// Return the compressed length of the data. /// Return the compressed length of the data.
pub fn compressed_len(&self) -> usize { pub fn compressed_len(&self) -> u32 {
BigEndian::read_u32(&self.bytes[8..=11]) as usize BigEndian::read_u32(&self.bytes[8..=11])
} }
/// Return the uncompressed length of the data. /// Return the uncompressed length of the data.
pub fn uncompressed_len(&self) -> usize { pub fn uncompressed_len(&self) -> i32 {
BigEndian::read_u32(&self.bytes[12..=15]) as usize BigEndian::read_i32(&self.bytes[12..=15])
} }
/// Return the revision upon which the data has been derived. /// Return the revision upon which the data has been derived.
pub fn base_revision_or_base_of_delta_chain(&self) -> Revision { pub fn base_revision_or_base_of_delta_chain(&self) -> Revision {
// TODO Maybe return an Option when base_revision == rev? // TODO Maybe return an Option when base_revision == rev?
// Requires to add rev to IndexEntry // Requires to add rev to IndexEntry
BigEndian::read_i32(&self.bytes[16..]) BigEndian::read_i32(&self.bytes[16..])

rust/hg-core/src/revlog/revlog.rs

use std::borrow::Cow; use std::borrow::Cow;
use std::convert::TryFrom;
use std::io::Read; use std::io::Read;
use std::ops::Deref; use std::ops::Deref;
use std::path::Path; use std::path::Path;
use flate2::read::ZlibDecoder; use flate2::read::ZlibDecoder;
use micro_timer::timed; use micro_timer::timed;
use sha1::{Digest, Sha1}; use sha1::{Digest, Sha1};
use zstd; use zstd;
&self, &self,
rev: Revision, rev: Revision,
) -> Result<RevlogEntry, RevlogError> { ) -> Result<RevlogEntry, RevlogError> {
let index_entry = self let index_entry = self
.index .index
.get_entry(rev) .get_entry(rev)
.ok_or(RevlogError::InvalidRevision)?; .ok_or(RevlogError::InvalidRevision)?;
let start = index_entry.offset(); let start = index_entry.offset();
let end = start + index_entry.compressed_len(); let end = start + index_entry.compressed_len() as usize;
let data = if self.index.is_inline() { let data = if self.index.is_inline() {
self.index.data(start, end) self.index.data(start, end)
} else { } else {
&self.data()[start..end] &self.data()[start..end]
}; };
let entry = RevlogEntry { let entry = RevlogEntry {
revlog: self, revlog: self,
rev, rev,
/// The revlog entry's bytes and the necessary informations to extract /// The revlog entry's bytes and the necessary informations to extract
/// the entry's data. /// the entry's data.
#[derive(Clone)] #[derive(Clone)]
pub struct RevlogEntry<'a> { pub struct RevlogEntry<'a> {
revlog: &'a Revlog, revlog: &'a Revlog,
rev: Revision, rev: Revision,
bytes: &'a [u8], bytes: &'a [u8],
compressed_len: usize, compressed_len: u32,
uncompressed_len: usize, uncompressed_len: i32,
base_rev_or_base_of_delta_chain: Option<Revision>, base_rev_or_base_of_delta_chain: Option<Revision>,
} }
impl<'a> RevlogEntry<'a> { impl<'a> RevlogEntry<'a> {
pub fn revision(&self) -> Revision { pub fn revision(&self) -> Revision {
self.rev self.rev
} }
pub fn uncompressed_len(&self) -> Option<u32> {
u32::try_from(self.uncompressed_len).ok()
}
/// The data for this entry, after resolving deltas if any. /// The data for this entry, after resolving deltas if any.
pub fn data(&self) -> Result<Cow<'a, [u8]>, HgError> { pub fn data(&self) -> Result<Cow<'a, [u8]>, HgError> {
let mut entry = self.clone(); let mut entry = self.clone();
let mut delta_chain = vec![]; let mut delta_chain = vec![];
// The meaning of `base_rev_or_base_of_delta_chain` depends on // The meaning of `base_rev_or_base_of_delta_chain` depends on
// generaldelta. See the doc on `ENTRY_DELTA_BASE` in // generaldelta. See the doc on `ENTRY_DELTA_BASE` in
// `mercurial/revlogutils/constants.py` and the code in // `mercurial/revlogutils/constants.py` and the code in
// A proper new format should have had a repo/store requirement. // A proper new format should have had a repo/store requirement.
_format_type => Err(corrupted()), _format_type => Err(corrupted()),
} }
} }
fn uncompressed_zlib_data(&self) -> Result<Vec<u8>, HgError> { fn uncompressed_zlib_data(&self) -> Result<Vec<u8>, HgError> {
let mut decoder = ZlibDecoder::new(self.bytes); let mut decoder = ZlibDecoder::new(self.bytes);
if self.is_delta() { if self.is_delta() {
let mut buf = Vec::with_capacity(self.compressed_len); let mut buf = Vec::with_capacity(self.compressed_len as usize);
decoder.read_to_end(&mut buf).map_err(|_| corrupted())?; decoder.read_to_end(&mut buf).map_err(|_| corrupted())?;
Ok(buf) Ok(buf)
} else { } else {
let mut buf = vec![0; self.uncompressed_len]; let cap = self.uncompressed_len.max(0) as usize;
let mut buf = vec![0; cap];
decoder.read_exact(&mut buf).map_err(|_| corrupted())?; decoder.read_exact(&mut buf).map_err(|_| corrupted())?;
Ok(buf) Ok(buf)
} }
} }
fn uncompressed_zstd_data(&self) -> Result<Vec<u8>, HgError> { fn uncompressed_zstd_data(&self) -> Result<Vec<u8>, HgError> {
if self.is_delta() { if self.is_delta() {
let mut buf = Vec::with_capacity(self.compressed_len); let mut buf = Vec::with_capacity(self.compressed_len as usize);
zstd::stream::copy_decode(self.bytes, &mut buf) zstd::stream::copy_decode(self.bytes, &mut buf)
.map_err(|_| corrupted())?; .map_err(|_| corrupted())?;
Ok(buf) Ok(buf)
} else { } else {
let mut buf = vec![0; self.uncompressed_len]; let cap = self.uncompressed_len.max(0) as usize;
let mut buf = vec![0; cap];
let len = zstd::block::decompress_to_buffer(self.bytes, &mut buf) let len = zstd::block::decompress_to_buffer(self.bytes, &mut buf)
.map_err(|_| corrupted())?; .map_err(|_| corrupted())?;
if len != self.uncompressed_len { if len != self.uncompressed_len as usize {
Err(corrupted()) Err(corrupted())
} else { } else {
Ok(buf) Ok(buf)
} }
} }
} }
/// Tell if the entry is a snapshot or a delta /// Tell if the entry is a snapshot or a delta