Repository: near/borsh-rs
Pull request #66 (2022-01-14 15:10:17)

Implemented (de)serialization for nonzero integer

Boiethios: Do you know when it's reviewed? I need this change, and there're very few line to review.
matklad: Yeah, let's merge this, the impl and tests look good to me!
matklad: :facepalm: travis refuses to go away, opened https://near.zulipchat.com/#narrow/stream/308449-nearinc.2Fcore-tools/topic/borsh.20repo.20admin
Boiethios: > facepalm travis refuses to go away, opened https://near.zulipchat.com/#narrow/stream/308449-nearinc.2Fcore-tools/topic/borsh.20repo.20admin

I hope it'll be solved soon 😛 
I think that the error message could be better, because it's a bit cryptic for now. What do you think about `"Zero value read for a NonZero integer"` or something like that?
matklad: >or something like that?

No strong feelings one way or another. My heuristic would be to check what `serde_json` does and copy that without much extra delibiration. 
Boiethios: > My heuristic would be to check what `serde_json` does and copy that without much extra delibiration.

Did that, thanks! I think it's important that people understand easily what happens when reading an error message.

Patch:
diff --git a/borsh/src/de/mod.rs b/borsh/src/de/mod.rs
index 5e94348b..ff4b7afe 100644
--- a/borsh/src/de/mod.rs
+++ b/borsh/src/de/mod.rs
@@ -23,6 +23,7 @@ mod hint;
 const ERROR_NOT_ALL_BYTES_READ: &str = "Not all bytes read";
 const ERROR_UNEXPECTED_LENGTH_OF_INPUT: &str = "Unexpected length of input";
 const ERROR_OVERFLOW_ON_MACHINE_WITH_32_BIT_USIZE: &str = "Overflow on machine with 32 bit usize";
