Skip to content

Commit

Permalink
Report an error for invalid CSS color values (bug 1381143).
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdm
jdm committed Jul 31, 2017
1 parent cf5602e commit 4b73635
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 82 deletions.
11 changes: 8 additions & 3 deletions components/style/gecko/generated/bindings.rs
View file
Expand Up @@ -2881,10 +2881,15 @@ extern "C" {
param:
*const ::std::os::raw::c_char,
paramLen: u32,
prefix:
*const ::std::os::raw::c_char,
prefixParam:
*const ::std::os::raw::c_char,
prefixParamLen: u32,
suffix:
*const ::std::os::raw::c_char,
source:
*const ::std::os::raw::c_char,
sourceLen: u32, lineNumber: u32,
colNumber: u32, aURI: *mut nsIURI,
followup:
*const ::std::os::raw::c_char);
colNumber: u32);
}
25 changes: 15 additions & 10 deletions components/style/properties/properties.mako.rs
View file
Expand Up @@ -38,7 +38,7 @@ use selectors::parser::SelectorParseError;
#[cfg(feature = "servo")] use servo_config::prefs::PREFS;
use shared_lock::StylesheetGuards;
use style_traits::{PARSING_MODE_DEFAULT, HasViewportPercentage, ToCss, ParseError};
use style_traits::{PropertyDeclarationParseError, StyleParseError};
use style_traits::{PropertyDeclarationParseError, StyleParseError, ValueParseError};
use stylesheets::{CssRuleType, MallocSizeOf, MallocSizeOfFn, Origin, UrlExtraData};
#[cfg(feature = "servo")] use values::Either;
use values::generics::text::LineHeight;
Expand Down Expand Up @@ -1489,7 +1489,8 @@ impl PropertyDeclaration {
Ok(keyword) => DeclaredValueOwned::CSSWideKeyword(keyword),
Err(_) => match ::custom_properties::SpecifiedValue::parse(context, input) {
Ok(value) => DeclaredValueOwned::Value(value),
Err(_) => return Err(PropertyDeclarationParseError::InvalidValue(name.to_string().into())),
Err(e) => return Err(PropertyDeclarationParseError::InvalidValue(name.to_string().into(),
ValueParseError::from_parse_error(e))),
}
};
declarations.push(PropertyDeclaration::Custom(name, value));
Expand All @@ -1502,13 +1503,14 @@ impl PropertyDeclaration {
input.look_for_var_functions();
let start = input.position();
input.parse_entirely(|input| id.parse_value(context, input))
.or_else(|_| {
.or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if input.seen_var_functions() {
input.reset(start);
let (first_token_type, css) =
::custom_properties::parse_non_custom_with_var(input).map_err(|_| {
PropertyDeclarationParseError::InvalidValue(id.name().into())
::custom_properties::parse_non_custom_with_var(input).map_err(|e| {
PropertyDeclarationParseError::InvalidValue(id.name().into(),
ValueParseError::from_parse_error(e))
})?;
Ok(PropertyDeclaration::WithVariables(id, Arc::new(UnparsedValue {
css: css.into_owned(),
Expand All @@ -1517,7 +1519,8 @@ impl PropertyDeclaration {
from_shorthand: None,
})))
} else {
Err(PropertyDeclarationParseError::InvalidValue(id.name().into()))
Err(PropertyDeclarationParseError::InvalidValue(id.name().into(),
ValueParseError::from_parse_error(err)))
}
})
}).map(|declaration| {
Expand All @@ -1539,13 +1542,14 @@ impl PropertyDeclaration {
let start = input.position();
// Not using parse_entirely here: each ${shorthand.ident}::parse_into function
// needs to do so *before* pushing to `declarations`.
id.parse_into(declarations, context, input).or_else(|_| {
id.parse_into(declarations, context, input).or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if input.seen_var_functions() {
input.reset(start);
let (first_token_type, css) =
::custom_properties::parse_non_custom_with_var(input).map_err(|_| {
PropertyDeclarationParseError::InvalidValue(id.name().into())
::custom_properties::parse_non_custom_with_var(input).map_err(|e| {
PropertyDeclarationParseError::InvalidValue(id.name().into(),
ValueParseError::from_parse_error(e))
})?;
let unparsed = Arc::new(UnparsedValue {
css: css.into_owned(),
Expand All @@ -1564,7 +1568,8 @@ impl PropertyDeclaration {
}
Ok(())
} else {
Err(PropertyDeclarationParseError::InvalidValue(id.name().into()))
Err(PropertyDeclarationParseError::InvalidValue(id.name().into(),
ValueParseError::from_parse_error(err)))
}
})
}
Expand Down
48 changes: 27 additions & 21 deletions components/style/values/specified/color.rs
View file
Expand Up @@ -13,7 +13,7 @@ use parser::{ParserContext, Parse};
use properties::longhands::color::SystemColor;
use std::fmt;
use std::io::Write;
use style_traits::{ToCss, ParseError, StyleParseError};
use style_traits::{ToCss, ParseError, StyleParseError, ValueParseError};
use super::AllowQuirks;
use values::computed::{Color as ComputedColor, Context, ToComputedValue};

Expand Down Expand Up @@ -76,24 +76,28 @@ impl Parse for Color {
_ => None,
};
input.reset(start_position);
if let Ok(value) = input.try(CSSParserColor::parse) {
Ok(match value {
CSSParserColor::CurrentColor => Color::CurrentColor,
CSSParserColor::RGBA(rgba) => Color::Numeric {
parsed: rgba,
authored: authored,
},
})
} else {
#[cfg(feature = "gecko")] {
if let Ok(system) = input.try(SystemColor::parse) {
Ok(Color::System(system))
} else {
gecko::SpecialColorKeyword::parse(input).map(Color::Special)
match input.try(CSSParserColor::parse) {
Ok(value) =>
Ok(match value {
CSSParserColor::CurrentColor => Color::CurrentColor,
CSSParserColor::RGBA(rgba) => Color::Numeric {
parsed: rgba,
authored: authored,
},
}),
Err(e) => {
#[cfg(feature = "gecko")] {
if let Ok(system) = input.try(SystemColor::parse) {
return Ok(Color::System(system));
} else if let Ok(c) = gecko::SpecialColorKeyword::parse(input) {
return Ok(Color::Special(c));
}
}
match e {
BasicParseError::UnexpectedToken(t) =>
Err(StyleParseError::ValueError(ValueParseError::InvalidColor(t)).into()),
e => Err(e.into())
}
}
#[cfg(not(feature = "gecko"))] {
Err(StyleParseError::UnspecifiedError.into())
}
}
}
Expand Down Expand Up @@ -161,11 +165,13 @@ impl Color {
input: &mut Parser<'i, 't>,
allow_quirks: AllowQuirks)
-> Result<Self, ParseError<'i>> {
input.try(|i| Self::parse(context, i)).or_else(|_| {
input.try(|i| Self::parse(context, i)).or_else(|e| {
if !allow_quirks.allowed(context.quirks_mode) {
return Err(StyleParseError::UnspecifiedError.into());
return Err(e);
}
Color::parse_quirky_color(input).map(|rgba| Color::rgba(rgba))
Color::parse_quirky_color(input)
.map(|rgba| Color::rgba(rgba))
.map_err(|_| e)
})
}

Expand Down
25 changes: 23 additions & 2 deletions components/style_traits/lib.rs
View file
Expand Up @@ -124,10 +124,31 @@ pub enum StyleParseError<'i> {
UnspecifiedError,
/// An unexpected token was found within a namespace rule.
UnexpectedTokenWithinNamespace(Token<'i>),
/// An error was encountered while parsing a property value.
ValueError(ValueParseError<'i>),
}

/// Specific errors that can be encountered while parsing property values.
#[derive(Clone, Debug, PartialEq)]
pub enum ValueParseError<'i> {
/// An invalid token was encountered while parsing a color value.
InvalidColor(Token<'i>),
}

impl<'i> ValueParseError<'i> {
/// Attempt to extract a ValueParseError value from a ParseError.
pub fn from_parse_error(this: ParseError<'i>) -> Option<ValueParseError<'i>> {
match this {
cssparser::ParseError::Custom(
SelectorParseError::Custom(
StyleParseError::ValueError(e))) => Some(e),
_ => None,
}
}
}

/// The result of parsing a property declaration.
#[derive(Eq, PartialEq, Clone, Debug)]
#[derive(PartialEq, Clone, Debug)]
pub enum PropertyDeclarationParseError<'i> {
/// The property declaration was for an unknown property.
UnknownProperty(CowRcStr<'i>),
Expand All @@ -136,7 +157,7 @@ pub enum PropertyDeclarationParseError<'i> {
/// The property declaration was for a disabled experimental property.
ExperimentalProperty,
/// The property declaration contained an invalid value.
InvalidValue(CowRcStr<'i>),
InvalidValue(CowRcStr<'i>, Option<ValueParseError<'i>>),
/// The declaration contained an animation property, and we were parsing
/// this as a keyframe block (so that property should be ignored).
///
Expand Down
125 changes: 80 additions & 45 deletions ports/geckolib/error_reporter.rs
View file
Expand Up @@ -18,7 +18,7 @@ use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter;
use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData;
use style::gecko_bindings::sugar::refptr::RefPtr;
use style::stylesheets::UrlExtraData;
use style_traits::{ParseError, StyleParseError, PropertyDeclarationParseError};
use style_traits::{ParseError, StyleParseError, PropertyDeclarationParseError, ValueParseError};

/// Wrapper around an instance of Gecko's CSS error reporter.
pub struct ErrorReporter(*mut GeckoErrorReporter);
Expand Down Expand Up @@ -194,8 +194,59 @@ enum Action {

trait ErrorHelpers<'a> {
fn error_data(self) -> (CowRcStr<'a>, ParseError<'a>);
fn error_param(self) -> ErrorString<'a>;
fn to_gecko_message(&self) -> (&'static [u8], Action);
fn error_params(self) -> (ErrorString<'a>, Option<ErrorString<'a>>);
fn to_gecko_message(&self) -> (Option<&'static [u8]>, &'static [u8], Action);
}

fn extract_error_param<'a>(err: ParseError<'a>) -> Option<ErrorString<'a>> {
Some(match err {
CssParseError::Basic(BasicParseError::UnexpectedToken(t)) =>
ErrorString::UnexpectedToken(t),

CssParseError::Basic(BasicParseError::AtRuleInvalid(i)) =>
ErrorString::Snippet(format!("@{}", escape_css_ident(&i)).into()),

CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::InvalidValue(property, None)))) =>
ErrorString::Snippet(property),

CssParseError::Custom(SelectorParseError::UnexpectedIdent(ident)) =>
ErrorString::Ident(ident),

CssParseError::Custom(SelectorParseError::ExpectedNamespace(namespace)) =>
ErrorString::Ident(namespace),

CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::UnknownProperty(property)))) =>
ErrorString::Ident(property),

CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::UnexpectedTokenWithinNamespace(token))) =>
ErrorString::UnexpectedToken(token),

_ => return None,
})
}

fn extract_value_error_param<'a>(err: ValueParseError<'a>) -> ErrorString<'a> {
match err {
ValueParseError::InvalidColor(t) => ErrorString::UnexpectedToken(t),
}
}

/// If an error parameter is present in the given error, return it. Additionally return
/// a second parameter if it exists, for use in the prefix for the eventual error message.
fn extract_error_params<'a>(err: ParseError<'a>) -> Option<(ErrorString<'a>, Option<ErrorString<'a>>)> {
match err {
CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::InvalidValue(property, Some(e))))) =>
Some((ErrorString::Snippet(property.into()), Some(extract_value_error_param(e)))),

