Skip to content
Merged
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
9 changes: 9 additions & 0 deletions buffa-descriptor/src/reflect/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ impl DynamicMessage {
/// [`with_element_memory_limit`](DynamicMessageSeed::with_element_memory_limit)
/// gives each charge.
///
/// Scalars decode as in generated messages. In particular, an unquoted
/// `float` with a fraction or exponent is rounded to `f64` and then to
/// `f32`, so it can decode one `f32` ulp away from the same value quoted;
/// see [`buffa::json_helpers::float`].
///
/// # Errors
///
/// Returns a `serde_json::Error` if the input is not valid JSON, does
Expand Down Expand Up @@ -1143,6 +1148,10 @@ fn scalar_from_u64(sc: ScalarType, v: u64) -> Option<Value> {
/// across the full range, and unquoted floats are rejected above the
/// magnitude where serde_json's float parsing can no longer identify the
/// token uniquely.
///
/// An unquoted `float` with a fraction or exponent arrives here already
/// rounded to `f64`, so it is rounded twice, as in generated code: see
/// `json_helpers::float`.
fn scalar_from_f64<E: de::Error>(sc: ScalarType, v: f64) -> Result<Value, E> {
let d = v.into_deserializer();
Ok(match sc {
Expand Down
18 changes: 18 additions & 0 deletions buffa-descriptor/tests/json_e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,24 @@ fn json_quoted_integer_bounds_match_generated_messages() {
}
}

/// An unquoted `float` reaches the reflective parser as an `f64` and is
/// narrowed, as in generated code, so a decimal next to an `f32` midpoint
/// decodes differently from the same decimal quoted (#647).
#[test]
fn json_unquoted_float_rounds_through_f64_as_generated_messages_do() {
let p = pool();
let idx = p.message_index("reflect.test.Scalars").unwrap();
let parse = |json: &str| {
let msg = DynamicMessage::from_json(Arc::clone(&p), idx, json).expect("float parses");
match msg.field_by_number(2) {
Some(Value::F32(f)) => f.to_bits(),
other => panic!("expected F32, got {other:?}"),
}
};
assert_eq!(parse(r#"{"fFloat": 1.0000000596046448}"#), 0x3f80_0000);
assert_eq!(parse(r#"{"fFloat": "1.0000000596046448"}"#), 0x3f80_0001);
}

/// Quoted floats go through the same `json_helpers` modules as generated
/// messages, so out-of-range strings are rejected rather than saturating to
/// infinity, and only the three exact tokens spell a non-finite value.
Expand Down
4 changes: 4 additions & 0 deletions buffa-types/src/wrapper_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ impl serde::Serialize for FloatValue {
}
}

/// Accepts what [`buffa::json_helpers::float`] accepts. An unquoted number
/// with a fraction or exponent is rounded to `f64` and then to `f32`, so it
/// can decode one `f32` ulp away from the same value quoted; that module's
/// documentation says when.
#[cfg(feature = "json")]
impl<'de> serde::Deserialize<'de> for FloatValue {
fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
Expand Down
17 changes: 17 additions & 0 deletions buffa/src/json_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,23 @@ int_serde_module!(
/// deserialization. A number or quoted decimal outside the `f32` range is an
/// error, not infinity, and `"inf"` / `"nan"` in other spellings are rejected.
///
/// A quoted decimal is parsed straight to `f32`, so it is rounded once, and
/// so is an unquoted integer in the `i64` or `u64` range. Any other unquoted
/// number reaches the visitor as the `f64` the deserializer parsed and is then
/// narrowed to `f32`, so it is rounded twice. The results differ, by one `f32`
/// ulp, when a decimal close to the midpoint between two adjacent `f32` values
/// becomes that midpoint as an `f64`, which then rounds to even:
/// `1.0000000596046448` decodes to `1.0` unquoted and to its nearest `f32`,
/// `1.0000001`, quoted. How close is close enough depends on the `f64` parse
/// (half an `f64` ulp with `serde_json`'s `float_roundtrip` feature, somewhat
/// more without it). Just below the `f32` overflow threshold, the same effect
/// makes an unquoted value an error that parses when quoted.
///
/// protobuf C++ also narrows a parsed `double`; protobuf-go rounds once. The
/// `f64` does not identify the decimal it came from, so the decoding side
/// cannot avoid this. A producer that wants single rounding can quote the
/// value. Tracked in [#647](https://github.com/anthropics/buffa/issues/647).
///
/// Use with `#[serde(with = "::buffa::json_helpers::float")]`.
pub mod float {
use serde::{Deserializer, Serializer};
Expand Down
18 changes: 18 additions & 0 deletions buffa/src/json_helpers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,24 @@ fn float_deserialize_table() {
}
}

#[test]
fn float_unquoted_rounds_through_f64_quoted_rounds_once() {
// Just above 1 + 2^-24, the midpoint of 1.0 and the next f32, so the
// nearest f32 is 1.0000001. As an f64 it is the midpoint exactly, which
// narrowing then rounds to even. Pins the documented difference (#647).
let json = "1.0000000596046448";
let unquoted: SerdeFloat = serde_json::from_str(json).unwrap();
assert_eq!(unquoted.0.to_bits(), 0x3f80_0000);
let quoted: SerdeFloat = serde_json::from_str(&format!("\"{json}\"")).unwrap();
assert_eq!(quoted.0.to_bits(), 0x3f80_0001);
// An unquoted integer is converted once, so 2^24 + 1 rounds to even the
// same way quoted or not.
for json in ["16777217", r#""16777217""#] {
let v: SerdeFloat = serde_json::from_str(json).unwrap();
assert_eq!(v.0.to_bits(), 0x4b80_0000, "input: {json}");
}
}

#[test]
fn double_serialize_table() {
#[rustfmt::skip]
Expand Down
10 changes: 10 additions & 0 deletions docs/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -1764,6 +1764,16 @@ build, run `cargo tree -e features -i serde_json` in your workspace, and look
for `arbitrary_precision` in the output. Tracked in
[#482](https://github.com/anthropics/buffa/issues/482).

An unquoted `float` value with a fraction or exponent is parsed as an `f64`
and then narrowed to `f32`, so it is rounded twice. A quoted one is parsed
straight to `f32`. The two can differ by one `f32` ulp for a decimal very
close to the midpoint between two `f32` values: `1.0000000596046448` decodes
to `1.0` unquoted and `1.0000001` quoted. protobuf C++ narrows the same way;
protobuf-go rounds once. A producer that needs single rounding can quote the
value. Generated messages, `FloatValue` and `DynamicMessage::from_json` all
behave this way. Tracked in
[#647](https://github.com/anthropics/buffa/issues/647).

If one of your own types keeps untrusted JSON as a `serde_json::Value` before
decoding it, do not deserialize that value with `Value`'s own `Deserialize`
impl. When any crate in the build enables `serde_json`'s `raw_value` feature,
Expand Down
Loading