Handle semicolons at end of keyTimes and values

Source/core/svg/SVGAnimationElement.cpp

 /*
  * Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
  * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Cameron McCormack <cam@mcc.id.au>
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 34 matching lines @@
     , m_toPropertyValueType(RegularPropertyValue)
     , m_animationValid(false)
     , m_attributeType(AttributeTypeAuto)
     , m_hasInvalidCSSAttributeType(false)
     , m_calcMode(CalcModeLinear)
     , m_animationMode(NoAnimation)
 {
     UseCounter::count(document, UseCounter::SVGAnimationElement);
 }
 
-static void parseKeyTimes(const String& string, Vector<float>& result, bool verifyOrder)
+static bool parseValues(const String& value, Vector<String>& result)
+{
+    // Per the SMIL specification, leading and trailing white space,
+    // and white space before and after semicolon separators, is allowed and will be ignored.
+    // http://www.w3.org/TR/SVG11/animate.html#ValuesAttribute
+    result.clear();
+    Vector<String> parseList;
+    value.split(';', true, parseList);
+    unsigned last = parseList.size() - 1;
+    for (unsigned i = 0; i <= last; ++i) {
+        if (parseList[i].isEmpty()) {
+            // Tolerate trailing ';'
+            if (i < last)
+                goto fail;

[fs, 2014/10/06 09:02:33]

I think we could get rid of the goto [1] by doing something like:

result.swap(parseList);
return true;

and not append to result in the loop. (And just return false instead of 'goto
fail'.)

That's follow-up material though.

[1] Not that I necessarily dislike gotos...

+        } else {
+            parseList[i] = parseList[i].stripWhiteSpace();
+            result.append(parseList[i]);
+        }
+    }
+
+    return true;
+fail:
+    result.clear();
+    return false;
+}
+
+static bool parseKeyTimes(const String& string, Vector<float>& result, bool verifyOrder)
 {
     result.clear();
     Vector<String> parseList;
-    string.split(';', parseList);
+    string.split(';', true, parseList);
     for (unsigned n = 0; n < parseList.size(); ++n) {
         String timeString = parseList[n];
         bool ok;
         float time = timeString.toFloat(&ok);
         if (!ok || time < 0 || time > 1)
             goto fail;
         if (verifyOrder) {
             if (!n) {
                 if (time)
                     goto fail;
             } else if (time < result.last())
                 goto fail;
         }
         result.append(time);
     }
-    return;
+    return true;
 fail:
     result.clear();
+    return false;
 }
 
 template<typename CharType>
 static bool parseKeySplinesInternal(const String& string, Vector<UnitBezier>& result)
 {
     const CharType* ptr = string.getCharacters<CharType>();
     const CharType* end = ptr + string.length();
 
     skipOptionalSVGSpaces(ptr, end);
 
@@ skipping 61 matching lines @@
 }
 
 void SVGAnimationElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (!isSupportedAttribute(name)) {
         SVGSMILElement::parseAttribute(name, value);
         return;
     }
 
     if (name == SVGNames::valuesAttr) {
-        // Per the SMIL specification, leading and trailing white space,
-        // and white space before and after semicolon separators, is allowed and will be ignored.
-        // http://www.w3.org/TR/SVG11/animate.html#ValuesAttribute
-        value.string().split(';', m_values);
-        for (unsigned i = 0; i < m_values.size(); ++i)
-            m_values[i] = m_values[i].stripWhiteSpace();
-
+        if (!parseValues(value, m_values)) {
+            reportAttributeParsingError(ParsingAttributeFailedError, name, value);
+            return;
+        }
         updateAnimationMode();
         return;
     }
 
     if (name == SVGNames::keyTimesAttr) {
-        parseKeyTimes(value, m_keyTimes, true);
+        if (!parseKeyTimes(value, m_keyTimes, true))
+            reportAttributeParsingError(ParsingAttributeFailedError, name, value);
         return;
     }
 
     if (name == SVGNames::keyPointsAttr) {
         if (isSVGAnimateMotionElement(*this)) {
             // This is specified to be an animateMotion attribute only but it is simpler to put it here
             // where the other timing calculatations are.
-            parseKeyTimes(value, m_keyPoints, false);
+            if (!parseKeyTimes(value, m_keyPoints, false))
+                reportAttributeParsingError(ParsingAttributeFailedError, name, value);
         }
         return;
     }
 
     if (name == SVGNames::keySplinesAttr) {
         if (!parseKeySplines(value, m_keySplines))
             reportAttributeParsingError(ParsingAttributeFailedError, name, value);
         return;
     }
 
@@ skipping 492 matching lines @@
     SVGSMILElement::setAttributeName(attributeName);
     checkInvalidCSSAttributeType(targetElement());
 }
 
 void SVGAnimationElement::checkInvalidCSSAttributeType(SVGElement* target)
 {
     m_hasInvalidCSSAttributeType = target && hasValidAttributeName() && attributeType() == AttributeTypeCSS && !isTargetAttributeCSSProperty(target, attributeName());
 }
 
 }