err => extract_error_param(err).map(|e| (e, None)),
}
}

impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
Expand All @@ -222,40 +273,13 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
}
}

fn error_param(self) -> ErrorString<'a> {
match self.error_data() {
(_, CssParseError::Basic(BasicParseError::UnexpectedToken(t))) =>
ErrorString::UnexpectedToken(t),

(_, CssParseError::Basic(BasicParseError::AtRuleInvalid(i))) =>
ErrorString::Snippet(format!("@{}", escape_css_ident(&i)).into()),

(_, CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::InvalidValue(property))))) =>
ErrorString::Snippet(property),

(_, CssParseError::Custom(SelectorParseError::UnexpectedIdent(ident))) =>
ErrorString::Ident(ident),

(_, CssParseError::Custom(SelectorParseError::ExpectedNamespace(namespace))) =>
ErrorString::Ident(namespace),

(_, CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::UnknownProperty(property))))) =>
ErrorString::Ident(property),

(_, CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::UnexpectedTokenWithinNamespace(token)))) =>
ErrorString::UnexpectedToken(token),

(s, _) => ErrorString::Snippet(s)
}
fn error_params(self) -> (ErrorString<'a>, Option<ErrorString<'a>>) {
let (s, error) = self.error_data();
extract_error_params(error).unwrap_or((ErrorString::Snippet(s), None))
}

