From 932539bd75062975155e88f0cbe2f15ce9ba520a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 25 Apr 2017 06:49:23 +0900 Subject: [PATCH] Fix overflow in rgb() parsing. `some_f32 as i32` would warp --- Cargo.toml | 2 +- src/color.rs | 47 ++++++++++++++++--------------- src/css-parsing-tests/color3.json | 3 ++ 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e1c1de94..ba3cd60d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "cssparser" -version = "0.13.0" +version = "0.13.1" authors = [ "Simon Sapin " ] description = "Rust implementation of CSS Syntax Level 3" diff --git a/src/color.rs b/src/color.rs index 23ff92fd..d6a44d1f 100644 --- a/src/color.rs +++ b/src/color.rs @@ -2,7 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::cmp; use std::fmt; use std::f32::consts::PI; @@ -31,7 +30,12 @@ impl RGBA { /// clamped to the 0.0 ... 1.0 range. #[inline] pub fn from_floats(red: f32, green: f32, blue: f32, alpha: f32) -> Self { - Self::new(clamp_f32(red), clamp_f32(green), clamp_f32(blue), clamp_f32(alpha)) + Self::new( + clamp_unit_f32(red), + clamp_unit_f32(green), + clamp_unit_f32(blue), + clamp_unit_f32(alpha), + ) } /// Returns a transparent color. @@ -99,7 +103,7 @@ impl ToCss for RGBA { { // Try first with two decimal places, then with three. let mut rounded_alpha = (self.alpha_f32() * 100.).round() / 100.; - if clamp_f32(rounded_alpha) != self.alpha { + if clamp_unit_f32(rounded_alpha) != self.alpha { rounded_alpha = (self.alpha_f32() * 1000.).round() / 1000.; } @@ -381,12 +385,7 @@ fn parse_color_hash(value: &str) -> Result { } } - -fn clamp_i32(val: i32) -> u8 { - cmp::min(cmp::max(0, val), 255) as u8 -} - -fn clamp_f32(val: f32) -> u8 { +fn clamp_unit_f32(val: f32) -> u8 { // Scale by 256, not 255, so that each of the 256 u8 values has an equal range // of f32 values mapping to it. Floor before clamping. // @@ -394,7 +393,11 @@ fn clamp_f32(val: f32) -> u8 { // `256.0_f32 as u8` is undefined behavior: // // https://github.com/rust-lang/rust/issues/10184 - (val * 256.).floor().max(0.).min(255.) as u8 + clamp_256_f32(val * 256.) +} + +fn clamp_256_f32(val: f32) -> u8 { + val.floor().max(0.).min(255.) as u8 } #[inline] @@ -417,10 +420,10 @@ fn parse_color_function(name: &str, arguments: &mut Parser) -> Result let token = try!(arguments.next()); match token { Token::Number(NumericValue { value: v, .. }) => { - clamp_f32(v) + clamp_unit_f32(v) } Token::Percentage(ref v) => { - clamp_f32(v.unit_value) + clamp_unit_f32(v.unit_value) } _ => { return Err(()) @@ -446,23 +449,23 @@ fn parse_rgb_components_rgb(arguments: &mut Parser) -> Result<(u8, u8, u8, bool) // https://drafts.csswg.org/css-color/#rgb-functions match try!(arguments.next()) { Token::Number(NumericValue { value: v, .. }) => { - red = clamp_i32(v as i32); - green = clamp_i32(match try!(arguments.next()) { + red = clamp_256_f32(v); + green = clamp_256_f32(match try!(arguments.next()) { Token::Number(NumericValue { value: v, .. }) => v, Token::Comma => { uses_commas = true; try!(arguments.expect_number()) } _ => return Err(()) - } as i32); + }); if uses_commas { try!(arguments.expect_comma()); } - blue = clamp_i32(try!(arguments.expect_number()) as i32); + blue = clamp_256_f32(try!(arguments.expect_number())); } Token::Percentage(ref v) => { - red = clamp_f32(v.unit_value); - green = clamp_f32(match try!(arguments.next()) { + red = clamp_unit_f32(v.unit_value); + green = clamp_unit_f32(match try!(arguments.next()) { Token::Percentage(ref v) => v.unit_value, Token::Comma => { uses_commas = true; @@ -473,7 +476,7 @@ fn parse_rgb_components_rgb(arguments: &mut Parser) -> Result<(u8, u8, u8, bool) if uses_commas { try!(arguments.expect_comma()); } - blue = clamp_f32(try!(arguments.expect_percentage())); + blue = clamp_unit_f32(try!(arguments.expect_percentage())); } _ => return Err(()) }; @@ -536,8 +539,8 @@ fn parse_rgb_components_hsl(arguments: &mut Parser) -> Result<(u8, u8, u8, bool) else { lightness + saturation - lightness * saturation }; let m1 = lightness * 2. - m2; let hue_times_3 = hue * 3.; - let red = clamp_f32(hue_to_rgb(m1, m2, hue_times_3 + 1.)); - let green = clamp_f32(hue_to_rgb(m1, m2, hue_times_3)); - let blue = clamp_f32(hue_to_rgb(m1, m2, hue_times_3 - 1.)); + let red = clamp_unit_f32(hue_to_rgb(m1, m2, hue_times_3 + 1.)); + let green = clamp_unit_f32(hue_to_rgb(m1, m2, hue_times_3)); + let blue = clamp_unit_f32(hue_to_rgb(m1, m2, hue_times_3 - 1.)); return Ok((red, green, blue, uses_commas)); } diff --git a/src/css-parsing-tests/color3.json b/src/css-parsing-tests/color3.json index 3a987ccd..d018749a 100644 --- a/src/css-parsing-tests/color3.json +++ b/src/css-parsing-tests/color3.json @@ -253,5 +253,8 @@ "hsl(10.4719755118rad, 75%, 50%)", [32, 32, 224, 255], "hsl(2.6666666666turn, 75%, 50%)", [32, 32, 224, 255], +"rgb(-2147483649, 4294967298, -18446744073709551619) /* https://github.com/w3c/web-platform-tests/blob/master/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.rgb-clamp-3.html */", +[0, 255, 0, 255], + "cmyk(0, 0, 0, 0)", null ]