Skip to content

Commit

Permalink
really test decode_varint_slow (#977)
Browse files Browse the repository at this point in the history
* really test decode_varint_slow instead of passing it slices that have already been emptied

* reborrow with methods not &*

---------

Co-authored-by: Lucio Franco <luciofranco14@gmail.com>
  • Loading branch information
mumbleskates and LucioFranco committed Feb 28, 2024
1 parent a4538f2 commit 9353407
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 22 deletions.
4 changes: 2 additions & 2 deletions conformance/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn main() -> io::Result<()> {
bytes.resize(len, 0);
io::stdin().read_exact(&mut bytes)?;

let result = match ConformanceRequest::decode(&*bytes) {
let result = match ConformanceRequest::decode(bytes.as_slice()) {
Ok(request) => handle_request(request),
Err(error) => conformance_response::Result::ParseError(format!("{:?}", error)),
};
Expand Down Expand Up @@ -93,7 +93,7 @@ fn handle_request(request: ConformanceRequest) -> conformance_response::Result {
Some(conformance_request::Payload::ProtobufPayload(buf)) => buf,
};

let roundtrip = match &*request.message_type {
let roundtrip = match request.message_type.as_str() {
"protobuf_test_messages.proto2.TestAllTypesProto2" => roundtrip::<TestAllTypesProto2>(&buf),
"protobuf_test_messages.proto3.TestAllTypesProto3" => roundtrip::<TestAllTypesProto3>(&buf),
_ => {
Expand Down
2 changes: 1 addition & 1 deletion prost-build/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ where
fn map_codeblock(kind: CodeBlockKind) -> CodeBlockKind {
match kind {
CodeBlockKind::Fenced(s) => {
if &*s == "rust" {
if s.as_ref() == "rust" {
CodeBlockKind::Fenced("compile_fail".into())
} else {
CodeBlockKind::Fenced(format!("text,{}", s).into())
Expand Down
2 changes: 1 addition & 1 deletion prost-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,7 @@ impl Config {
),
)
})?;
let file_descriptor_set = FileDescriptorSet::decode(&*buf).map_err(|error| {
let file_descriptor_set = FileDescriptorSet::decode(buf.as_slice()).map_err(|error| {
Error::new(
ErrorKind::InvalidInput,
format!("invalid FileDescriptorSet: {}", error),
Expand Down
2 changes: 1 addition & 1 deletion prost-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Any {
) {
(Some(expected), Some(actual)) => {
if expected == actual {
return Ok(M::decode(&*self.value)?);
return Ok(M::decode(self.value.as_slice())?);
}
}
_ => (),
Expand Down
2 changes: 1 addition & 1 deletion protobuf/benches/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn load_dataset(dataset: &Path) -> Result<BenchmarkDataset, Box<dyn Error>> {
let mut f = File::open(dataset)?;
let mut buf = Vec::new();
f.read_to_end(&mut buf)?;
Ok(BenchmarkDataset::decode(&*buf)?)
Ok(BenchmarkDataset::decode(buf.as_slice())?)
}

fn benchmark_dataset<M>(criterion: &mut Criterion, name: &str, dataset: &'static Path)
Expand Down
15 changes: 7 additions & 8 deletions src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1602,7 +1602,7 @@ mod test {

#[test]
fn varint() {
fn check(value: u64, mut encoded: &[u8]) {
fn check(value: u64, encoded: &[u8]) {
// Small buffer.
let mut buf = Vec::with_capacity(1);
encode_varint(value, &mut buf);
Expand All @@ -1615,11 +1615,11 @@ mod test {

assert_eq!(encoded_len_varint(value), encoded.len());

let roundtrip_value =
decode_varint(&mut <&[u8]>::clone(&encoded)).expect("decoding failed");
let roundtrip_value = decode_varint(&mut encoded.clone()).expect("decoding failed");
assert_eq!(value, roundtrip_value);

let roundtrip_value = decode_varint_slow(&mut encoded).expect("slow decoding failed");
let roundtrip_value =
decode_varint_slow(&mut encoded.clone()).expect("slow decoding failed");
assert_eq!(value, roundtrip_value);
}

Expand Down Expand Up @@ -1680,11 +1680,10 @@ mod test {

#[test]
fn varint_overflow() {
let mut u64_max_plus_one: &[u8] =
&[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x02];
let u64_max_plus_one: &[u8] = &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x02];

decode_varint(&mut u64_max_plus_one).expect_err("decoding u64::MAX + 1 succeeded");
decode_varint_slow(&mut u64_max_plus_one)
decode_varint(&mut u64_max_plus_one.clone()).expect_err("decoding u64::MAX + 1 succeeded");
decode_varint_slow(&mut u64_max_plus_one.clone())
.expect_err("slow decoding u64::MAX + 1 succeeded");
}

Expand Down
18 changes: 10 additions & 8 deletions tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ where
if let Err(error) = all_types.encode(&mut buf1) {
return RoundtripResult::Error(error.into());
}
let buf1 = buf1;
if encoded_len != buf1.len() {
return RoundtripResult::Error(anyhow!(
"expected encoded len ({}) did not match actual encoded len ({})",
Expand All @@ -207,7 +208,7 @@ where
));
}

let roundtrip = match M::decode(&*buf1) {
let roundtrip = match M::decode(buf1.as_slice()) {
Ok(roundtrip) => roundtrip,
Err(error) => return RoundtripResult::Error(anyhow::Error::new(error)),
};
Expand All @@ -216,6 +217,7 @@ where
if let Err(error) = roundtrip.encode(&mut buf2) {
return RoundtripResult::Error(error.into());
}
let buf2 = buf2;
let buf3 = roundtrip.encode_to_vec();

/*
Expand Down Expand Up @@ -249,7 +251,7 @@ where
msg.encode(&mut buf).unwrap();
assert_eq!(expected_len, buf.len());

let mut buf = &*buf;
let mut buf = buf.as_slice();
let roundtrip = M::decode(&mut buf).unwrap();

assert!(
Expand Down Expand Up @@ -430,7 +432,7 @@ mod tests {

let mut buf = Vec::new();
a.encode(&mut buf).unwrap();
A::decode(&*buf).map(|_| ())
A::decode(buf.as_slice()).map(|_| ())
}

assert!(build_and_roundtrip(100).is_ok());
Expand All @@ -453,7 +455,7 @@ mod tests {

let mut buf = Vec::new();
a.encode(&mut buf).unwrap();
A::decode(&*buf).map(|_| ())
A::decode(buf.as_slice()).map(|_| ())
}

assert!(build_and_roundtrip(99).is_ok());
Expand All @@ -476,7 +478,7 @@ mod tests {

let mut buf = Vec::new();
a.encode(&mut buf).unwrap();
NestedGroup2::decode(&*buf).map(|_| ())
NestedGroup2::decode(buf.as_slice()).map(|_| ())
}

assert!(build_and_roundtrip(50).is_ok());
Expand All @@ -497,7 +499,7 @@ mod tests {

let mut buf = Vec::new();
c.encode(&mut buf).unwrap();
C::decode(&*buf).map(|_| ())
C::decode(buf.as_slice()).map(|_| ())
}

assert!(build_and_roundtrip(100).is_ok());
Expand All @@ -518,7 +520,7 @@ mod tests {

let mut buf = Vec::new();
d.encode(&mut buf).unwrap();
D::decode(&*buf).map(|_| ())
D::decode(buf.as_slice()).map(|_| ())
}

assert!(build_and_roundtrip(50).is_ok());
Expand Down Expand Up @@ -681,7 +683,7 @@ mod tests {

let mut bytes = Vec::new();
msg2.encode(&mut bytes).unwrap();
assert_eq!(&*bytes, msg2_bytes);
assert_eq!(bytes.as_slice(), msg2_bytes);

assert_eq!(groups::Test2::decode(msg2_bytes), Ok(msg2));
}
Expand Down

0 comments on commit 9353407

Please sign in to comment.