fn to_gecko_message(&self) -> (&'static [u8], Action) {
match *self {
fn to_gecko_message(&self) -> (Option<&'static [u8]>, &'static [u8], Action) {
let (msg, action): (&[u8], Action) = match *self {
ContextualParseError::UnsupportedPropertyDeclaration(
_, CssParseError::Basic(BasicParseError::UnexpectedToken(_))) |
ContextualParseError::UnsupportedPropertyDeclaration(
Expand All @@ -264,8 +288,13 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::UnsupportedPropertyDeclaration(
_, CssParseError::Custom(SelectorParseError::Custom(
StyleParseError::PropertyDeclaration(
PropertyDeclarationParseError::InvalidValue(_))))) =>
(b"PEValueParsingError\0", Action::Drop),
PropertyDeclarationParseError::InvalidValue(_, ref err))))) => {
let prefix = match *err {
Some(ValueParseError::InvalidColor(_)) => Some(&b"PEColorNotColor\0"[..]),
_ => None,
};
return (prefix, b"PEValueParsingError\0", Action::Drop);
}
ContextualParseError::UnsupportedPropertyDeclaration(..) =>
(b"PEUnknownProperty\0", Action::Drop),
ContextualParseError::UnsupportedFontFaceDescriptor(..) =>
Expand Down Expand Up @@ -295,7 +324,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) |
ContextualParseError::InvalidFontFeatureValuesRule(..) =>
(b"PEUnknownAtRule\0", Action::Skip),
}
};
(None, msg, action)
}
}