+const ERROR_INVALID_ZERO_VALUE: &str = "Expected a non-zero value";
 
 /// A data-structure that can be de-serialized from binary format by NBOR.
 pub trait BorshDeserialize: Sized {
@@ -131,6 +132,30 @@ impl_for_integer!(u32);
 impl_for_integer!(u64);
 impl_for_integer!(u128);
 
+macro_rules! impl_for_nonzero_integer {
+    ($type: ty) => {
+        impl BorshDeserialize for $type {
+            #[inline]
+            fn deserialize(buf: &mut &[u8]) -> Result<Self> {
+                <$type>::new(BorshDeserialize::deserialize(buf)?)
+                    .ok_or_else(|| Error::new(ErrorKind::InvalidData, ERROR_INVALID_ZERO_VALUE))
+            }
+        }
+    };
+}
+
+impl_for_nonzero_integer!(core::num::NonZeroI8);
+impl_for_nonzero_integer!(core::num::NonZeroI16);
+impl_for_nonzero_integer!(core::num::NonZeroI32);
+impl_for_nonzero_integer!(core::num::NonZeroI64);
+impl_for_nonzero_integer!(core::num::NonZeroI128);
+impl_for_nonzero_integer!(core::num::NonZeroU8);
+impl_for_nonzero_integer!(core::num::NonZeroU16);
+impl_for_nonzero_integer!(core::num::NonZeroU32);
+impl_for_nonzero_integer!(core::num::NonZeroU64);
+impl_for_nonzero_integer!(core::num::NonZeroU128);
+impl_for_nonzero_integer!(core::num::NonZeroUsize);
+
 impl BorshDeserialize for usize {
     fn deserialize(buf: &mut &[u8]) -> Result<Self> {
         let u: u64 = BorshDeserialize::deserialize(buf)?;
diff --git a/borsh/src/ser/mod.rs b/borsh/src/ser/mod.rs
index fb3ff560..56b248c3 100644
--- a/borsh/src/ser/mod.rs
+++ b/borsh/src/ser/mod.rs
@@ -95,6 +95,29 @@ impl_for_integer!(u32);
 impl_for_integer!(u64);
 impl_for_integer!(u128);
 
+macro_rules! impl_for_nonzero_integer {
+    ($type: ty) => {
+        impl BorshSerialize for $type {
+            #[inline]
+            fn serialize<W: Write>(&self, writer: &mut W) -> Result<()> {
+                BorshSerialize::serialize(&self.get(), writer)
+            }
+        }
+    };
+}
+
+impl_for_nonzero_integer!(core::num::NonZeroI8);
+impl_for_nonzero_integer!(core::num::NonZeroI16);
+impl_for_nonzero_integer!(core::num::NonZeroI32);
+impl_for_nonzero_integer!(core::num::NonZeroI64);
+impl_for_nonzero_integer!(core::num::NonZeroI128);
+impl_for_nonzero_integer!(core::num::NonZeroU8);
+impl_for_nonzero_integer!(core::num::NonZeroU16);
+impl_for_nonzero_integer!(core::num::NonZeroU32);
+impl_for_nonzero_integer!(core::num::NonZeroU64);
+impl_for_nonzero_integer!(core::num::NonZeroU128);
+impl_for_nonzero_integer!(core::num::NonZeroUsize);
+
 impl BorshSerialize for usize {
     fn serialize<W: Write>(&self, writer: &mut W) -> Result<()> {
         BorshSerialize::serialize(&(*self as u64), writer)


Test patch:
diff --git a/borsh/tests/test_de_errors.rs b/borsh/tests/test_de_errors.rs
index 82fa341c..4a9963a4 100644
--- a/borsh/tests/test_de_errors.rs
+++ b/borsh/tests/test_de_errors.rs
@@ -15,6 +15,7 @@ struct B {
 }
 
 const ERROR_UNEXPECTED_LENGTH_OF_INPUT: &str = "Unexpected length of input";
+const ERROR_INVALID_ZERO_VALUE: &str = "Expected a non-zero value";
 
 #[test]
 fn test_missing_bytes() {
@@ -140,7 +141,62 @@ fn test_evil_bytes_string_extra() {
     // Might fail if reading too much
     let bytes = vec![255, 255, 255, 255, 32, 32];
     assert_eq!(
-        <String>::try_from_slice(&bytes).unwrap_err().to_string(),
+        String::try_from_slice(&bytes).unwrap_err().to_string(),
+        ERROR_UNEXPECTED_LENGTH_OF_INPUT
+    );
+}
+
+#[test]
+fn test_zero_on_nonzero_integer_u8() {
+    let bytes = &[0];
+    assert_eq!(
+        std::num::NonZeroU8::try_from_slice(bytes)
+            .unwrap_err()
+            .to_string(),
+        ERROR_INVALID_ZERO_VALUE
+    );
+}
+
+#[test]
+fn test_zero_on_nonzero_integer_u32() {
+    let bytes = &[0; 4];
+    assert_eq!(
+        std::num::NonZeroU32::try_from_slice(bytes)
+            .unwrap_err()
+            .to_string(),
+        ERROR_INVALID_ZERO_VALUE
+    );
+}
+
+#[test]
+fn test_zero_on_nonzero_integer_i64() {
+    let bytes = &[0; 8];
+    assert_eq!(
+        std::num::NonZeroI64::try_from_slice(bytes)
+            .unwrap_err()
+            .to_string(),
+        ERROR_INVALID_ZERO_VALUE
+    );
+}
+
+#[test]
+fn test_zero_on_nonzero_integer_usize() {
+    let bytes = &[0; 8];
+    assert_eq!(
+        std::num::NonZeroUsize::try_from_slice(bytes)
+            .unwrap_err()
+            .to_string(),
+        ERROR_INVALID_ZERO_VALUE
+    );
+}
+
+#[test]
+fn test_zero_on_nonzero_integer_missing_byte() {
+    let bytes = &[0; 7];
+    assert_eq!(
+        std::num::NonZeroUsize::try_from_slice(bytes)
+            .unwrap_err()
+            .to_string(),
         ERROR_UNEXPECTED_LENGTH_OF_INPUT
     );
 }
diff --git a/borsh/tests/test_nonzero_integers.rs b/borsh/tests/test_nonzero_integers.rs
new file mode 100644
index 00000000..a0b0a469
--- /dev/null
+++ b/borsh/tests/test_nonzero_integers.rs
@@ -0,0 +1,32 @@
+use borsh::BorshDeserialize;
+use std::num::*;
+
+#[test]
+fn test_nonzero_integer_u8() {
+    let bytes = &[1];
+    assert_eq!(NonZeroU8::try_from_slice(bytes).unwrap().get(), 1);
+}
+
+#[test]
+fn test_nonzero_integer_u32() {
+    let bytes = &[255, 0, 0, 0];
+    assert_eq!(NonZeroU32::try_from_slice(bytes).unwrap().get(), 255);
+}
+
+#[test]
+fn test_nonzero_integer_usize() {
+    let bytes = &[1, 1, 0, 0, 0, 0, 0, 0];
+    assert_eq!(NonZeroUsize::try_from_slice(bytes).unwrap().get(), 257);
+}
+
+#[test]
+fn test_nonzero_integer_i64() {
+    let bytes = &[255; 8];
+    assert_eq!(NonZeroI64::try_from_slice(bytes).unwrap().get(), -1);
+}
+
+#[test]
+fn test_nonzero_integer_i16b() {
+    let bytes = &[0, 0b1000_0000];
+    assert_eq!(NonZeroI16::try_from_slice(bytes).unwrap().get(), i16::MIN);
+}