Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix serialization for Duration* and Timestamp* types for very large values #772

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ explicit_auto_deref = "allow"
manual-unwrap-or-default = "allow"

# alloc_instead_of_core = "warn"
# as_conversions = "warn"
# arithmetic_side_effects = "warn"
# Checks for usage of `cloned()` on an `Iterator` or `Option` where `copied()` could be used instead.
cloned_instead_of_copied = "warn"
# Checks for literal calls to `Default::default()`.
Expand Down
11 changes: 6 additions & 5 deletions serde_with/src/chrono_0_4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ pub mod datetime_utc_ts_seconds_from_any {
}
[seconds, subseconds] => {
if let Ok(seconds) = seconds.parse() {
let subseclen = subseconds.chars().count() as u32;
if subseclen > 9 {
return Err(DeError::custom(format_args!(
let subseclen =
match u32::try_from(subseconds.chars().count()) {
Ok(subseclen) if subseclen <= 9 => subseclen,
_ => return Err(DeError::custom(format_args!(
"DateTimes only support nanosecond precision but '{value}' has more than 9 digits."
)));
}
))),
};

if let Ok(mut subseconds) = subseconds.parse() {
// convert subseconds to nanoseconds (10^-9), require 9 places for nanoseconds
Expand Down
71 changes: 53 additions & 18 deletions serde_with/src/content/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! In the future this can hopefully be replaced by a public type in `serde` itself.
//! <https://github.com/serde-rs/serde/pull/2348>

use self::utils::{get_unexpected_i128, get_unexpected_u128};
use crate::{
prelude::*,
utils::{size_hint_cautious, size_hint_from_bounds},
Expand Down Expand Up @@ -59,20 +60,20 @@ pub(crate) enum Content<'de> {

impl<'de> Content<'de> {
#[cold]
fn unexpected(&self) -> Unexpected<'_> {
fn unexpected<'a>(&'a self, buf: &'a mut [u8; 58]) -> Unexpected<'a> {
match *self {
Content::Bool(b) => Unexpected::Bool(b),
Content::U8(n) => Unexpected::Unsigned(n as u64),
Content::U16(n) => Unexpected::Unsigned(n as u64),
Content::U32(n) => Unexpected::Unsigned(n as u64),
Content::U8(n) => Unexpected::Unsigned(u64::from(n)),
Content::U16(n) => Unexpected::Unsigned(u64::from(n)),
Content::U32(n) => Unexpected::Unsigned(u64::from(n)),
Content::U64(n) => Unexpected::Unsigned(n),
Content::U128(_) => Unexpected::Other("u128"),
Content::I8(n) => Unexpected::Signed(n as i64),
Content::I16(n) => Unexpected::Signed(n as i64),
Content::I32(n) => Unexpected::Signed(n as i64),
Content::U128(n) => get_unexpected_u128(n, buf),
Content::I8(n) => Unexpected::Signed(i64::from(n)),
Content::I16(n) => Unexpected::Signed(i64::from(n)),
Content::I32(n) => Unexpected::Signed(i64::from(n)),
Content::I64(n) => Unexpected::Signed(n),
Content::I128(_) => Unexpected::Other("i128"),
Content::F32(f) => Unexpected::Float(f as f64),
Content::I128(n) => get_unexpected_i128(n, buf),
Content::F32(f) => Unexpected::Float(f64::from(f)),
Content::F64(f) => Unexpected::Float(f),
Content::Char(c) => Unexpected::Char(c),
Content::String(ref s) => Unexpected::Str(s),
Expand Down Expand Up @@ -325,7 +326,8 @@ where
{
#[cold]
fn invalid_type(self, exp: &dyn Expected) -> E {
DeError::invalid_type(self.content.unexpected(), exp)
let mut buf = [0; 58];
DeError::invalid_type(self.content.unexpected(&mut buf), exp)
}

fn deserialize_integer<V>(self, visitor: V) -> Result<V::Value, E>
Expand Down Expand Up @@ -767,7 +769,11 @@ where
}
s @ Content::String(_) | s @ Content::Str(_) => (s, None),
other => {
return Err(DeError::invalid_type(other.unexpected(), &"string or map"));
let mut buf = [0; 58];
return Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"string or map",
));
}
};

Expand Down Expand Up @@ -913,7 +919,13 @@ where
SeqDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"tuple variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"tuple variant",
Expand All @@ -938,7 +950,13 @@ where
SeqDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"struct variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"struct variant",
Expand Down Expand Up @@ -1129,7 +1147,8 @@ where
{
#[cold]
fn invalid_type(self, exp: &dyn Expected) -> E {
DeError::invalid_type(self.content.unexpected(), exp)
let mut buf = [0; 58];
DeError::invalid_type(self.content.unexpected(&mut buf), exp)
}

fn deserialize_integer<V>(self, visitor: V) -> Result<V::Value, E>
Expand Down Expand Up @@ -1542,7 +1561,11 @@ where
}
ref s @ Content::String(_) | ref s @ Content::Str(_) => (s, None),
ref other => {
return Err(DeError::invalid_type(other.unexpected(), &"string or map"));
let mut buf = [0; 58];
return Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"string or map",
));
}
};

Expand Down Expand Up @@ -1670,7 +1693,13 @@ where
SeqRefDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"tuple variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"tuple variant",
Expand All @@ -1695,7 +1724,13 @@ where
SeqRefDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"struct variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"struct variant",
Expand Down
26 changes: 16 additions & 10 deletions serde_with/src/de/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,7 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Unsigned(unexp as u64),
Unexpected::Unsigned(u64::from(unexp)),
&"0 or 1",
)),
}
Expand All @@ -1840,7 +1840,7 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Signed(unexp as i64),
Unexpected::Signed(i64::from(unexp)),
&"0 or 1",
)),
}
Expand Down Expand Up @@ -1878,10 +1878,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
match v {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Unsigned(unexp as u64),
&"0 or 1",
)),
unexp => {
let mut buf: [u8; 58] = [0u8; 58];
Err(DeError::invalid_value(
crate::utils::get_unexpected_u128(unexp, &mut buf),
&self,
))
}
}
}