Expand All @@ -304,31 +334,36 @@ impl ParseErrorReporter for ErrorReporter {
input: &mut Parser,
position: SourcePosition,
error: ContextualParseError<'a>,
url: &UrlExtraData,
_url: &UrlExtraData,
line_number_offset: u64) {
let location = input.source_location(position);
let line_number = location.line + line_number_offset as u32;

let (name, action) = error.to_gecko_message();
let followup = match action {
let (pre, name, action) = error.to_gecko_message();
let suffix = match action {
Action::Nothing => ptr::null(),
Action::Skip => b"PEDeclSkipped\0".as_ptr(),
Action::Drop => b"PEDeclDropped\0".as_ptr(),
};
let param = error.error_param().into_str();
let (param, pre_param) = error.error_params();
let param = param.into_str();
let pre_param = pre_param.map(|p| p.into_str());
let pre_param_ptr = pre_param.as_ref().map_or(ptr::null(), |p| p.as_ptr());
// The CSS source text is unused and will be removed in bug 1381188.
let source = "";
unsafe {
Gecko_ReportUnexpectedCSSError(self.0,
name.as_ptr() as *const _,
param.as_ptr() as *const _,
param.len() as u32,
pre.map_or(ptr::null(), |p| p.as_ptr()) as *const _,
pre_param_ptr as *const _,
pre_param.as_ref().map_or(0, |p| p.len()) as u32,
suffix as *const _,
source.as_ptr() as *const _,
source.len() as u32,
line_number as u32,
location.column as u32,
url.mBaseURI.raw::<nsIURI>(),
followup as *const _);
location.column as u32);
}
}
}

0 comments on commit 4b73635

Please sign in to comment.