Skip to content

GitLab

T
tangled
Commit 0965f08d authored by fs's avatar fs Committed by Commit bot
Browse files
Options

Extended error reporting for SVG path parsing 

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input type (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612

Review URL: https://codereview.chromium.org/1642463004

Cr-Commit-Position: refs/heads/master@{#371849}
parent 97690d86
Hide whitespace changes
Inline Side-by-side
Showing with 124 additions and 25 deletions
third_party/WebKit/LayoutTests/svg/custom/path-arc-invalid-expected.txt
View file @ 0965f08d
CONSOLE ERROR: line 6: Error: Invalid value for <path> attribute d="M0 0 A0 0 0 0 0 0 0 0 0 0"
CONSOLE ERROR: line 6: Error: <path> attribute d: Unexpected end of attribute. Expected arc flag ('0' or '1'), "… 0 0 0 0 0 0 0 0".
If this text is visible the malformed SVG path is handled correctly.
third_party/WebKit/LayoutTests/svg/parser/svg-path-error-reporting-expected.txt 0 → 100644
View file @ 0965f08d
CONSOLE ERROR: line 7: Error: <path> attribute d: Expected moveto path command ('M' or 'm'), " 10,10 L 20,20 30…".
CONSOLE ERROR: line 8: Error: <path> attribute d: Expected number, "…0 C 20,20 30 40 fiftyfive twenty…".
CONSOLE ERROR: line 9: Error: <path> attribute d: Expected arc flag ('0' or '1'), "…10,10 A 20,20 0 large,yes 40 50".
CONSOLE ERROR: line 10: Error: <path> attribute d: Expected path command, "M 10,10 E 20,20 z".
third_party/WebKit/LayoutTests/svg/parser/svg-path-error-reporting.html 0 → 100644
View file @ 0965f08d
<!DOCTYPE html>
<script>
if (window.testRunner)
testRunner.dumpAsText();
</script>
<svg>
<path d=" 10,10 L 20,20 30 40 50 60 70 80"/>
<path d="M 10,10 C 20,20 30 40 fiftyfive twentyseven z"/>
<path d="M 10,10 A 20,20 0 large,yes 40 50"/>
<path d="M 10,10 E 20,20 z"/>
</svg>
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
View file @ 0965f08d
......@@ -1927,7 +1927,8 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumePath(CSSParserTokenRange& range)
String pathString = functionArgs.consumeIncludingWhitespace().value();
RefPtr<SVGPathByteStream> byteStream = SVGPathByteStream::create();
if (!buildByteStreamFromString(pathString, *byteStream) || !functionArgs.atEnd())
if (buildByteStreamFromString(pathString, *byteStream) != SVGParseStatus::NoError
|| !functionArgs.atEnd())
return nullptr;
range = functionRange;
......
third_party/WebKit/Source/core/svg/SVGParserUtilities.cpp
View file @ 0965f08d
......@@ -161,7 +161,7 @@ bool genericParseArcFlag(const CharType*& ptr, const CharType* end, bool& flag)
{
if (ptr >= end)
return false;
const CharType flagChar = *ptr++;
const CharType flagChar = *ptr;
if (flagChar == '0')
flag = false;
else if (flagChar == '1')
......@@ -169,6 +169,7 @@ bool genericParseArcFlag(const CharType*& ptr, const CharType* end, bool& flag)
else
return false;
ptr++;
skipOptionalSVGSpacesOrDelimiter(ptr, end);
return true;
......
third_party/WebKit/Source/core/svg/SVGParsingError.cpp
View file @ 0965f08d
......@@ -28,14 +28,20 @@ std::pair<const char*, const char*> messageForStatus(SVGParseStatus status)
switch (status) {
case SVGParseStatus::TrailingGarbage:
return std::make_pair("Trailing garbage, ", ".");
case SVGParseStatus::ExpectedArcFlag:
return std::make_pair("Expected arc flag ('0' or '1'), ", ".");
case SVGParseStatus::ExpectedBoolean:
return std::make_pair("Expected 'true' or 'false', ", ".");
case SVGParseStatus::ExpectedEnumeration:
return std::make_pair("Unrecognized enumerated value, ", ".");
case SVGParseStatus::ExpectedLength:
return std::make_pair("Expected length, ", ".");
case SVGParseStatus::ExpectedMoveToCommand:
return std::make_pair("Expected moveto path command ('M' or 'm'), ", ".");
case SVGParseStatus::ExpectedNumber:
return std::make_pair("Expected number, ", ".");
case SVGParseStatus::ExpectedPathCommand:
return std::make_pair("Expected path command, ", ".");
case SVGParseStatus::NegativeValue:
return std::make_pair("A negative value is not valid. (", ")");
case SVGParseStatus::ParsingFailed:
......
third_party/WebKit/Source/core/svg/SVGParsingError.h
View file @ 0965f08d
......@@ -39,10 +39,13 @@ enum class SVGParseStatus {
// Syntax errors
TrailingGarbage,
ExpectedArcFlag,
ExpectedBoolean,
ExpectedEnumeration,
ExpectedLength,
ExpectedMoveToCommand,
ExpectedNumber,
ExpectedPathCommand,
// Semantic errors
NegativeValue,
......
third_party/WebKit/Source/core/svg/SVGPath.cpp
View file @ 0965f08d
......@@ -95,10 +95,8 @@ PassRefPtrWillBeRawPtr<SVGPath> SVGPath::clone() const
SVGParsingError SVGPath::setValueAsString(const String& string)
{
SVGParsingError parseStatus;
RefPtr<SVGPathByteStream> byteStream = SVGPathByteStream::create();
if (!buildByteStreamFromString(string, *byteStream))
parseStatus = SVGParseStatus::ParsingFailed;
SVGParsingError parseStatus = buildByteStreamFromString(string, *byteStream);
m_pathValue = CSSPathValue::create(byteStream.release());
return parseStatus;
}
......
third_party/WebKit/Source/core/svg/SVGPathParserTest.cpp
View file @ 0965f08d
......@@ -141,6 +141,49 @@ TEST(SVGPathParserTest, Simple)
#undef MALFORMED
#undef VALID
SVGParsingError parsePathWithError(const char* input)
{
String inputString(input);
SVGPathStringSource source(inputString);
SVGPathStringBuilder builder;
SVGPathParser parser(&source, &builder);
parser.parsePathDataFromSource();
return source.parseError();
}
#define EXPECT_ERROR(input, expectedLocus, expectedError) \
{ \
SVGParsingError error = parsePathWithError(input); \
EXPECT_EQ(expectedError, error.status()); \
EXPECT_TRUE(error.hasLocus()); \
EXPECT_EQ(expectedLocus, error.locus()); \
}
TEST(SVGPathParserTest, ErrorReporting)
{
// Missing initial moveto.
EXPECT_ERROR(" 10 10", 1u, SVGParseStatus::ExpectedMoveToCommand);
EXPECT_ERROR("L 10 10", 0u, SVGParseStatus::ExpectedMoveToCommand);
// Invalid command letter.
EXPECT_ERROR("M 10 10 #", 8u, SVGParseStatus::ExpectedPathCommand);
EXPECT_ERROR("M 10 10 E 100 100", 8u, SVGParseStatus::ExpectedPathCommand);
// Invalid number.
EXPECT_ERROR("M 10 10 L100 ", 13u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M 10 10 L100 #", 13u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M 10 10 L100#100", 12u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M0,0 A#,10 0 0,0 20,20", 6u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M0,0 A10,# 0 0,0 20,20", 9u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M0,0 A10,10 # 0,0 20,20", 12u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M0,0 A10,10 0 0,0 #,20", 18u, SVGParseStatus::ExpectedNumber);
EXPECT_ERROR("M0,0 A10,10 0 0,0 20,#", 21u, SVGParseStatus::ExpectedNumber);
// Invalid arc-flag.
EXPECT_ERROR("M0,0 A10,10 0 #,0 20,20", 14u, SVGParseStatus::ExpectedArcFlag);
EXPECT_ERROR("M0,0 A10,10 0 0,# 20,20", 16u, SVGParseStatus::ExpectedArcFlag);
EXPECT_ERROR("M0,0 A10,10 0 0,2 20,20", 16u, SVGParseStatus::ExpectedArcFlag);
}
#undef EXPECT_ERROR
} // namespace
} // namespace blink
third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp
View file @ 0965f08d
......@@ -26,10 +26,9 @@
namespace blink {
SVGPathStringSource::SVGPathStringSource(const String& string)
: m_string(string)
, m_is8BitSource(string.is8Bit())
, m_seenError(false)
: m_is8BitSource(string.is8Bit())
, m_previousCommand(PathSegUnknown)
, m_string(string)
{
ASSERT(!string.isNull());
......@@ -124,23 +123,39 @@ static bool nextCommandHelper(unsigned lookahead, SVGPathSegType previousCommand
return false;
}
void SVGPathStringSource::setErrorMark(SVGParseStatus status)
{
if (m_error.status() != SVGParseStatus::NoError)
return;
size_t locus = m_is8BitSource
? m_current.m_character8 - m_string.characters8()
: m_current.m_character16 - m_string.characters16();
m_error = SVGParsingError(status, locus);
}
float SVGPathStringSource::parseNumberWithError()
{
float numberValue = 0;
bool error;
if (m_is8BitSource)
m_seenError |= !parseNumber(m_current.m_character8, m_end.m_character8, numberValue);
error = !parseNumber(m_current.m_character8, m_end.m_character8, numberValue);
else
m_seenError |= !parseNumber(m_current.m_character16, m_end.m_character16, numberValue);
error = !parseNumber(m_current.m_character16, m_end.m_character16, numberValue);
if (UNLIKELY(error))
setErrorMark(SVGParseStatus::ExpectedNumber);
return numberValue;
}
bool SVGPathStringSource::parseArcFlagWithError()
{
bool flagValue = false;
bool error;
if (m_is8BitSource)
m_seenError |= !parseArcFlag(m_current.m_character8, m_end.m_character8, flagValue);
error = !parseArcFlag(m_current.m_character8, m_end.m_character8, flagValue);
else
m_seenError |= !parseArcFlag(m_current.m_character16, m_end.m_character16, flagValue);
error = !parseArcFlag(m_current.m_character16, m_end.m_character16, flagValue);
if (UNLIKELY(error))
setErrorMark(SVGParseStatus::ExpectedArcFlag);
return flagValue;
}
......@@ -149,7 +164,14 @@ SVGPathSegType SVGPathStringSource::peekSegmentType()
ASSERT(hasMoreData());
// This won't work in all cases because of the state required to "detect" implicit commands.
unsigned lookahead = m_is8BitSource ? *m_current.m_character8 : *m_current.m_character16;
return parseSVGSegmentTypeHelper(lookahead);
SVGPathSegType segmentType = parseSVGSegmentTypeHelper(lookahead);
// Here we assume that this is only called via SVGPathParser::initialCommandIsMoveTo.
// TODO(fs): It ought to be possible to refactor away this entirely, and
// just handle this in parseSegment (which we sort of do already...) The
// only user is the method mentioned above.
if (segmentType != PathSegMoveToAbs && segmentType != PathSegMoveToRel)
setErrorMark(SVGParseStatus::ExpectedMoveToCommand);
return segmentType;
}
PathSegmentData SVGPathStringSource::parseSegment()
......@@ -160,10 +182,14 @@ PathSegmentData SVGPathStringSource::parseSegment()
SVGPathSegType command = parseSVGSegmentTypeHelper(lookahead);
if (command == PathSegUnknown) {
// Possibly an implicit command. Not allowed if this is the first command.
if (m_previousCommand == PathSegUnknown)
if (m_previousCommand == PathSegUnknown) {
setErrorMark(SVGParseStatus::ExpectedMoveToCommand);
return segment;
if (!nextCommandHelper(lookahead, m_previousCommand, command))
}
if (!nextCommandHelper(lookahead, m_previousCommand, command)) {
setErrorMark(SVGParseStatus::ExpectedPathCommand);
return segment;
}
} else {
// Valid explicit command.
if (m_is8BitSource)
......@@ -174,7 +200,7 @@ PathSegmentData SVGPathStringSource::parseSegment()
segment.command = m_previousCommand = command;
ASSERT(!m_seenError);
ASSERT(m_error.status() == SVGParseStatus::NoError);
switch (segment.command) {
case PathSegCurveToCubicRel:
......@@ -228,7 +254,7 @@ PathSegmentData SVGPathStringSource::parseSegment()
ASSERT_NOT_REACHED();
}
if (UNLIKELY(m_seenError))
if (UNLIKELY(m_error.status() != SVGParseStatus::NoError))
segment.command = PathSegUnknown;
return segment;
}
......
third_party/WebKit/Source/core/svg/SVGPathStringSource.h
View file @ 0965f08d
......@@ -22,6 +22,7 @@
#define SVGPathStringSource_h
#include "core/CoreExport.h"
#include "core/svg/SVGParsingError.h"
#include "core/svg/SVGPathSource.h"
#include "wtf/text/WTFString.h"
......@@ -31,6 +32,8 @@ class CORE_EXPORT SVGPathStringSource final : public SVGPathSource {
public:
explicit SVGPathStringSource(const String&);
SVGParsingError parseError() const { return m_error; }
private:
bool hasMoreData() const override;
SVGPathSegType peekSegmentType() override;
......@@ -39,10 +42,9 @@ private:
void eatWhitespace();
float parseNumberWithError();
bool parseArcFlagWithError();
void setErrorMark(SVGParseStatus);
String m_string;
bool m_is8BitSource;
bool m_seenError;
union {
const LChar* m_character8;
......@@ -54,6 +56,8 @@ private:
} m_end;
SVGPathSegType m_previousCommand;
SVGParsingError m_error;
String m_string;
};
} // namespace blink
......
third_party/WebKit/Source/core/svg/SVGPathUtilities.cpp
View file @ 0965f08d
......@@ -62,11 +62,11 @@ String buildStringFromByteStream(const SVGPathByteStream& stream)
return builder.result();
}
bool buildByteStreamFromString(const String& d, SVGPathByteStream& result)
SVGParsingError buildByteStreamFromString(const String& d, SVGPathByteStream& result)
{
result.clear();
if (d.isEmpty())
return true;
return SVGParseStatus::NoError;
// The string length is typically a minor overestimate of eventual byte stream size, so it avoids us a lot of reallocs.
result.reserveInitialCapacity(d.length());
......@@ -74,9 +74,9 @@ bool buildByteStreamFromString(const String& d, SVGPathByteStream& result)
SVGPathByteStreamBuilder builder(result);
SVGPathStringSource source(d);
SVGPathParser parser(&source, &builder);
bool ok = parser.parsePathDataFromSource();
parser.parsePathDataFromSource();
result.shrinkToFit();
return ok;
return source.parseError();
}
} // namespace blink
third_party/WebKit/Source/core/svg/SVGPathUtilities.h
View file @ 0965f08d
......@@ -21,6 +21,7 @@
#define SVGPathUtilities_h
#include "core/CoreExport.h"
#include "core/svg/SVGParsingError.h"
#include "wtf/text/WTFString.h"
namespace blink {
......@@ -33,7 +34,7 @@ bool CORE_EXPORT buildPathFromString(const String&, Path&);
bool buildPathFromByteStream(const SVGPathByteStream&, Path&);
// String -> SVGPathByteStream
bool buildByteStreamFromString(const String&, SVGPathByteStream&);
SVGParsingError buildByteStreamFromString(const String&, SVGPathByteStream&);
// SVGPathByteStream -> String
String buildStringFromByteStream(const SVGPathByteStream&);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment