From 984ac57b70f746c02b9e15a01e345ff649dbe526 Mon Sep 17 00:00:00 2001 From: ryan kurte Date: Wed, 14 Dec 2022 15:21:43 +1300 Subject: [PATCH 1/4] add support for const length arrays (`[u8; N]`) --- src/encoding.rs | 62 +++++++++++++++++++++++++++++++++++++-- tests/src/derive_const.rs | 24 +++++++++++++++ tests/src/lib.rs | 2 ++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 tests/src/derive_const.rs diff --git a/src/encoding.rs b/src/encoding.rs index e4d2aa274..cd1181730 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -769,7 +769,7 @@ macro_rules! length_delimited { B: Buf, { check_wire_type(WireType::LengthDelimited, wire_type)?; - let mut value = Default::default(); + let mut value = crate::encoding::sealed::Newable::new(); merge(wire_type, &mut value, buf, ctx)?; values.push(value); Ok(()) @@ -873,12 +873,13 @@ pub mod string { } } -pub trait BytesAdapter: sealed::BytesAdapter {} +pub trait BytesAdapter: sealed::BytesAdapter + sealed::Newable {} + mod sealed { use super::{Buf, BufMut}; - pub trait BytesAdapter: Default + Sized + 'static { + pub trait BytesAdapter: Sized + 'static { fn len(&self) -> usize; /// Replace contents of this buffer with the contents of another buffer. @@ -895,10 +896,23 @@ mod sealed { self.len() == 0 } } + + /// Alternate to `Default` as this is not (yet?) implemented over [T; N] + /// (and cannot be automatic as `Default` _is_ implemented for N < 32) + pub trait Newable: Sized { + fn new() -> Self; + } + } impl BytesAdapter for Bytes {} +impl sealed::Newable for Bytes { + fn new() -> Self { + Default::default() + } +} + impl sealed::BytesAdapter for Bytes { fn len(&self) -> usize { Buf::remaining(self) @@ -921,7 +935,14 @@ impl sealed::BytesAdapter for Bytes { impl BytesAdapter for Vec {} +impl sealed::Newable for Vec { + fn new() -> Self { + Default::default() + } +} + impl sealed::BytesAdapter for Vec { + fn len(&self) -> usize { Vec::len(self) } @@ -943,6 +964,41 @@ impl sealed::BytesAdapter for Vec { } } +impl BytesAdapter for [u8; N] {} + +impl sealed::Newable for [u8; N] { + fn new() -> Self { + [0u8; N] + } +} + +impl sealed::BytesAdapter for [u8; N] { + + fn len(&self) -> usize { + N + } + + fn replace_with(&mut self, buf: B) + where + B: Buf, + { + self.copy_from_slice(buf.chunk()) + } + + fn append_to(&self, buf: &mut B) + where + B: BufMut, + { + buf.put(&self[..]) + } +} + +impl sealed::Newable for String { + fn new() -> Self { + Default::default() + } +} + pub mod bytes { use super::*; diff --git a/tests/src/derive_const.rs b/tests/src/derive_const.rs new file mode 100644 index 000000000..fa21b9c07 --- /dev/null +++ b/tests/src/derive_const.rs @@ -0,0 +1,24 @@ +use core::fmt::Debug; +use prost::Message; + +/// Const array container +#[derive(Clone, PartialEq, Message)] +pub struct TestArray { + #[prost(message, required, tag = "1")] + pub b: [u8; 3], +} + +#[test] +fn encode_decode_const_array() { + let t = TestArray { + b: [1, 2, 3], + }; + + let buff = t.encode_to_vec(); + + let b = TestArray::decode(&*buff).unwrap(); + + assert_eq!(a, b); + + panic!("whjoops"); +} diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 65d393c36..8d4a5b3c0 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -44,6 +44,8 @@ mod message_encoding; mod no_unused_results; #[cfg(test)] mod well_known_types; +#[cfg(test)] +mod derive_const; pub mod foo { pub mod bar_baz { From a64606a9bc6369061e4706f085b76ee81d19873b Mon Sep 17 00:00:00 2001 From: ryan kurte Date: Wed, 14 Dec 2022 15:28:26 +1300 Subject: [PATCH 2/4] apply cargo fmt --- src/encoding.rs | 12 ++++-------- tests/src/derive_const.rs | 4 +--- tests/src/lib.rs | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/encoding.rs b/src/encoding.rs index cd1181730..7219340ff 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -875,7 +875,6 @@ pub mod string { pub trait BytesAdapter: sealed::BytesAdapter + sealed::Newable {} - mod sealed { use super::{Buf, BufMut}; @@ -902,7 +901,6 @@ mod sealed { pub trait Newable: Sized { fn new() -> Self; } - } impl BytesAdapter for Bytes {} @@ -935,14 +933,13 @@ impl sealed::BytesAdapter for Bytes { impl BytesAdapter for Vec {} -impl sealed::Newable for Vec { +impl sealed::Newable for Vec { fn new() -> Self { Default::default() } } impl sealed::BytesAdapter for Vec { - fn len(&self) -> usize { Vec::len(self) } @@ -964,16 +961,15 @@ impl sealed::BytesAdapter for Vec { } } -impl BytesAdapter for [u8; N] {} +impl BytesAdapter for [u8; N] {} -impl sealed::Newable for [u8; N] { +impl sealed::Newable for [u8; N] { fn new() -> Self { [0u8; N] } } -impl sealed::BytesAdapter for [u8; N] { - +impl sealed::BytesAdapter for [u8; N] { fn len(&self) -> usize { N } diff --git a/tests/src/derive_const.rs b/tests/src/derive_const.rs index fa21b9c07..5d9ac6b0f 100644 --- a/tests/src/derive_const.rs +++ b/tests/src/derive_const.rs @@ -10,9 +10,7 @@ pub struct TestArray { #[test] fn encode_decode_const_array() { - let t = TestArray { - b: [1, 2, 3], - }; + let t = TestArray { b: [1, 2, 3] }; let buff = t.encode_to_vec(); diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 8d4a5b3c0..e604d252a 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -37,6 +37,8 @@ mod debug; #[cfg(test)] mod deprecated_field; #[cfg(test)] +mod derive_const; +#[cfg(test)] mod generic_derive; #[cfg(test)] mod message_encoding; @@ -44,8 +46,6 @@ mod message_encoding; mod no_unused_results; #[cfg(test)] mod well_known_types; -#[cfg(test)] -mod derive_const; pub mod foo { pub mod bar_baz { From f6b8694f381d3c81f755ef7212bca8cb04f1a4a8 Mon Sep 17 00:00:00 2001 From: ryan kurte Date: Mon, 2 Jan 2023 11:57:47 +1300 Subject: [PATCH 3/4] cleanup const support make BytesAdapter externally implementable, propagate errors from BytesAdapter decode fn --- src/encoding.rs | 109 ++++++++++++++++++++++---------------- tests/src/derive_const.rs | 39 ++++++++++---- 2 files changed, 93 insertions(+), 55 deletions(-) diff --git a/src/encoding.rs b/src/encoding.rs index 7219340ff..fac2baa18 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -873,54 +873,63 @@ pub mod string { } } -pub trait BytesAdapter: sealed::BytesAdapter + sealed::Newable {} +pub trait BytesAdapter: Sized + 'static { + /// Create a new instance (required as `Default` is not available for all types) + fn new() -> Self; -mod sealed { - use super::{Buf, BufMut}; - - pub trait BytesAdapter: Sized + 'static { - fn len(&self) -> usize; + fn len(&self) -> usize; - /// Replace contents of this buffer with the contents of another buffer. - fn replace_with(&mut self, buf: B) - where - B: Buf; + /// Replace contents of this buffer with the contents of another buffer. + fn replace_with(&mut self, buf: B) -> Result<(), DecodeError> + where + B: Buf; - /// Appends this buffer to the (contents of) other buffer. - fn append_to(&self, buf: &mut B) - where - B: BufMut; + /// Appends this buffer to the (contents of) other buffer. + fn append_to(&self, buf: &mut B) + where + B: BufMut; - fn is_empty(&self) -> bool { - self.len() == 0 - } + fn is_empty(&self) -> bool { + self.len() == 0 } - /// Alternate to `Default` as this is not (yet?) implemented over [T; N] - /// (and cannot be automatic as `Default` _is_ implemented for N < 32) + fn clear(&mut self); +} + +mod sealed { pub trait Newable: Sized { fn new() -> Self; } -} -impl BytesAdapter for Bytes {} + impl Newable for T { + fn new() -> Self { + super::BytesAdapter::new() + } + } + + impl Newable for String { + fn new() -> Self { + Default::default() + } + } +} -impl sealed::Newable for Bytes { +impl BytesAdapter for Bytes { fn new() -> Self { Default::default() } -} -impl sealed::BytesAdapter for Bytes { fn len(&self) -> usize { Buf::remaining(self) } - fn replace_with(&mut self, mut buf: B) + fn replace_with(&mut self, mut buf: B) -> Result<(), DecodeError> where B: Buf, { *self = buf.copy_to_bytes(buf.remaining()); + + Ok(()) } fn append_to(&self, buf: &mut B) @@ -929,28 +938,30 @@ impl sealed::BytesAdapter for Bytes { { buf.put(self.clone()) } -} -impl BytesAdapter for Vec {} + fn clear(&mut self) { + Bytes::clear(self) + } +} -impl sealed::Newable for Vec { +impl BytesAdapter for Vec { fn new() -> Self { Default::default() } -} -impl sealed::BytesAdapter for Vec { fn len(&self) -> usize { Vec::len(self) } - fn replace_with(&mut self, buf: B) + fn replace_with(&mut self, buf: B) -> Result<(), DecodeError> where B: Buf, { - self.clear(); + Vec::clear(self); self.reserve(buf.remaining()); self.put(buf); + + Ok(()) } fn append_to(&self, buf: &mut B) @@ -959,26 +970,32 @@ impl sealed::BytesAdapter for Vec { { buf.put(self.as_slice()) } -} -impl BytesAdapter for [u8; N] {} + fn clear(&mut self) { + Vec::clear(self) + } +} -impl sealed::Newable for [u8; N] { +impl BytesAdapter for [u8; N] { fn new() -> Self { [0u8; N] } -} -impl sealed::BytesAdapter for [u8; N] { fn len(&self) -> usize { N } - fn replace_with(&mut self, buf: B) + fn replace_with(&mut self, buf: B) -> Result<(), DecodeError> where B: Buf, { - self.copy_from_slice(buf.chunk()) + if buf.remaining() != N { + return Err(DecodeError::new("invalid byte array length")); + } + + self.copy_from_slice(buf.chunk()); + + Ok(()) } fn append_to(&self, buf: &mut B) @@ -987,11 +1004,11 @@ impl sealed::BytesAdapter for [u8; N] { { buf.put(&self[..]) } -} -impl sealed::Newable for String { - fn new() -> Self { - Default::default() + fn clear(&mut self) { + for b in &mut self[..] { + *b = 0; + } } } @@ -1037,7 +1054,8 @@ pub mod bytes { // This is intended for A and B both being Bytes so it is zero-copy. // Some combinations of A and B types may cause a double-copy, // in which case merge_one_copy() should be used instead. - value.replace_with(buf.copy_to_bytes(len)); + value.replace_with(buf.copy_to_bytes(len))?; + Ok(()) } @@ -1059,7 +1077,8 @@ pub mod bytes { let len = len as usize; // If we must copy, make sure to copy only once. - value.replace_with(buf.take(len)); + value.replace_with(buf.take(len))?; + Ok(()) } diff --git a/tests/src/derive_const.rs b/tests/src/derive_const.rs index 5d9ac6b0f..09af28b66 100644 --- a/tests/src/derive_const.rs +++ b/tests/src/derive_const.rs @@ -1,22 +1,41 @@ -use core::fmt::Debug; -use prost::Message; +use prost::{encoding::BytesAdapter, Message}; -/// Const array container +// NOTE: [Message] still requires [Default], which is not implemented for [u8; N], +// so this will only work directly for [u8; <=32] for the moment... +// see: https://github.com/rust-lang/rust/issues/61415 + +/// Const array container A #[derive(Clone, PartialEq, Message)] -pub struct TestArray { - #[prost(message, required, tag = "1")] +pub struct TestA { + #[prost(bytes, required, tag = "1")] pub b: [u8; 3], } +/// Const array container B +#[derive(Clone, PartialEq, Message)] +pub struct TestB { + #[prost(bytes, required, tag = "1")] + pub b: [u8; 4], +} + +// Test valid encode/decode #[test] -fn encode_decode_const_array() { - let t = TestArray { b: [1, 2, 3] }; +fn const_array_encode_decode() { + let t = TestA { b: [1, 2, 3] }; let buff = t.encode_to_vec(); - let b = TestArray::decode(&*buff).unwrap(); + let t1 = TestA::decode(&*buff).unwrap(); - assert_eq!(a, b); + assert_eq!(t, t1); +} + +// test encode/decode length mismatch +#[test] +fn const_array_length_mismatch() { + let t = TestA { b: [1, 2, 3] }; + + let buff = t.encode_to_vec(); - panic!("whjoops"); + assert!(TestB::decode(&*buff).is_err()); } From 753aa7af8ce405614b6f7fb0a94a3da6c308227c Mon Sep 17 00:00:00 2001 From: ryan kurte Date: Tue, 3 Jan 2023 11:24:44 +1300 Subject: [PATCH 4/4] fix no default features test --- src/encoding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/encoding.rs b/src/encoding.rs index fac2baa18..7ef356a5e 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -907,7 +907,7 @@ mod sealed { } } - impl Newable for String { + impl Newable for alloc::string::String { fn new() -> Self { Default::default() }