[BartWaardenburg]

Fixes #460.

A hue that overflows f32, such as hsl(1e40 50% 50%), made normalize_hue return NaN. The NaN hue failed the debug assertion in hsl_to_rgb. In a release build, hsl(1e40 50% 50%) serialized as rgb(64, 64, 64).

This change makes normalize_hue return 0 for an infinite or NaN hue. This follows the spec:

• CSS Color 4 normalizes an infinite hue to 0 degrees.
• CSS Values 4 censors NaN to 0.

The fix also covers lch() and oklch(), which use the same function. Before, lch(50% 30 1e40) serialized as lch(50 30 calc(NaN)). Now it serializes as lch(50 30 0).

Tests:

• normalize_non_finite_hue covers 1e40 and -1e40 in hsl(), hwb(), lch() and oklch(). It also covers angle units that overflow, such as 1e38turn.
• normalize_finite_hue checks that finite hues outside [0, 360) still wrap as before.

The new non-finite test fails on main in debug and release builds. cargo test -p cssparser-color passes with this change, in debug and release builds.

Found by fuzzing Fallow (https://github.com/fallow-rs/fallow) with OSS-Fuzz.

Thanks for maintaining this crate.

[mrobinson]

Do you mind contributing this directly upstream to Gecko? This is a downstream version of Stylo that regularly syncs with upstream. If you submit your change there, then it will soon make it's way here as well.

[BartWaardenburg]

Thanks for looking at this, @mrobinson, and for the pointer to the Gecko docs.

I want to make sure I send it to the right place, so I tried to find this code in Gecko first. I may be missing something, but I could not find it there:

• Gecko's Cargo.lock seems to take cssparser and cssparser-macros from this repository at a git revision (9f3ceddf), and vendors them in third_party/rust/.
• I could not find the cssparser-color crate (color/lib.rs, where this change is) in Gecko's lock file or vendor tree. I only saw it in supply-chain/audits.toml.

Is cssparser-color maintained in Gecko under another path that I did not find? If so, I am happy to send the patch there. If it lives only here, would a fix in this repository be the right route after all?

I also noticed that Stylo in Gecko has the same normalize_hue formula in servo/components/style/color/convert.rs. I have not checked whether a non-finite hue can reach it in Firefox. Would a Bugzilla bug for that be useful?

[mrobinson]

Ah, apologies. I thought this was the stylo repository. 🤦 This is indeed the right place to submit this!

[yisibl]

As far as I know, the same issue exists in Firefox as well. So it would be great if you could submit a WPT test—that way, all browsers would benefit.

[BartWaardenburg]

Thanks for the suggestion, @yisibl. Before I add a test, I want to check one thing with you.

WPT already tests calc(infinity) and calc(-infinity) as the hue of hsl(), hwb(), lch() and oklch(), and expects 0 degrees (css/css-color/parsing/color-valid-hsl.html, color-valid-hwb.html, color-computed-lab.html). This PR also gives 0 for an infinite hue. If I read wpt.fyi correctly, Firefox fails the LCH and OkLCh cases, where the hue stays NaN (lch(50 10 calc(NaN))). Bug 2067978 tracks those failures.

For a literal like 1e40, I am less sure what a test should expect. CSS Values 4 says a value that an implementation cannot represent "must be converted to the closest value supported by the implementation". Blink clamps number tokens to FLT_MAX, which is a multiple of 360, so it gives a hue of 0. An engine that keeps the literal as a double gets 112 with a plain fmod. So the expected value for 1e40 depends on each engine's number precision, and the spec does not fix it.

Would a test with a value beyond every engine's range, such as 1e400deg, be useful? Or are the existing calc(infinity) tests enough here? I am happy to write the WPT PR either way.