Repository: toml-rs/toml
Pull request #637 (2023-10-26 04:09:24)

Fix handling of NaN sign throughout parsing and serialization
I learned recently in https://github.com/rust-lang/miri/issues/3139 that `as` produces NaN with a nondeterministic sign, and separately that the sign of f64::NAN is not specified (may be a negative NaN). As of https://github.com/rust-lang/rust/pull/116551 Miri has begun intentionally exercising these cases.

This PR fixes places where `-nan` would incorrectly be deserialized as a positive NaN, `nan` or `+nan` would be deserialized as a negative NaN, negative NaN would be serialized as `nan`, or positive NaN would be serialized as `-nan`. It adds tests to confirm the expected sign of NaN values, and improves existing tests related to NaN.


Patch:
diff --git a/crates/toml/src/macros.rs b/crates/toml/src/macros.rs
index d86cc52f..a2959700 100644
--- a/crates/toml/src/macros.rs
+++ b/crates/toml/src/macros.rs
@@ -198,15 +198,15 @@ macro_rules! toml_internal {
     }};
 
     (@value (-nan)) => {
-        $crate::Value::Float(-::std::f64::NAN)
+        $crate::Value::Float(::std::f64::NAN.copysign(-1.0))
     };
 
     (@value (nan)) => {
-        $crate::Value::Float(::std::f64::NAN)
+        $crate::Value::Float(::std::f64::NAN.copysign(1.0))
     };
 
     (@value nan) => {
-        $crate::Value::Float(::std::f64::NAN)
+        $crate::Value::Float(::std::f64::NAN.copysign(1.0))
     };
 
     (@value (-inf)) => {
diff --git a/crates/toml/src/value.rs b/crates/toml/src/value.rs
index e52e5a6a..c8065a6c 100644
--- a/crates/toml/src/value.rs
+++ b/crates/toml/src/value.rs
@@ -921,7 +921,9 @@ impl ser::Serializer for ValueSerializer {
     }
 
     fn serialize_f32(self, value: f32) -> Result<Value, crate::ser::Error> {
-        self.serialize_f64(value.into())
+        // Preserve sign of NaN. The `as` produces a nondeterministic sign.
+        let sign = if value.is_sign_positive() { 1.0 } else { -1.0 };
+        self.serialize_f64((value as f64).copysign(sign))
     }
 
     fn serialize_f64(self, value: f64) -> Result<Value, crate::ser::Error> {
diff --git a/crates/toml_edit/src/parser/numbers.rs b/crates/toml_edit/src/parser/numbers.rs
index 6e4757f0..4c77f51c 100644
--- a/crates/toml_edit/src/parser/numbers.rs
+++ b/crates/toml_edit/src/parser/numbers.rs
@@ -301,7 +301,7 @@ pub(crate) fn inf(input: &mut Input<'_>) -> PResult<f64> {
 const INF: &[u8] = b"inf";
 // nan = %x6e.61.6e  ; nan
 pub(crate) fn nan(input: &mut Input<'_>) -> PResult<f64> {
-    tag(NAN).value(f64::NAN).parse_next(input)
+    tag(NAN).value(f64::NAN.copysign(1.0)).parse_next(input)
 }
 const NAN: &[u8] = b"nan";
 
@@ -353,6 +353,7 @@ mod test {
     fn assert_float_eq(actual: f64, expected: f64) {
         if expected.is_nan() {
             assert!(actual.is_nan());
+            assert_eq!(expected.is_sign_positive(), actual.is_sign_positive());
         } else if expected.is_infinite() {
             assert!(actual.is_infinite());
             assert_eq!(expected.is_sign_positive(), actual.is_sign_positive());
@@ -376,9 +377,9 @@ mod test {
             ("9_224_617.445_991_228_313", 9_224_617.445_991_227),
             ("-1.7976931348623157e+308", std::f64::MIN),
             ("1.7976931348623157e+308", std::f64::MAX),
-            ("nan", f64::NAN),
-            ("+nan", f64::NAN),
-            ("-nan", f64::NAN),
+            ("nan", f64::NAN.copysign(1.0)),
+            ("+nan", f64::NAN.copysign(1.0)),
+            ("-nan", f64::NAN.copysign(-1.0)),
             ("inf", f64::INFINITY),
             ("+inf", f64::INFINITY),
             ("-inf", f64::NEG_INFINITY),
diff --git a/crates/toml_edit/src/ser/value.rs b/crates/toml_edit/src/ser/value.rs
index 808bb900..f250d63d 100644
--- a/crates/toml_edit/src/ser/value.rs
+++ b/crates/toml_edit/src/ser/value.rs
@@ -105,7 +105,9 @@ impl serde::ser::Serializer for ValueSerializer {
     }
 
     fn serialize_f32(self, v: f32) -> Result<Self::Ok, Self::Error> {
-        self.serialize_f64(v as f64)
+        // Preserve sign of NaN. The `as` produces a nondeterministic sign.
+        let sign = if v.is_sign_positive() { 1.0 } else { -1.0 };
+        self.serialize_f64((v as f64).copysign(sign))
     }
 
     fn serialize_f64(self, v: f64) -> Result<Self::Ok, Self::Error> {


Test patch:
diff --git a/crates/toml/tests/testsuite/macros.rs b/crates/toml/tests/testsuite/macros.rs
index 51007051..e002cd91 100644
--- a/crates/toml/tests/testsuite/macros.rs
+++ b/crates/toml/tests/testsuite/macros.rs
@@ -198,9 +198,18 @@ fn test_nan() {
         sf5 = +nan
         sf6 = -nan
     };
-    assert!(actual["sf4"].as_float().unwrap().is_nan());
-    assert!(actual["sf5"].as_float().unwrap().is_nan());
-    assert!(actual["sf6"].as_float().unwrap().is_nan());
+
+    let sf4 = actual["sf4"].as_float().unwrap();
+    assert!(sf4.is_nan());
+    assert!(sf4.is_sign_positive());
+
+    let sf5 = actual["sf5"].as_float().unwrap();
+    assert!(sf5.is_nan());
+    assert!(sf5.is_sign_positive());
+
+    let sf6 = actual["sf6"].as_float().unwrap();
+    assert!(sf6.is_nan());
+    assert!(sf6.is_sign_negative());
 }
 
 #[test]