Expand All @@ -1892,10 +1895,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
match v {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Signed(unexp as i64),
&"0 or 1",
)),
unexp => {
let mut buf: [u8; 58] = [0u8; 58];
Err(DeError::invalid_value(
crate::utils::get_unexpected_i128(unexp, &mut buf),
&"0 or 1",
))
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion serde_with/src/ser/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ impl<STRICTNESS: Strictness> SerializeAs<bool> for BoolFromInt<STRICTNESS> {
where
S: Serializer,
{
serializer.serialize_u8(*source as u8)
serializer.serialize_u8(u8::from(*source))
}
}

Expand Down
56 changes: 51 additions & 5 deletions serde_with/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ where
_size_hint_from_bounds(iter.size_hint())
}

pub(crate) const NANOS_PER_SEC: u32 = 1_000_000_000;
pub(crate) const NANOS_PER_SEC: u128 = 1_000_000_000;
pub(crate) const NANOS_PER_SEC_F64: f64 = 1_000_000_000.0;
// pub(crate) const NANOS_PER_MILLI: u32 = 1_000_000;
// pub(crate) const NANOS_PER_MICRO: u32 = 1_000;
// pub(crate) const MILLIS_PER_SEC: u64 = 1_000;
// pub(crate) const MICROS_PER_SEC: u64 = 1_000_000;
pub(crate) const U64_MAX: u128 = u64::MAX as u128;

pub(crate) struct MapIter<'de, A, K, V> {
pub(crate) access: A,
Expand Down Expand Up @@ -114,10 +116,10 @@ where
}

pub(crate) fn duration_signed_from_secs_f64(secs: f64) -> Result<DurationSigned, &'static str> {
const MAX_NANOS_F64: f64 = ((u64::MAX as u128 + 1) * (NANOS_PER_SEC as u128)) as f64;
const MAX_NANOS_F64: f64 = ((U64_MAX + 1) * NANOS_PER_SEC) as f64;
// TODO why are the seconds converted to nanoseconds first?
// Does it make sense to just truncate the value?
let mut nanos = secs * (NANOS_PER_SEC as f64);
let mut nanos = secs * NANOS_PER_SEC_F64;
if !nanos.is_finite() {
return Err("got non-finite value when converting float to duration");
}
Expand All @@ -132,8 +134,8 @@ pub(crate) fn duration_signed_from_secs_f64(secs: f64) -> Result<DurationSigned,
let nanos = nanos as u128;
Ok(DurationSigned::new(
sign,
(nanos / (NANOS_PER_SEC as u128)) as u64,
(nanos % (NANOS_PER_SEC as u128)) as u32,
(nanos / NANOS_PER_SEC) as u64,
(nanos % NANOS_PER_SEC) as u32,
))
}

Expand Down Expand Up @@ -194,3 +196,47 @@ where
// https://github.com/rust-lang/rust/issues/61956
Ok(unsafe { core::mem::transmute_copy::<_, [T; N]>(&arr) })
}

/// Writer that writes into a `&mut [u8]` while checking the length of the buffer
struct BufWriter<'a> {
bytes: &'a mut [u8],
offset: usize,
}

impl<'a> BufWriter<'a> {
fn new(bytes: &'a mut [u8]) -> Self {
BufWriter { bytes, offset: 0 }
}

fn into_str(self) -> &'a str {
let slice = &self.bytes[..self.offset];
core::str::from_utf8(slice)
.unwrap_or("Failed to extract valid string from BufWriter. This should never happen.")
}
}

impl<'a> core::fmt::Write for BufWriter<'a> {
fn write_str(&mut self, s: &str) -> fmt::Result {
if s.len() > self.bytes.len() - self.offset {
Err(fmt::Error)
} else {
self.bytes[self.offset..self.offset + s.len()].copy_from_slice(s.as_bytes());
self.offset += s.len();
Ok(())
}
}
}

// 58 chars is long enough for any i128 and u128 value
pub(crate) fn get_unexpected_i128(value: i128, buf: &mut [u8; 58]) -> Unexpected<'_> {
let mut writer = BufWriter::new(buf);
fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as i128")).unwrap();
Unexpected::Other(writer.into_str())
}

// 58 chars is long enough for any i128 and u128 value
pub(crate) fn get_unexpected_u128(value: u128, buf: &mut [u8; 58]) -> Unexpected<'_> {
let mut writer = BufWriter::new(buf);
fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as u128")).unwrap();
Unexpected::Other(writer.into_str())
}
Loading
Loading