Bug 142872 – [Content Extensions] Test regular expression parse failures

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Alex Christensen]

I made things I think can never happen into assertions.  I had some questions, though.  They're in the first patch.

[Alex Christensen]

Created attachment 249048 [details]
Patch

[Benjamin Poulain]

Comment on attachment 249048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249048&action=review

> Source/WebCore/contentextensions/URLFilterParser.cpp:481
> +        ASSERT(m_openGroups.isEmpty());
> +        ASSERT(m_subtreeStart != m_subtreeEnd);

Let's make them into ASSERT_WITH_MESSAGE to say why they should not happen.

> Source/WebCore/contentextensions/URLFilterParser.cpp:527
> -        if (!m_floatingTerm.isValid())
> -            fail(URLFilterParser::MisplacedQuantifier);
> +        ASSERT(m_floatingTerm.isValid());

Wouldn't this happen with greedy quantifiers?

> Source/WebCore/contentextensions/URLFilterParser.cpp:-727
> -    ASSERT(!pattern.isEmpty());
> -

Those are no longer excluded by the JSON parser?

> Source/WebCore/contentextensions/URLFilterParser.h:44
>      enum ParseStatus {

This thing is really weird.

> Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:355
> +    testPatternStatus("(?!)", ContentExtensions::URLFilterParser::ParseStatus::Group);
> +    testPatternStatus("(?=)", ContentExtensions::URLFilterParser::ParseStatus::Group);

Please also test that those two produce errors with a non-empty term inside.

> Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:370
> +    // I think these should all be YarrErrors, but they're not. Should we do something about this?

What the heck. I have no explanation.

[Alex Christensen]

Comment on attachment 249048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249048&action=review

>> Source/WebCore/contentextensions/URLFilterParser.cpp:527
>> +        ASSERT(m_floatingTerm.isValid());
> 
> Wouldn't this happen with greedy quantifiers?

I'm adding .*a and .*?a, but they do not hit this assert.

>> Source/WebCore/contentextensions/URLFilterParser.cpp:-727
>> -
> 
> Those are no longer excluded by the JSON parser?

I added a test case that tests an empty string.  They probably are excluded by the JSON parser, but the following if statement will catch them.

[Alex Christensen]

https://trac.webkit.org/r181762