style: Reject empty svg path string for basic shape path function

Per CSS shape spec, the empty path string is invalid. However, for SVG
d property, the EBNF allows the path data string in the d property to be
empty. We just disable the rendering of the path.

Note: only offset-path and clip-path are affected.

Differential Revision: https://phabricator.services.mozilla.com/D178123

components/style/values/specified/basic_shape.rs

@@ -15,8 +15,7 @@ use crate::values::specified::border::BorderRadius;
 use crate::values::specified::image::Image;
 use crate::values::specified::position::{HorizontalPosition, Position, VerticalPosition};
 use crate::values::specified::url::SpecifiedUrl;
-use crate::values::specified::SVGPathData;
+use crate::values::specified::{LengthPercentage, NonNegativeLengthPercentage, SVGPathData};
-use crate::values::specified::{LengthPercentage, NonNegativeLengthPercentage};
 use crate::Zero;
 use cssparser::Parser;
 use style_traits::{ParseError, StyleParseErrorKind};
@@ -294,20 +293,21 @@ impl Polygon {
 
 impl Parse for Path {
     fn parse<'i, 't>(
-        context: &ParserContext,
+        _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
         input.expect_function_matching("path")?;
-        input.parse_nested_block(|i| Self::parse_function_arguments(context, i))
+        input.parse_nested_block(Self::parse_function_arguments)
     }
 }
 
 impl Path {
     /// Parse the inner arguments of a `path` function.
     fn parse_function_arguments<'i, 't>(
-        context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
+        use crate::values::specified::svg_path::AllowEmpty;
+
         let fill = input
             .try_parse(|i| -> Result<_, ParseError> {
                 let fill = FillRule::parse(i)?;
@@ -315,7 +315,7 @@ impl Path {
                 Ok(fill)
             })
             .unwrap_or_default();
-        let path = SVGPathData::parse(context, input)?;
+        let path = SVGPathData::parse(input, AllowEmpty::No)?;
         Ok(Path { fill, path })
     }
 }

components/style/values/specified/motion.rs

@@ -78,6 +78,8 @@ impl Parse for OffsetPath {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
+        use crate::values::specified::svg_path::AllowEmpty;
+
         // Parse none.
         if input.try_parse(|i| i.expect_ident_matching("none")).is_ok() {
             return Ok(OffsetPath::none());
@@ -90,8 +92,8 @@ impl Parse for OffsetPath {
             match_ignore_ascii_case! { &function,
                 // Bug 1186329: Implement the parser for <basic-shape>, <geometry-box>,
                 // and <url>.
-                "path" => SVGPathData::parse(context, i).map(GenericOffsetPath::Path),
+                "path" => SVGPathData::parse(i, AllowEmpty::No).map(OffsetPath::Path),
-                "ray" => RayFunction::parse(context, i).map(GenericOffsetPath::Ray),
+                "ray" => RayFunction::parse(context, i).map(OffsetPath::Ray),
                 _ => {
                     Err(location.new_custom_error(
                         StyleParseErrorKind::UnexpectedFunction(function.clone())

components/style/values/specified/svg.rs

@@ -385,7 +385,7 @@ impl Parse for DProperty {
 
         // Parse possible functions.
         input.expect_function_matching("path")?;
-        let path_data = input.parse_nested_block(|i| SVGPathData::parse(context, i))?;
+        let path_data = input.parse_nested_block(|i| Parse::parse(context, i))?;
         Ok(DProperty::Path(path_data))
     }
 }

components/style/values/specified/svg_path.rs

@@ -5,7 +5,7 @@
 //! Specified types for SVG Path.
 
 use crate::parser::{Parse, ParserContext};
-use crate::values::animated::{Animate, Procedure, ToAnimatedZero, lists};
+use crate::values::animated::{lists, Animate, Procedure, ToAnimatedZero};
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::CSSFloat;
 use cssparser::Parser;
@@ -15,6 +15,14 @@ use std::slice;
 use style_traits::values::SequenceWriter;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
+/// Whether to allow empty string in the parser.
+#[derive(Clone, Debug, Eq, PartialEq)]
+#[allow(missing_docs)]
+pub enum AllowEmpty {
+    Yes,
+    No,
+}
+
 /// The SVG path data.
 ///
 /// https://www.w3.org/TR/SVG11/paths.html#PathData
@@ -160,6 +168,41 @@ impl SVGPathData {
 
         Ok(SVGPathData(crate::ArcSlice::from_iter(result.into_iter())))
     }
+
+    /// Parse this SVG path string with the argument that indicates whether we should allow the
+    /// empty string.
+    // We cannot use cssparser::Parser to parse a SVG path string because the spec wants to make
+    // the SVG path string as compact as possible. (i.e. The whitespaces may be dropped.)
+    // e.g. "M100 200L100 200" is a valid SVG path string. If we use tokenizer, the first ident
+    // is "M100", instead of "M", and this is not correct. Therefore, we use a Peekable
+    // str::Char iterator to check each character.
+    pub fn parse<'i, 't>(
+        input: &mut Parser<'i, 't>,
+        allow_empty: AllowEmpty,
+    ) -> Result<Self, ParseError<'i>> {
+        let location = input.current_source_location();
+        let path_string = input.expect_string()?.as_ref();
+
+        // Parse the svg path string as multiple sub-paths.
+        let mut path_parser = PathParser::new(path_string);
+        while skip_wsp(&mut path_parser.chars) {
+            if path_parser.parse_subpath().is_err() {
+                return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+            }
+        }
+
+        // The css-shapes-1 says a path data string that does conform but defines an empty path is
+        // invalid and causes the entire path() to be invalid, so we use the argement to decide
+        // whether we should allow the empty string.
+        // https://drafts.csswg.org/css-shapes-1/#typedef-basic-shape
+        if matches!(allow_empty, AllowEmpty::No) && path_parser.path.is_empty() {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+        }
+
+        Ok(SVGPathData(crate::ArcSlice::from_iter(
+            path_parser.path.into_iter(),
+        )))
+    }
 }
 
 impl ToCss for SVGPathData {
@@ -180,29 +223,14 @@ impl ToCss for SVGPathData {
 }
 
 impl Parse for SVGPathData {
-    // We cannot use cssparser::Parser to parse a SVG path string because the spec wants to make
-    // the SVG path string as compact as possible. (i.e. The whitespaces may be dropped.)
-    // e.g. "M100 200L100 200" is a valid SVG path string. If we use tokenizer, the first ident
-    // is "M100", instead of "M", and this is not correct. Therefore, we use a Peekable
-    // str::Char iterator to check each character.
     fn parse<'i, 't>(
         _context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        let location = input.current_source_location();
+        // Note that the EBNF allows the path data string in the d property to be empty, so we
-        let path_string = input.expect_string()?.as_ref();
+        // don't reject empty SVG path data.
-
+        // https://svgwg.org/svg2-draft/single-page.html#paths-PathDataBNF
-        // Parse the svg path string as multiple sub-paths.
+        SVGPathData::parse(input, AllowEmpty::Yes)
-        let mut path_parser = PathParser::new(path_string);
-        while skip_wsp(&mut path_parser.chars) {
-            if path_parser.parse_subpath().is_err() {
-                return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-            }
-        }
-
-        Ok(SVGPathData(crate::ArcSlice::from_iter(
-            path_parser.path.into_iter(),
-        )))
     }
 }