Avoid parsing to non-finite floating point numbers

[SimonSapin]

In #165, this input 3E833 is parsed as a Token::Number whose value is std::f32::INFINITY, which serializes as inf.

We should definitely not serialize any number as inf (maybe an arbitrary large number instead), but probably also avoid non-finite numbers in tokens in the first place.

CC https://github.com/servo/servo/issues/15729

[federicomenaquintero]

FWIW, in librsvg I ended up with this:

pub trait CssParserExt {
    fn expect_finite_number(&mut self) -> Result<f32, ValueErrorKind>;
}

impl<'i, 't> CssParserExt for Parser<'i, 't> {
    fn expect_finite_number(&mut self) -> Result<f32, ValueErrorKind> {
        finite_f32(self.expect_number()?)
    }
}

pub fn finite_f32(n: f32) -> Result<f32, ValueErrorKind> {
    if n.is_finite() {
        Ok(n)
    } else {
        Err(ValueErrorKind::Value("expected finite number".to_string()))
    }
}

Of course this is tied to librsvg's error enums. The code uses expect_finite_number everywhere instead of cssparser's stock expect_number.

[federicomenaquintero]

I guess the question is, should encountering a non-finite number just yield an UnexpectedToken error? I don't know the details of the CSS spec well enough :)

[SimonSapin]

https://drafts.csswg.org/css-values/#numeric-ranges

CSS theoretically supports infinite precision and infinite ranges for all value types; however in reality implementations have finite capacity. UAs should support reasonably useful ranges and precisions.

Unfortunately I couldn’t easily find spec text that says what to do with inputs that exceed that chosen capacity.

[emilio]

I think we should parse it, and clamp it to some appropriate range. We should probably just not serialize inf. Gecko doesn't serialize inf, maybe this bug was fixed?

[SimonSapin]

Probably when we started using the dtoa crate:

fn main() {
    dbg!(dtoa_s(std::f32::INFINITY));
    dbg!(dtoa_s(std::f32::NAN));
    dbg!(dtoa_s(std::f32::MAX));
}

fn dtoa_s(x: f32) -> String {
    let mut v = Vec::<u8>::new();
    dtoa::write(&mut v, x).unwrap();
    String::from_utf8(v).unwrap()
}

Playground, output:

[src/main.rs:2] dtoa_s(std::f32::INFINITY) = "3.4028237e38"
[src/main.rs:3] dtoa_s(std::f32::NAN) = "5.1042355e38"
[src/main.rs:4] dtoa_s(std::f32::MAX) = "3.4028235e38"

[federicomenaquintero]

Section 5.1 says:

If the value is outside the allowed range, then unless otherwise specified, the declaration is invalid and must be ignored.

So, "too large" -> "parse error" sounds reasonable.

But section 10.9 has an interesting note:

Note: By definition, ±∞ are outside the allowed range for any property, and will clamp to the minimum/maximum value allowed.

So clamping to $large_value sounds fine too! If that makes number values rountrippable, this sounds good.

[SimonSapin]

Parse errors seems like it might not be web-compatible at least for z-index where some pages use ever-larger value