Skip to content

Commit 57b7d02

Browse files
authored
serdect: safer bytes handling (#1112)
See #1111 for the discussion leading to this PR.
1 parent 92155cb commit 57b7d02

File tree

13 files changed

+380
-226
lines changed

13 files changed

+380
-226
lines changed

Cargo.lock

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

serdect/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ bincode = "1"
2626
ciborium = "0.2"
2727
hex-literal = "0.4"
2828
proptest = "1"
29+
rmp-serde = "1"
2930
serde = { version = "1.0.184", default-features = false, features = ["derive"] }
3031
serde_json = "1"
3132
serde-json-core = { version = "0.5", default-features = false, features = ["std"] }

serdect/README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@ other kinds of data-dependent branching on the contents of the serialized data,
3030
using a constant-time hex serialization with human-readable formats should
3131
help reduce the overall timing variability.
3232

33+
`serdect` is tested against the following crates:
34+
- [`bincode`](https://crates.io/crates/bincode) v1
35+
- [`ciborium`](https://crates.io/crates/ciborium) v0.2
36+
- [`rmp-serde`](https://crates.io/crates/rmp-serde) v1
37+
- [`serde-json-core`](https://crates.io/crates/serde-json-core) v0.5
38+
- [`serde-json`](https://crates.io/crates/serde-json) v1
39+
- [`toml`](https://crates.io/crates/toml) v0.7
40+
41+
3342
## Minimum Supported Rust Version
3443

3544
Rust **1.60** or newer.

serdect/src/array.rs

Lines changed: 28 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
11
//! Serialization primitives for arrays.
22
3+
// Unfortunately, we currently cannot tell `serde` in a uniform fashion that we are serializing
4+
// a fixed-size byte array.
5+
// See https://github.com/serde-rs/serde/issues/2120 for the discussion.
6+
// Therefore we have to fall back to the slice methods,
7+
// which will add the size information in the binary formats.
8+
// The only difference is that for the arrays we require the size of the data
9+
// to be exactly equal to the size of the buffer during deserialization,
10+
// while for slices the buffer can be larger than the deserialized data.
11+
312
use core::fmt;
13+
use core::marker::PhantomData;
414

5-
use serde::de::{Error, SeqAccess, Visitor};
6-
use serde::ser::SerializeTuple;
715
use serde::{Deserialize, Deserializer, Serialize, Serializer};
816

17+
use crate::common::{self, LengthCheck, SliceVisitor, StrIntoBufVisitor};
18+
919
#[cfg(feature = "zeroize")]
1020
use zeroize::Zeroize;
1121

@@ -16,17 +26,7 @@ where
1626
S: Serializer,
1727
T: AsRef<[u8]>,
1828
{
19-
if serializer.is_human_readable() {
20-
crate::serialize_hex::<_, _, false>(value, serializer)
21-
} else {
22-
let mut seq = serializer.serialize_tuple(value.as_ref().len())?;
23-
24-
for byte in value.as_ref() {
25-
seq.serialize_element(byte)?;
26-
}
27-
28-
seq.end()
29-
}
29+
common::serialize_hex_lower_or_bin(value, serializer)
3030
}
3131

3232
/// Serialize the given type as upper case hex when using human-readable
@@ -36,16 +36,21 @@ where
3636
S: Serializer,
3737
T: AsRef<[u8]>,
3838
{
39-
if serializer.is_human_readable() {
40-
crate::serialize_hex::<_, _, true>(value, serializer)
41-
} else {
42-
let mut seq = serializer.serialize_tuple(value.as_ref().len())?;
39+
common::serialize_hex_upper_or_bin(value, serializer)
40+
}
4341

44-
for byte in value.as_ref() {
45-
seq.serialize_element(byte)?;
46-
}
42+
struct ExactLength;
4743

48-
seq.end()
44+
impl LengthCheck for ExactLength {
45+
fn length_check(buffer_length: usize, data_length: usize) -> bool {
46+
buffer_length == data_length
47+
}
48+
fn expecting(
49+
formatter: &mut fmt::Formatter<'_>,
50+
data_type: &str,
51+
data_length: usize,
52+
) -> fmt::Result {
53+
write!(formatter, "{} of length {}", data_type, data_length)
4954
}
5055
}
5156

@@ -57,56 +62,9 @@ where
5762
D: Deserializer<'de>,
5863
{
5964
if deserializer.is_human_readable() {
60-
struct StrVisitor<'b>(&'b mut [u8]);
61-
62-
impl<'de> Visitor<'de> for StrVisitor<'_> {
63-
type Value = ();
64-
65-
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
66-
write!(formatter, "a string of length {}", self.0.len() * 2)
67-
}
68-
69-
fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
70-
where
71-
E: Error,
72-
{
73-
if v.len() != self.0.len() * 2 {
74-
return Err(Error::invalid_length(v.len(), &self));
75-
}
76-
77-
base16ct::mixed::decode(v, self.0).map_err(E::custom)?;
78-
79-
Ok(())
80-
}
81-
}
82-
83-
deserializer.deserialize_str(StrVisitor(buffer))
65+
deserializer.deserialize_str(StrIntoBufVisitor::<ExactLength>(buffer, PhantomData))
8466
} else {
85-
struct ArrayVisitor<'b>(&'b mut [u8]);
86-
87-
impl<'de> Visitor<'de> for ArrayVisitor<'_> {
88-
type Value = ();
89-
90-
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
91-
write!(formatter, "an array of length {}", self.0.len())
92-
}
93-
94-
fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
95-
where
96-
A: SeqAccess<'de>,
97-
{
98-
for (index, byte) in self.0.iter_mut().enumerate() {
99-
*byte = match seq.next_element()? {
100-
Some(byte) => byte,
101-
None => return Err(Error::invalid_length(index, &self)),
102-
};
103-
}
104-
105-
Ok(())
106-
}
107-
}
108-
109-
deserializer.deserialize_tuple(buffer.len(), ArrayVisitor(buffer))
67+
deserializer.deserialize_byte_buf(SliceVisitor::<ExactLength>(buffer, PhantomData))
11068
}
11169
}
11270

serdect/src/common.rs

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
use core::fmt;
2+
use core::marker::PhantomData;
3+
4+
use serde::{
5+
de::{Error, Visitor},
6+
Serializer,
7+
};
8+
9+
#[cfg(feature = "alloc")]
10+
use ::{alloc::vec::Vec, serde::Serialize};
11+
12+
#[cfg(not(feature = "alloc"))]
13+
use serde::ser::Error as SerError;
14+
15+
pub(crate) fn serialize_hex<S, T, const UPPERCASE: bool>(
16+
value: &T,
17+
serializer: S,
18+
) -> Result<S::Ok, S::Error>
19+
where
20+
S: Serializer,
21+
T: AsRef<[u8]>,
22+
{
23+
#[cfg(feature = "alloc")]
24+
if UPPERCASE {
25+
return base16ct::upper::encode_string(value.as_ref()).serialize(serializer);
26+
} else {
27+
return base16ct::lower::encode_string(value.as_ref()).serialize(serializer);
28+
}
29+
#[cfg(not(feature = "alloc"))]
30+
{
31+
let _ = value;
32+
let _ = serializer;
33+
return Err(S::Error::custom(
34+
"serializer is human readable, which requires the `alloc` crate feature",
35+
));
36+
}
37+
}
38+
39+
pub(crate) fn serialize_hex_lower_or_bin<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
40+
where
41+
S: Serializer,
42+
T: AsRef<[u8]>,
43+
{
44+
if serializer.is_human_readable() {
45+
serialize_hex::<_, _, false>(value, serializer)
46+
} else {
47+
serializer.serialize_bytes(value.as_ref())
48+
}
49+
}
50+
51+
/// Serialize the given type as upper case hex when using human-readable
52+
/// formats or binary if the format is binary.
53+
pub(crate) fn serialize_hex_upper_or_bin<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
54+
where
55+
S: Serializer,
56+
T: AsRef<[u8]>,
57+
{
58+
if serializer.is_human_readable() {
59+
serialize_hex::<_, _, true>(value, serializer)
60+
} else {
61+
serializer.serialize_bytes(value.as_ref())
62+
}
63+
}
64+
65+
pub(crate) trait LengthCheck {
66+
fn length_check(buffer_length: usize, data_length: usize) -> bool;
67+
fn expecting(
68+
formatter: &mut fmt::Formatter<'_>,
69+
data_type: &str,
70+
data_length: usize,
71+
) -> fmt::Result;
72+
}
73+
74+
pub(crate) struct StrIntoBufVisitor<'b, T: LengthCheck>(pub &'b mut [u8], pub PhantomData<T>);
75+
76+
impl<'de, 'b, T: LengthCheck> Visitor<'de> for StrIntoBufVisitor<'b, T> {
77+
type Value = ();
78+
79+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
80+
T::expecting(formatter, "a string", self.0.len() * 2)
81+
}
82+
83+
fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
84+
where
85+
E: Error,
86+
{
87+
if !T::length_check(self.0.len() * 2, v.len()) {
88+
return Err(Error::invalid_length(v.len(), &self));
89+
}
90+
// TODO: Map `base16ct::Error::InvalidLength` to `Error::invalid_length`.
91+
base16ct::mixed::decode(v, self.0)
92+
.map(|_| ())
93+
.map_err(E::custom)
94+
}
95+
}
96+
97+
#[cfg(feature = "alloc")]
98+
pub(crate) struct StrIntoVecVisitor;
99+
100+
#[cfg(feature = "alloc")]
101+
impl<'de> Visitor<'de> for StrIntoVecVisitor {
102+
type Value = Vec<u8>;
103+
104+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
105+
write!(formatter, "a string")
106+
}
107+
108+
fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
109+
where
110+
E: Error,
111+
{
112+
base16ct::mixed::decode_vec(v).map_err(E::custom)
113+
}
114+
}
115+
116+
pub(crate) struct SliceVisitor<'b, T: LengthCheck>(pub &'b mut [u8], pub PhantomData<T>);
117+
118+
impl<'de, 'b, T: LengthCheck> Visitor<'de> for SliceVisitor<'b, T> {
119+
type Value = ();
120+
121+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
122+
T::expecting(formatter, "an array", self.0.len())
123+
}
124+
125+
fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
126+
where
127+
E: Error,
128+
{
129+
// Workaround for
130+
// https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions
131+
if T::length_check(self.0.len(), v.len()) {
132+
let buffer = &mut self.0[..v.len()];
133+
buffer.copy_from_slice(v);
134+
return Ok(());
135+
}
136+
137+
Err(E::invalid_length(v.len(), &self))
138+
}
139+
140+
#[cfg(feature = "alloc")]
141+
fn visit_byte_buf<E>(self, mut v: Vec<u8>) -> Result<Self::Value, E>
142+
where
143+
E: Error,
144+
{
145+
// Workaround for
146+
// https://github.com/rust-lang/rfcs/blob/b1de05846d9bc5591d753f611ab8ee84a01fa500/text/2094-nll.md#problem-case-3-conditional-control-flow-across-functions
147+
if T::length_check(self.0.len(), v.len()) {
148+
let buffer = &mut self.0[..v.len()];
149+
buffer.swap_with_slice(&mut v);
150+
return Ok(());
151+
}
152+
153+
Err(E::invalid_length(v.len(), &self))
154+
}
155+
}
156+
157+
#[cfg(feature = "alloc")]
158+
pub(crate) struct VecVisitor;
159+
160+
#[cfg(feature = "alloc")]
161+
impl<'de> Visitor<'de> for VecVisitor {
162+
type Value = Vec<u8>;
163+
164+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
165+
write!(formatter, "a bytestring")
166+
}
167+
168+
fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
169+
where
170+
E: Error,
171+
{
172+
Ok(v.into())
173+
}
174+
175+
fn visit_byte_buf<E>(self, v: Vec<u8>) -> Result<Self::Value, E>
176+
where
177+
E: Error,
178+
{
179+
Ok(v)
180+
}
181+
}

0 commit comments

Comments
 (0)