This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9100

hg-core: return `Err` on decompression error (D8958#inline-15004 followup)
ClosedPublic

Authored by acezar on Sep 28 2020, 9:48 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG497657895b54: hg-core: return `Err` on decompression error (D8958#inline-15004 followup)

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

acezar created this revision.Sep 28 2020, 9:48 AM
Alphare requested changes to this revision.Sep 28 2020, 10:18 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/revlog/revlog.rs
263

.map(RevlogError::Corrupted) is simpler and just as effective.

This revision now requires changes to proceed.Sep 28 2020, 10:18 AM
Alphare added inline comments.Sep 28 2020, 10:32 AM
rust/hg-core/src/revlog/revlog.rs
263

Sorry .or(Err(RevlogError::Corrupted)). I could expect the optimizer to take care of the closure, but this is more explicit.

acezar updated this revision to Diff 22895.Sep 28 2020, 10:33 AM
acezar marked 2 inline comments as done.Sep 28 2020, 10:34 AM

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/revlog/revlog.rs (33 lines)

Diff 22886

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

} }
match self.bytes[0] { match self.bytes[0] {
// Revision data is the entirety of the entry, including this // Revision data is the entirety of the entry, including this
// header. // header.
b'\0' => Ok(Cow::Borrowed(self.bytes)), b'\0' => Ok(Cow::Borrowed(self.bytes)),
// Raw revision data follows. // Raw revision data follows.
b'u' => Ok(Cow::Borrowed(&self.bytes[1..])), b'u' => Ok(Cow::Borrowed(&self.bytes[1..])),
// zlib (RFC 1950) data. // zlib (RFC 1950) data.
b'x' => Ok(Cow::Owned(self.uncompressed_zlib_data())), b'x' => Ok(Cow::Owned(self.uncompressed_zlib_data()?)),
// zstd data. // zstd data.
b'\x28' => Ok(Cow::Owned(self.uncompressed_zstd_data())), b'\x28' => Ok(Cow::Owned(self.uncompressed_zstd_data()?)),
format_type => Err(RevlogError::UnknowDataFormat(format_type)), format_type => Err(RevlogError::UnknowDataFormat(format_type)),
} }
} }
fn uncompressed_zlib_data(&self) -> Vec<u8> { fn uncompressed_zlib_data(&self) -> Result<Vec<u8>, RevlogError> {
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);
decoder.read_to_end(&mut buf).expect("corrupted zlib data"); decoder
buf .read_to_end(&mut buf)
.map_err(|_| RevlogError::Corrupted)?;
AlphareUnsubmitted
Done

.map(RevlogError::Corrupted) is simpler and just as effective.

Alphare: `.map(RevlogError::Corrupted)` is simpler and just as effective.
AlphareUnsubmitted
Done

Sorry .or(Err(RevlogError::Corrupted)). I could expect the optimizer to take care of the closure, but this is more explicit.

Alphare: Sorry `.or(Err(RevlogError::Corrupted))`. I could expect the optimizer to take care of the…
Ok(buf)
} else { } else {
let mut buf = vec![0; self.uncompressed_len]; let mut buf = vec![0; self.uncompressed_len];
decoder.read_exact(&mut buf).expect("corrupted zlib data"); decoder
buf .read_exact(&mut buf)
.map_err(|_| RevlogError::Corrupted)?;
Ok(buf)
} }
} }
fn uncompressed_zstd_data(&self) -> Vec<u8> { fn uncompressed_zstd_data(&self) -> Result<Vec<u8>, RevlogError> {
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);
zstd::stream::copy_decode(self.bytes, &mut buf) zstd::stream::copy_decode(self.bytes, &mut buf)
.expect("corrupted zstd data"); .map_err(|_| RevlogError::Corrupted)?;
buf Ok(buf)
} else { } else {
let mut buf = vec![0; self.uncompressed_len]; let mut buf = vec![0; self.uncompressed_len];
let len = zstd::block::decompress_to_buffer(self.bytes, &mut buf) let len = zstd::block::decompress_to_buffer(self.bytes, &mut buf)
.expect("corrupted zstd data"); .map_err(|_| RevlogError::Corrupted)?;
assert_eq!(len, self.uncompressed_len, "corrupted zstd data"); if len != self.uncompressed_len {
buf Err(RevlogError::Corrupted)
} else {
Ok(buf)
}
} }
} }
/// Tell if the entry is a snapshot or a delta /// Tell if the entry is a snapshot or a delta
/// (influences on decompression). /// (influences on decompression).
fn is_delta(&self) -> bool { fn is_delta(&self) -> bool {
self.base_rev.is_some() self.base_rev.is_some()
} }