Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 04:41:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1925551 - Better validate relative color components. r=tlouw,firefox-style-system-reviewers,layout-reviewers

Browse Source 
This is simpler and fixes the remaining parsing issues.

Differential Revision: https://phabricator.services.mozilla.com/D226160
This commit is contained in:
Emilio Cobos Álvarez 2024-10-18 19:28:12 +00:00
parent 7e3b5a4a20
commit 9bcfcdda97
4 changed files with 52 additions and 189 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
servo/components/style/color/color_function.rs
View File

@ -90,7 +90,8 @@ pub enum ColorFunction<OriginColor> {
}
impl ColorFunction<AbsoluteColor> {
fn resolve_to_absolute(&self) -> Result<AbsoluteColor, ()> {
/// Try to resolve into a valid absolute color.
pub fn resolve_to_absolute(&self) -> Result<AbsoluteColor, ()> {
macro_rules! alpha {
($alpha:expr, $origin_color:expr) => {{
$alpha

33
servo/components/style/color/component.rs
View File

@ -19,7 +19,7 @@ use crate::{
},
};
use cssparser::{color::OPAQUE, Parser, Token};
use style_traits::{ParseError, StyleParseErrorKind, ToCss};
use style_traits::{ParseError, ToCss};
/// A single color component.
#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
@ -69,7 +69,6 @@ impl<ValueType: ColorComponentType> ColorComponent<ValueType> {
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_none: bool,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<Self, ParseError<'i>> {
let location = input.current_source_location();
@ -81,11 +80,6 @@ impl<ValueType: ColorComponentType> ColorComponent<ValueType> {
let Ok(channel_keyword) = ChannelKeyword::from_ident(ident) else {
return Err(location.new_unexpected_token_error(t.clone()));
};
if !allowed_channel_keywords.contains(&channel_keyword) {
return Err(location.new_unexpected_token_error(t.clone()));
}
let node = GenericCalcNode::Leaf(Leaf::ColorComponent(channel_keyword));
Ok(ColorComponent::Calc(Box::new(node)))
},
@ -98,31 +92,6 @@ impl<ValueType: ColorComponentType> ColorComponent<ValueType> {
};
let mut node = GenericCalcNode::parse(context, input, function, units)?;
if rcs_enabled() {
// Check that we only have allowed channel_keywords.
// TODO(tlouw): Optimize this to fail when we hit the first error, or even
// better, do the validation during parsing the calc node.
let mut is_valid = true;
node.visit_depth_first(|node| {
let GenericCalcNode::Leaf(leaf) = node else {
return;
};
let Leaf::ColorComponent(channel_keyword) = leaf else {
return;
};
if !allowed_channel_keywords.contains(channel_keyword) {
is_valid = false;
}
});
if !is_valid {
return Err(
location.new_custom_error(StyleParseErrorKind::UnspecifiedError)
);
}
}
// TODO(tlouw): We only have to simplify the node when we have to store it, but we
// only know if we have to store it much later when the whole color
// can't be resolved to absolute at which point the calc nodes are

157
servo/components/style/color/parsing.rs
View File

@ -62,48 +62,6 @@ pub enum ChannelKeyword {
Z,
}
const RGB_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::R,
ChannelKeyword::G,
ChannelKeyword::B,
ChannelKeyword::Alpha,
];
const HSL_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::H,
ChannelKeyword::S,
ChannelKeyword::L,
ChannelKeyword::Alpha,
];
const HWB_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::H,
ChannelKeyword::W,
ChannelKeyword::B,
ChannelKeyword::Alpha,
];
const LAB_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::L,
ChannelKeyword::A,
ChannelKeyword::B,
ChannelKeyword::Alpha,
];
const LCH_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::L,
ChannelKeyword::C,
ChannelKeyword::H,
ChannelKeyword::Alpha,
];
const XYZ_CHANNEL_KEYWORDS: &[ChannelKeyword] = &[
ChannelKeyword::X,
ChannelKeyword::Y,
ChannelKeyword::Z,
ChannelKeyword::Alpha,
];
/// Return the named color with the given name.
///
/// Matching is case-insensitive in the ASCII range.
@ -169,6 +127,7 @@ fn parse_color_function<'i, 't>(
arguments: &mut Parser<'i, 't>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let origin_color = parse_origin_color(context, arguments)?;
let has_origin_color = origin_color.is_some();
let color = match_ignore_ascii_case! { &name,
"rgb" | "rgba" => parse_rgb(context, arguments, origin_color),
@ -182,6 +141,16 @@ fn parse_color_function<'i, 't>(
_ => return Err(arguments.new_unexpected_token_error(Token::Ident(name))),
}?;
if has_origin_color {
// Validate the channels and calc expressions by trying to resolve them against
// transparent.
// FIXME(emilio, bug 1925572): This could avoid cloning, or be done earlier.
let abs = color.map_origin_color(|_| Some(AbsoluteColor::TRANSPARENT_BLACK));
if abs.resolve_to_absolute().is_err() {
return Err(arguments.new_custom_error(StyleParseErrorKind::UnspecifiedError));
}
}
arguments.expect_exhausted()?;
Ok(color)
@ -214,7 +183,7 @@ fn parse_rgb<'i, 't>(
arguments: &mut Parser<'i, 't>,
origin_color: Option<SpecifiedColor>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let maybe_red = parse_number_or_percentage(context, arguments, true, RGB_CHANNEL_KEYWORDS)?;
let maybe_red = parse_number_or_percentage(context, arguments, true)?;
// If the first component is not "none" and is followed by a comma, then we
// are parsing the legacy syntax. Legacy syntax also doesn't support an
@ -225,25 +194,25 @@ fn parse_rgb<'i, 't>(
Ok(if is_legacy_syntax {
let (green, blue) = if maybe_red.could_be_percentage() {
let green = parse_percentage(context, arguments, false, RGB_CHANNEL_KEYWORDS)?;
let green = parse_percentage(context, arguments, false)?;
arguments.expect_comma()?;
let blue = parse_percentage(context, arguments, false, RGB_CHANNEL_KEYWORDS)?;
let blue = parse_percentage(context, arguments, false)?;
(green, blue)
} else {
let green = parse_number(context, arguments, false, RGB_CHANNEL_KEYWORDS)?;
let green = parse_number(context, arguments, false)?;
arguments.expect_comma()?;
let blue = parse_number(context, arguments, false, RGB_CHANNEL_KEYWORDS)?;
let blue = parse_number(context, arguments, false)?;
(green, blue)
};
let alpha = parse_legacy_alpha(context, arguments, RGB_CHANNEL_KEYWORDS)?;
let alpha = parse_legacy_alpha(context, arguments)?;
ColorFunction::Rgb(origin_color.into(), maybe_red, green, blue, alpha)
} else {
let green = parse_number_or_percentage(context, arguments, true, RGB_CHANNEL_KEYWORDS)?;
let blue = parse_number_or_percentage(context, arguments, true, RGB_CHANNEL_KEYWORDS)?;
let green = parse_number_or_percentage(context, arguments, true)?;
let blue = parse_number_or_percentage(context, arguments, true)?;
let alpha = parse_modern_alpha(context, arguments, RGB_CHANNEL_KEYWORDS)?;
let alpha = parse_modern_alpha(context, arguments)?;
ColorFunction::Rgb(origin_color.into(), maybe_red, green, blue, alpha)
})
@ -258,7 +227,7 @@ fn parse_hsl<'i, 't>(
arguments: &mut Parser<'i, 't>,
origin_color: Option<SpecifiedColor>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let hue = parse_number_or_angle(context, arguments, true, HSL_CHANNEL_KEYWORDS)?;
let hue = parse_number_or_angle(context, arguments, true)?;
// If the hue is not "none" and is followed by a comma, then we are parsing
// the legacy syntax. Legacy syntax also doesn't support an origin color.
@ -267,16 +236,15 @@ fn parse_hsl<'i, 't>(
arguments.try_parse(|p| p.expect_comma()).is_ok();
let (saturation, lightness, alpha) = if is_legacy_syntax {
let saturation = parse_percentage(context, arguments, false, HSL_CHANNEL_KEYWORDS)?;
let saturation = parse_percentage(context, arguments, false)?;
arguments.expect_comma()?;
let lightness = parse_percentage(context, arguments, false, HSL_CHANNEL_KEYWORDS)?;
let alpha = parse_legacy_alpha(context, arguments, HSL_CHANNEL_KEYWORDS)?;
let lightness = parse_percentage(context, arguments, false)?;
let alpha = parse_legacy_alpha(context, arguments)?;
(saturation, lightness, alpha)
} else {
let saturation =
parse_number_or_percentage(context, arguments, true, HSL_CHANNEL_KEYWORDS)?;
let lightness = parse_number_or_percentage(context, arguments, true, HSL_CHANNEL_KEYWORDS)?;
let alpha = parse_modern_alpha(context, arguments, HSL_CHANNEL_KEYWORDS)?;
let saturation = parse_number_or_percentage(context, arguments, true)?;
let lightness = parse_number_or_percentage(context, arguments, true)?;
let alpha = parse_modern_alpha(context, arguments)?;
(saturation, lightness, alpha)
};
@ -298,11 +266,11 @@ fn parse_hwb<'i, 't>(
arguments: &mut Parser<'i, 't>,
origin_color: Option<SpecifiedColor>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let hue = parse_number_or_angle(context, arguments, true, HWB_CHANNEL_KEYWORDS)?;
let whiteness = parse_number_or_percentage(context, arguments, true, HWB_CHANNEL_KEYWORDS)?;
let blackness = parse_number_or_percentage(context, arguments, true, HWB_CHANNEL_KEYWORDS)?;
let hue = parse_number_or_angle(context, arguments, true)?;
let whiteness = parse_number_or_percentage(context, arguments, true)?;
let blackness = parse_number_or_percentage(context, arguments, true)?;
let alpha = parse_modern_alpha(context, arguments, HWB_CHANNEL_KEYWORDS)?;
let alpha = parse_modern_alpha(context, arguments)?;
Ok(ColorFunction::Hwb(
origin_color.into(),
@ -328,11 +296,11 @@ fn parse_lab_like<'i, 't>(
origin_color: Option<SpecifiedColor>,
into_color: IntoLabFn<ColorFunction<SpecifiedColor>>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let lightness = parse_number_or_percentage(context, arguments, true, LAB_CHANNEL_KEYWORDS)?;
let a = parse_number_or_percentage(context, arguments, true, LAB_CHANNEL_KEYWORDS)?;
let b = parse_number_or_percentage(context, arguments, true, LAB_CHANNEL_KEYWORDS)?;
let lightness = parse_number_or_percentage(context, arguments, true)?;
let a = parse_number_or_percentage(context, arguments, true)?;
let b = parse_number_or_percentage(context, arguments, true)?;
let alpha = parse_modern_alpha(context, arguments, LAB_CHANNEL_KEYWORDS)?;
let alpha = parse_modern_alpha(context, arguments)?;
Ok(into_color(origin_color.into(), lightness, a, b, alpha))
}
@ -352,11 +320,11 @@ fn parse_lch_like<'i, 't>(
origin_color: Option<SpecifiedColor>,
into_color: IntoLchFn<ColorFunction<SpecifiedColor>>,
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let lightness = parse_number_or_percentage(context, arguments, true, LCH_CHANNEL_KEYWORDS)?;
let chroma = parse_number_or_percentage(context, arguments, true, LCH_CHANNEL_KEYWORDS)?;
let hue = parse_number_or_angle(context, arguments, true, LCH_CHANNEL_KEYWORDS)?;
let lightness = parse_number_or_percentage(context, arguments, true)?;
let chroma = parse_number_or_percentage(context, arguments, true)?;
let hue = parse_number_or_angle(context, arguments, true)?;
let alpha = parse_modern_alpha(context, arguments, LCH_CHANNEL_KEYWORDS)?;
let alpha = parse_modern_alpha(context, arguments)?;
Ok(into_color(
origin_color.into(),
@ -376,21 +344,11 @@ fn parse_color_with_color_space<'i, 't>(
) -> Result<ColorFunction<SpecifiedColor>, ParseError<'i>> {
let color_space = PredefinedColorSpace::parse(arguments)?;
let allowed_channel_keywords = match color_space {
PredefinedColorSpace::Srgb |
PredefinedColorSpace::SrgbLinear |
PredefinedColorSpace::DisplayP3 |
PredefinedColorSpace::A98Rgb |
PredefinedColorSpace::ProphotoRgb |
PredefinedColorSpace::Rec2020 => RGB_CHANNEL_KEYWORDS,
PredefinedColorSpace::XyzD50 | PredefinedColorSpace::XyzD65 => XYZ_CHANNEL_KEYWORDS,
};
let c1 = parse_number_or_percentage(context, arguments, true)?;
let c2 = parse_number_or_percentage(context, arguments, true)?;
let c3 = parse_number_or_percentage(context, arguments, true)?;
let c1 = parse_number_or_percentage(context, arguments, true, allowed_channel_keywords)?;
let c2 = parse_number_or_percentage(context, arguments, true, allowed_channel_keywords)?;
let c3 = parse_number_or_percentage(context, arguments, true, allowed_channel_keywords)?;
let alpha = parse_modern_alpha(context, arguments, allowed_channel_keywords)?;
let alpha = parse_modern_alpha(context, arguments)?;
Ok(ColorFunction::Color(
origin_color.into(),
@ -542,9 +500,8 @@ fn parse_number_or_angle<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_none: bool,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrAngleComponent>, ParseError<'i>> {
ColorComponent::parse(context, input, allow_none, allowed_channel_keywords)
ColorComponent::parse(context, input, allow_none)
}
/// Parse a `<percentage>` value.
@ -552,17 +509,10 @@ fn parse_percentage<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_none: bool,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrPercentageComponent>, ParseError<'i>> {
let location = input.current_source_location();
let value = ColorComponent::<NumberOrPercentageComponent>::parse(
context,
input,
allow_none,
allowed_channel_keywords,
)?;
let value = ColorComponent::<NumberOrPercentageComponent>::parse(context, input, allow_none)?;
if !value.could_be_percentage() {
return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
}
@ -575,16 +525,10 @@ fn parse_number<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_none: bool,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrPercentageComponent>, ParseError<'i>> {
let location = input.current_source_location();
let value = ColorComponent::<NumberOrPercentageComponent>::parse(
context,
input,
allow_none,
allowed_channel_keywords,
)?;
let value = ColorComponent::<NumberOrPercentageComponent>::parse(context, input, allow_none)?;
if !value.could_be_number() {
return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
@ -598,19 +542,17 @@ fn parse_number_or_percentage<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
allow_none: bool,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrPercentageComponent>, ParseError<'i>> {
ColorComponent::parse(context, input, allow_none, allowed_channel_keywords)
ColorComponent::parse(context, input, allow_none)
}
fn parse_legacy_alpha<'i, 't>(
context: &ParserContext,
arguments: &mut Parser<'i, 't>,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrPercentageComponent>, ParseError<'i>> {
if !arguments.is_exhausted() {
arguments.expect_comma()?;
parse_number_or_percentage(context, arguments, false, allowed_channel_keywords)
parse_number_or_percentage(context, arguments, false)
} else {
Ok(ColorComponent::AlphaOmitted)
}
@ -619,11 +561,10 @@ fn parse_legacy_alpha<'i, 't>(
fn parse_modern_alpha<'i, 't>(
context: &ParserContext,
arguments: &mut Parser<'i, 't>,
allowed_channel_keywords: &[ChannelKeyword],
) -> Result<ColorComponent<NumberOrPercentageComponent>, ParseError<'i>> {
if !arguments.is_exhausted() {
arguments.expect_delim('/')?;
parse_number_or_percentage(context, arguments, true, allowed_channel_keywords)
parse_number_or_percentage(context, arguments, true)
} else {
Ok(ColorComponent::AlphaOmitted)
}

48
testing/web-platform/meta/css/css-color/parsing/color-invalid-relative-color.html.ini
View File

@ -1,48 +0,0 @@
[color-invalid-relative-color.html]
[e.style['color'\] = "rgb(from rebeccapurple calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "hsl(from rebeccapurple calc(h + 1deg) s l)" should not set the property value]
expected: FAIL
[e.style['color'\] = "hwb(from rebeccapurple calc(h + 1deg) w b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "lab(from lab(.25 20 50) l calc(a + 1%) b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "oklab(from oklab(.25 20 50) l calc(a + 1%) b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "lch(from lch(.70 45 30) l c calc(h + 1deg))" should not set the property value]
expected: FAIL
[e.style['color'\] = "oklch(from oklch(.70 45 30) l c calc(h + 1deg))" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(srgb 0.7 0.5 0.3) srgb calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(srgb-linear 0.7 0.5 0.3) srgb-linear calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(a98-rgb 0.7 0.5 0.3) a98-rgb calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(rec2020 0.7 0.5 0.3) rec2020 calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(prophoto-rgb 0.7 0.5 0.3) prophoto-rgb calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(display-p3 0.7 0.5 0.3) display-p3 calc(r + 1%) g b)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(xyz 7 -20.5 100) xyz calc(x + 1%) y z)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(xyz-d50 7 -20.5 100) xyz-d50 calc(x + 1%) y z)" should not set the property value]
expected: FAIL
[e.style['color'\] = "color(from color(xyz-d65 7 -20.5 100) xyz-d65 calc(x + 1%) y z)" should not set the property value]
expected: FAIL