Zvariant asserts on f32::NAN values

[Spindel]

Seems that f32::NAN get serialized to dbus as f64::NAN, but when read back again, it passes through an assert! statement that the value is in acceptable range for f32 and thus panics the application.

A suitable fix might be to either return an Result error rather than panic in a function that returns an Result, or simply convert f64::NAN to f32::NAN and hope nobody notices that it's not the same NAN value?

Testcase:

use zvariant::{self, serialized::Context, serialized::Data, Type};
use serde::{Deserialize, Serialize};
use std::time::SystemTime;

#[derive(Debug, Serialize, Type, Deserialize)]
struct First {
    time: SystemTime,
    value: f64,
}

impl Default for First {
    fn default() -> Self {
        Self {
            time: SystemTime::now(),
            value: f64::NAN,
        }
    }
}
#[derive(Debug, Serialize, Type, Deserialize)]
struct Second {
    time: SystemTime,
    value: f32,
}
impl Default for Second {
    fn default() -> Self {
        Self {
            time: SystemTime::now(),
            value: f32::NAN,
        }
    }
}

fn encoder<T>(source: &T) -> Vec<u8>
where
    T: zvariant::Type + serde::Serialize,
{
    let ctxt = Context::new_dbus(zvariant::LE, 0);
    let bytes = zvariant::to_bytes(ctxt, &source).expect("Failed to serialize");
    bytes.bytes().into()
}

fn decoder<T>(bytes: &[u8]) -> T
where
    T: zvariant::Type + for<'a> serde::Deserialize<'a>,
{
    let ctxt = Context::new_dbus(zvariant::LE, 0);
    let data = Data::new(bytes, ctxt);
    let decoded: T = data.deserialize().expect("Failed deserialization").0;
    decoded
}

fn first_case() {
    let source = First {
        time: SystemTime::now(),
        value: f64::NAN,
    };
    let bytes = encoder(&source);
    let back: First = decoder(&bytes);
    assert_eq!(source.time, back.time);
    assert!(back.value.is_nan());
}
fn second_case() {
    let source = Second {
        time: SystemTime::now(),
        value: f32::NAN,
    };
    let bytes = encoder(&source);
    let back: Second = decoder(&bytes);
    assert_eq!(source.time, back.time);
    assert!(back.value.is_nan());
}

fn main() {
    println!("F64 values");
    first_case();
    println!("F32 values");
    second_case();
}

[zeenix]

Freaking floating points and NaNs. :)

A suitable fix might be to either return an Result error rather than panic in a function that returns an Result, or simply convert f64::NAN to f32::NAN and hope nobody notices that it's not the same NAN value?

I'd rather prefer the latter since it's not a nice thing to not be able to handle your own serialized data. Any chance you could provide a PR?

[Spindel]

I can try, didn't get it to build the tests due to the C deps last, so I haven't gotten a test-case integrated to check.

I think a basic patch like this ought to do it, simply allowing Nan & Inf conversions to happen as-is.

diff --git a/zvariant/src/utils.rs b/zvariant/src/utils.rs
index 3c6cb60a..615af564 100644
--- a/zvariant/src/utils.rs
+++ b/zvariant/src/utils.rs
@@ -67,7 +67,7 @@ pub(crate) fn usize_to_u8(value: usize) -> u8 {
 }

 pub(crate) fn f64_to_f32(value: f64) -> f32 {
-    assert!(value <= (f32::MAX as f64), "{} too large for `f32`", value,);
+    assert!(!value.is_finite() || value <= (f32::MAX as f64), "{} too large for `f32`", value,);

     value as f32
 }

[zeenix]

I can try, didn't get it to build the tests due to the C deps last

What C deps? you mean glib dep? That's only a dev-dep and you can easily disable it in the Cargo.toml w/o any issues (it's only needed for gvariant feature, which is disabled by default). Besides doesn't your distro package glib?

[Spindel]

I can try, didn't get it to build the tests due to the C deps last

What C deps? you mean glib dep? That's only a dev-dep and you can easily disable it in the Cargo.toml w/o any issues (it's only needed for gvariant feature, which is disabled by default). Besides doesn't your distro package glib?

I had glib, but I was also a bit pressed on time and when compiling ended up with another C splat of deps I just shrugged and moved on rather than figure out the deps needed that time. ( I had had enough of that from fightiing with python and incompatible libxml+libxslt versions that day. )

Anyhow, PR is up and works on my machine (tm)