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 6 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
1 change: 1 addition & 0 deletions serde_with/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ hashbrown_0_15 = {package = "hashbrown", version = "0.15.0", optional = true, de
hex = {version = "0.4.3", optional = true, default-features = false}
indexmap_1 = {package = "indexmap", version = "1.8", optional = true, default-features = false, features = ["serde-1"]}
indexmap_2 = {package = "indexmap", version = "2.0", optional = true, default-features = false, features = ["serde"]}
num-traits = { version = "0.2.19", features = ["std"] }
schemars_0_8 = {package = "schemars", version = "0.8.16", optional = true, default-features = false}
serde = {version = "1.0.152", default-features = false}
serde_derive = "1.0.152"
Expand Down
30 changes: 19 additions & 11 deletions serde_with/src/chrono_0_4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ fn unix_epoch_naive() -> NaiveDateTime {
#[cfg(feature = "std")]
pub mod datetime_utc_ts_seconds_from_any {
use super::*;
use num_traits::ToPrimitive as _;

/// Deserialize a Unix timestamp with optional subsecond precision into a `DateTime<Utc>`.
pub fn deserialize<'de, D>(deserializer: D) -> Result<DateTime<Utc>, D::Error>
Expand Down Expand Up @@ -87,20 +88,26 @@ pub mod datetime_utc_ts_seconds_from_any {
where
E: DeError,
{
DateTime::from_timestamp(value as i64, 0).ok_or_else(|| {
DeError::custom(format_args!(
i64::try_from(value)
.ok()
.and_then(|value| DateTime::from_timestamp(value, 0))
.ok_or_else(|| {
DeError::custom(format_args!(
"a timestamp which can be represented in a DateTime but received '{value}'"
))
})
})
}

fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: DeError,
{
let seconds = value.trunc() as i64;
let nsecs = (value.fract() * 1_000_000_000_f64).abs() as u32;
DateTime::from_timestamp(seconds, nsecs).ok_or_else(|| {
fn f64_to_value(value: f64) -> Option<DateTime<Utc>> {
let seconds = value.trunc().to_i64()?;
let nsecs = (value.fract() * 1_000_000_000_f64).abs().to_u32()?;
DateTime::from_timestamp(seconds, nsecs)
}
f64_to_value(value).ok_or_else(|| {
DeError::custom(format_args!(
"a timestamp which can be represented in a DateTime but received '{value}'"
))
Expand All @@ -127,12 +134,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 Content<'_> {
#[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 @@ -1839,7 +1839,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 @@ -1853,7 +1853,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 @@ -1891,10 +1891,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 @@ -1905,10 +1908,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 @@ -1024,7 +1024,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 @@
_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;

Check failure on line 43 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 43 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

using a potentially dangerous silent `as` conversion

Check failure on line 43 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 43 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

using a potentially dangerous silent `as` conversion

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

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;

Check failure on line 119 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 119 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

using a potentially dangerous silent `as` conversion

Check failure on line 119 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 119 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

using a potentially dangerous silent `as` conversion
// 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 @@ -129,11 +131,11 @@
nanos = -nanos;
sign = Sign::Negative;
}
let nanos = nanos as u128;

Check failure on line 134 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 134 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

using a potentially dangerous silent `as` conversion

Check failure on line 134 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 134 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

using a potentially dangerous silent `as` conversion
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,

Check failure on line 137 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 137 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

using a potentially dangerous silent `as` conversion

Check failure on line 137 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 137 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

using a potentially dangerous silent `as` conversion
(nanos % NANOS_PER_SEC) as u32,

Check failure on line 138 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 138 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

using a potentially dangerous silent `as` conversion

Check failure on line 138 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, stable)

using a potentially dangerous silent `as` conversion

Check failure on line 138 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

using a potentially dangerous silent `as` conversion
))
}

Expand Down Expand Up @@ -194,3 +196,47 @@
// 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> {

Check failure on line 218 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (ubuntu-latest, nightly)

the following explicit lifetimes could be elided: 'a

Check failure on line 218 in serde_with/src/utils.rs

View workflow job for this annotation

GitHub Actions / clippy_check (windows-latest, nightly)

the following explicit lifetimes could be elided: '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