WebKit Bugzilla
Attachment 349187 Details for
Bug 189407
: Test262 failure with Named Capture Groups - using a reference before the group is defined
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
189407.patch (text/plain), 21.30 KB, created by
Michael Saboff
on 2018-09-07 14:03:35 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Michael Saboff
Created:
2018-09-07 14:03:35 PDT
Size:
21.30 KB
patch
obsolete
>Index: JSTests/ChangeLog >=================================================================== >--- JSTests/ChangeLog (revision 235803) >+++ JSTests/ChangeLog (working copy) >@@ -1,3 +1,15 @@ >+2018-09-07 Michael Saboff <msaboff@apple.com> >+ >+ Test262 failure with Named Capture Groups - using a reference before the group is defined >+ https://bugs.webkit.org/show_bug.cgi?id=189407 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Re-enabled previously failing test. >+ >+ * test262.yaml: >+ * test262/expectations.yaml >+ > 2018-09-06 Yusuke Suzuki <yusukesuzuki@slowstart.org> > > [DFG] DFG should handle String#toString >Index: JSTests/test262.yaml >=================================================================== >--- JSTests/test262.yaml (revision 235763) >+++ JSTests/test262.yaml (working copy) >@@ -42180,9 +42180,9 @@ > - path: test262/test/built-ins/RegExp/named-groups/non-unicode-property-names.js > cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [:strict] > - path: test262/test/built-ins/RegExp/named-groups/non-unicode-references.js >- cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [] >+ cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [] > - path: test262/test/built-ins/RegExp/named-groups/non-unicode-references.js >- cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [:strict] >+ cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [:strict] > - path: test262/test/built-ins/RegExp/named-groups/string-replace-escaped.js > cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [] > - path: test262/test/built-ins/RegExp/named-groups/string-replace-escaped.js >@@ -42224,9 +42224,9 @@ > - path: test262/test/built-ins/RegExp/named-groups/unicode-property-names.js > cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict] > - path: test262/test/built-ins/RegExp/named-groups/unicode-references.js >- cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [] >+ cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [] > - path: test262/test/built-ins/RegExp/named-groups/unicode-references.js >- cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [:strict] >+ cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js", "../../../../harness/compareArray.js"], [:strict] > - path: test262/test/built-ins/RegExp/property-escapes/binary-properties-with-value.js > cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [] > - path: test262/test/built-ins/RegExp/property-escapes/binary-properties-with-value.js >Index: JSTests/test262/expectations.yaml >=================================================================== >--- JSTests/test262/expectations.yaml (revision 235763) >+++ JSTests/test262/expectations.yaml (working copy) >@@ -1122,9 +1122,6 @@ test/built-ins/RegExp/named-groups/group > test/built-ins/RegExp/named-groups/non-unicode-malformed.js: > default: 'Test262Error: Expected a SyntaxError to be thrown but no exception was thrown at all' > strict mode: 'Test262Error: Expected a SyntaxError to be thrown but no exception was thrown at all' >-test/built-ins/RegExp/named-groups/non-unicode-references.js: >- default: "TypeError: null is not an object (evaluating 'b.length')" >- strict mode: "TypeError: null is not an object (evaluating 'b.length')" > test/built-ins/RegExp/named-groups/string-replace-nocaptures.js: > default: 'Test262Error: Expected SameValue(ë$<snd>$<fst>cdû, ë$<$<fst>cdû) to be true' > strict mode: 'Test262Error: Expected SameValue(ë$<snd>$<fst>cdû, ë$<$<fst>cdû) to be true' >@@ -1134,9 +1131,6 @@ test/built-ins/RegExp/named-groups/unico > test/built-ins/RegExp/named-groups/unicode-property-names.js: > default: 'SyntaxError: Invalid regular expression: invalid group specifier name' > strict mode: 'SyntaxError: Invalid regular expression: invalid group specifier name' >-test/built-ins/RegExp/named-groups/unicode-references.js: >- default: 'SyntaxError: Invalid regular expression: invalid backreference for unicode pattern' >- strict mode: 'SyntaxError: Invalid regular expression: invalid backreference for unicode pattern' > test/built-ins/RegExp/proto-from-ctor-realm.js: > default: 'Test262Error: Expected SameValue(ë/(?:)/û, ë/(?:)/û) to be true' > strict mode: 'Test262Error: Expected SameValue(ë/(?:)/û, ë/(?:)/û) to be true' >Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 235763) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -1,3 +1,40 @@ >+2018-09-07 Michael Saboff <msaboff@apple.com> >+ >+ Test262 failure with Named Capture Groups - using a reference before the group is defined >+ https://bugs.webkit.org/show_bug.cgi?id=189407 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Added code to save the named forward references we see during parsing and validating that >+ they are all present when parsing the RegExp is complete. If there are unnamed references, >+ we reparse with some variation of behavior. Just like for numeric references, the >+ behavior is different depending on whether or not the unicode (u flag) is present. >+ For non-unicode patterns, we treat the \k<...> as a literal pattern. For a unicode >+ pattern we throw an exception. >+ >+ Did some refactoring, renaming YarrPattern::reset() and YarrPatternConstructor::reset() >+ resetForReparsing() as that is the only use for those methods. >+ >+ * yarr/YarrParser.h: >+ (JSC::Yarr::Parser::CharacterClassParserDelegate::isValidNamedForwardReference): >+ (JSC::Yarr::Parser::CharacterClassParserDelegate::atomNamedForwardReference): >+ (JSC::Yarr::Parser::parseEscape): >+ * yarr/YarrPattern.cpp: >+ (JSC::Yarr::YarrPatternConstructor::resetForReparsing): >+ (JSC::Yarr::YarrPatternConstructor::saveUnmatchedNamedForwardReferences): >+ (JSC::Yarr::YarrPatternConstructor::isValidNamedForwardReference): >+ (JSC::Yarr::YarrPatternConstructor::atomNamedForwardReference): >+ (JSC::Yarr::YarrPattern::compile): >+ (JSC::Yarr::YarrPattern::YarrPattern): >+ (JSC::Yarr::YarrPatternConstructor::reset): Deleted. >+ * yarr/YarrPattern.h: >+ (JSC::Yarr::YarrPattern::resetForReparsing): >+ (JSC::Yarr::YarrPattern::containsIllegalNamedForwardReferences): >+ (JSC::Yarr::YarrPattern::reset): Deleted. >+ * yarr/YarrSyntaxChecker.cpp: >+ (JSC::Yarr::SyntaxChecker::isValidNamedForwardReference): >+ (JSC::Yarr::SyntaxChecker::atomNamedForwardReference): >+ > 2018-09-06 Mark Lam <mark.lam@apple.com> > > Gardening: only visit m_cachedStructureID if it's not null. >Index: Source/JavaScriptCore/yarr/YarrParser.h >=================================================================== >--- Source/JavaScriptCore/yarr/YarrParser.h (revision 235763) >+++ Source/JavaScriptCore/yarr/YarrParser.h (working copy) >@@ -195,6 +195,8 @@ private: > NO_RETURN_DUE_TO_ASSERT void assertionWordBoundary(bool) { RELEASE_ASSERT_NOT_REACHED(); } > NO_RETURN_DUE_TO_ASSERT void atomBackReference(unsigned) { RELEASE_ASSERT_NOT_REACHED(); } > NO_RETURN_DUE_TO_ASSERT void atomNamedBackReference(String) { RELEASE_ASSERT_NOT_REACHED(); } >+ NO_RETURN_DUE_TO_ASSERT bool isValidNamedForwardReference(String) { RELEASE_ASSERT_NOT_REACHED(); } >+ NO_RETURN_DUE_TO_ASSERT void atomNamedForwardReference(String) { RELEASE_ASSERT_NOT_REACHED(); } > > private: > Delegate& m_delegate; >@@ -421,9 +423,16 @@ private: > if (!atEndOfPattern() && !inCharacterClass) { > if (consume() == '<') { > auto groupName = tryConsumeGroupName(); >- if (groupName && m_captureGroupNames.contains(groupName.value())) { >- delegate.atomNamedBackReference(groupName.value()); >- break; >+ if (groupName) { >+ if (m_captureGroupNames.contains(groupName.value())) { >+ delegate.atomNamedBackReference(groupName.value()); >+ break; >+ } >+ >+ if (delegate.isValidNamedForwardReference(groupName.value())) { >+ delegate.atomNamedForwardReference(groupName.value()); >+ break; >+ } > } > if (m_isUnicode) { > m_errorCode = ErrorCode::InvalidBackreference; >@@ -1137,6 +1146,8 @@ private: > * void atomParenthesesEnd(); > * void atomBackReference(unsigned subpatternId); > * void atomNamedBackReference(String subpatternName); >+ * bool isValidNamedForwardReference(String subpatternName); >+ * void atomNamedForwardReference(String subpatternName); > * > * void quantifyAtom(unsigned min, unsigned max, bool greedy); > * >Index: Source/JavaScriptCore/yarr/YarrPattern.cpp >=================================================================== >--- Source/JavaScriptCore/yarr/YarrPattern.cpp (revision 235763) >+++ Source/JavaScriptCore/yarr/YarrPattern.cpp (working copy) >@@ -446,9 +446,9 @@ public: > { > } > >- void reset() >+ void resetForReparsing() > { >- m_pattern.reset(); >+ m_pattern.resetForReparsing(); > m_characterClassConstructor.reset(); > > auto body = std::make_unique<PatternDisjunction>(); >@@ -456,7 +456,17 @@ public: > m_alternative = body->addNewAlternative(); > m_pattern.m_disjunctions.append(WTFMove(body)); > } >- >+ >+ void saveUnmatchedNamedForwardReferences() >+ { >+ m_invalidNamedForwardReferences.shrink(0); >+ >+ for (auto it = m_pattern.m_namedForwardReferences.begin(), itEnd = m_pattern.m_namedForwardReferences.end(); it < itEnd; ++it) { >+ if (!m_pattern.m_captureGroupNames.contains(*it)) >+ m_invalidNamedForwardReferences.append(*it); >+ } >+ } >+ > void assertionBOL() > { > if (!m_alternative->m_terms.size() && !m_invertParentheticalAssertion) { >@@ -672,6 +682,20 @@ public: > atomBackReference(m_pattern.m_namedGroupToParenIndex.get(subpatternName)); > } > >+ bool isValidNamedForwardReference(String subpatternName) >+ { >+ if (!m_invalidNamedForwardReferences.find(subpatternName)) >+ return false; >+ >+ return true; >+ } >+ >+ void atomNamedForwardReference(String subpatternName) >+ { >+ m_pattern.m_namedForwardReferences.appendIfNotContains(subpatternName); >+ m_alternative->m_terms.append(PatternTerm::ForwardReference()); >+ } >+ > // deep copy the argument disjunction. If filterStartsWithBOL is true, > // skip alternatives with m_startsWithBOL set true. > PatternDisjunction* copyDisjunction(PatternDisjunction* disjunction, bool filterStartsWithBOL = false) >@@ -1080,6 +1104,7 @@ private: > YarrPattern& m_pattern; > PatternAlternative* m_alternative; > CharacterClassConstructor m_characterClassConstructor; >+ Vector<String> m_invalidNamedForwardReferences; > void* m_stackLimit; > bool m_invertCharacterClass; > bool m_invertParentheticalAssertion { false }; >@@ -1102,13 +1127,14 @@ ErrorCode YarrPattern::compile(const Str > // Quoting Netscape's "What's new in JavaScript 1.2", > // "Note: if the number of left parentheses is less than the number specified > // in \#, the \# is taken as an octal escape as described in the next row." >- if (containsIllegalBackReference()) { >+ if (containsIllegalBackReference() || containsIllegalNamedForwardReferences()) { > if (unicode()) > return ErrorCode::InvalidBackreference; > > unsigned numSubpatterns = m_numSubpatterns; > >- constructor.reset(); >+ constructor.saveUnmatchedNamedForwardReferences(); >+ constructor.resetForReparsing(); > ErrorCode error = parse(constructor, patternString, unicode(), numSubpatterns); > ASSERT_UNUSED(error, !hasError(error)); > ASSERT(numSubpatterns == m_numSubpatterns); >@@ -1139,6 +1165,7 @@ YarrPattern::YarrPattern(const String& p > , m_flags(flags) > { > error = compile(pattern, stackLimit); >+ // &&&& > } > > void indentForNestingLevel(PrintStream& out, unsigned nestingDepth) >Index: Source/JavaScriptCore/yarr/YarrPattern.h >=================================================================== >--- Source/JavaScriptCore/yarr/YarrPattern.h (revision 235763) >+++ Source/JavaScriptCore/yarr/YarrPattern.h (working copy) >@@ -354,7 +354,7 @@ struct TermChain { > struct YarrPattern { > JS_EXPORT_PRIVATE YarrPattern(const String& pattern, RegExpFlags, ErrorCode&, void* stackLimit = nullptr); > >- void reset() >+ void resetForReparsing() > { > m_numSubpatterns = 0; > m_maxBackReference = 0; >@@ -381,12 +381,28 @@ struct YarrPattern { > m_disjunctions.clear(); > m_userCharacterClasses.clear(); > m_captureGroupNames.shrink(0); >+ m_namedForwardReferences.shrink(0); > } > > bool containsIllegalBackReference() > { > return m_maxBackReference > m_numSubpatterns; > } >+ >+ bool containsIllegalNamedForwardReferences() >+ { >+ if (m_namedForwardReferences.isEmpty()) >+ return false; >+ >+ bool result = false; >+ >+ for (auto it = m_namedForwardReferences.begin(), itEnd = m_namedForwardReferences.end(); it < itEnd; ++it) { >+ if (!m_captureGroupNames.contains(*it)) >+ result = true; >+ } >+ >+ return result; >+ } > > bool containsUnsignedLengthPattern() > { >@@ -513,6 +529,7 @@ struct YarrPattern { > Vector<std::unique_ptr<PatternDisjunction>, 4> m_disjunctions; > Vector<std::unique_ptr<CharacterClass>> m_userCharacterClasses; > Vector<String> m_captureGroupNames; >+ Vector<String> m_namedForwardReferences; > HashMap<String, unsigned> m_namedGroupToParenIndex; > > private: >Index: Source/JavaScriptCore/yarr/YarrSyntaxChecker.cpp >=================================================================== >--- Source/JavaScriptCore/yarr/YarrSyntaxChecker.cpp (revision 235763) >+++ Source/JavaScriptCore/yarr/YarrSyntaxChecker.cpp (working copy) >@@ -49,6 +49,8 @@ public: > void atomParenthesesEnd() {} > void atomBackReference(unsigned) {} > void atomNamedBackReference(String) {} >+ bool isValidNamedForwardReference(String) { return true; } >+ void atomNamedForwardReference(String) {} > void quantifyAtom(unsigned, unsigned, bool) {} > void disjunction() {} > }; >Index: Source/WebCore/ChangeLog >=================================================================== >--- Source/WebCore/ChangeLog (revision 235763) >+++ Source/WebCore/ChangeLog (working copy) >@@ -1,3 +1,21 @@ >+2018-09-07 Michael Saboff <msaboff@apple.com> >+ >+ Test262 failure with Named Capture Groups - using a reference before the group is defined >+ https://bugs.webkit.org/show_bug.cgi?id=189407 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Created new delegate stubs for RegExp parsing. These are not needed for the URL >+ filtering use cases. >+ >+ No new tests, no functionality changed. >+ >+ * contentextensions/URLFilterParser.cpp: >+ (WebCore::ContentExtensions::PatternParser::isValidNamedForwardReference): >+ (WebCore::ContentExtensions::PatternParser::atomNamedForwardReference): >+ (WebCore::ContentExtensions::URLFilterParser::statusString): >+ * contentextensions/URLFilterParser.h: >+ > 2018-09-06 Zalan Bujtas <zalan@apple.com> > > [LFC][BFC] Add support for min(max)-height >Index: Source/WebCore/contentextensions/URLFilterParser.cpp >=================================================================== >--- Source/WebCore/contentextensions/URLFilterParser.cpp (revision 235763) >+++ Source/WebCore/contentextensions/URLFilterParser.cpp (working copy) >@@ -134,6 +134,16 @@ public: > fail(URLFilterParser::BackReference); > } > >+ bool isValidNamedForwardReference(String) >+ { >+ return true; >+ } >+ >+ void atomNamedForwardReference(String) >+ { >+ fail(URLFilterParser::ForwardReference); >+ } >+ > void assertionBOL() > { > if (hasError()) >@@ -372,6 +382,8 @@ String URLFilterParser::statusString(Par > return "Character class is not supported."; > case BackReference: > return "Patterns cannot contain backreferences."; >+ case ForwardReference: >+ return "Patterns cannot contain forward references."; > case MisplacedStartOfLine: > return "Start of line assertion can only appear as the first term in a filter."; > case WordBoundary: >Index: Source/WebCore/contentextensions/URLFilterParser.h >=================================================================== >--- Source/WebCore/contentextensions/URLFilterParser.h (revision 235763) >+++ Source/WebCore/contentextensions/URLFilterParser.h (working copy) >@@ -43,6 +43,7 @@ public: > NonASCII, > UnsupportedCharacterClass, > BackReference, >+ ForwardReference, > MisplacedStartOfLine, > WordBoundary, > AtomCharacter, >Index: LayoutTests/ChangeLog >=================================================================== >--- LayoutTests/ChangeLog (revision 235763) >+++ LayoutTests/ChangeLog (working copy) >@@ -1,3 +1,15 @@ >+2018-09-07 Michael Saboff <msaboff@apple.com> >+ >+ Test262 failure with Named Capture Groups - using a reference before the group is defined >+ https://bugs.webkit.org/show_bug.cgi?id=189407 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Updated tests. >+ >+ * js/regexp-named-capture-groups-expected.txt: >+ * js/script-tests/regexp-named-capture-groups.js: >+ > 2018-09-06 Zalan Bujtas <zalan@apple.com> > > [LFC][BFC] Add support for min(max)-height >Index: LayoutTests/js/regexp-named-capture-groups-expected.txt >=================================================================== >--- LayoutTests/js/regexp-named-capture-groups-expected.txt (revision 235763) >+++ LayoutTests/js/regexp-named-capture-groups-expected.txt (working copy) >@@ -61,6 +61,12 @@ PASS let r = new RegExp("/(?<ðgroupNa > PASS let r = new RegExp("/(?<gðoupName1>abc)/u") threw exception SyntaxError: Invalid regular expression: invalid group specifier name. > PASS let r = new RegExp("/(?<âgroupName1>abc)/u") threw exception SyntaxError: Invalid regular expression: invalid group specifier name. > PASS let r = new RegExp("/(?<âgroupName1>abc)/u") threw exception SyntaxError: Invalid regular expression: invalid group specifier name. >+PASS "XzzXzz".match(/\k<z>X(?<z>z*)X\k<z>/) is ["XzzXzz", "zz"] >+PASS "XzzXzz".match(/\k<z>X(?<z>z*)X\k<z>/u) is ["XzzXzz", "zz"] >+PASS "1122332211".match(/\k<ones>\k<twos>\k<threes>(?<ones>1*)(?<twos>2*)(?<threes>3*)\k<threes>\k<twos>\k<ones>/) is ["1122332211", "11", "22", "3"] >+PASS "1122332211".match(/\k<ones>\k<twos>\k<threes>(?<ones>1*)(?<twos>2*)(?<threes>3*)\k<threes>\k<twos>\k<ones>/u) is ["1122332211", "11", "22", "3"] >+PASS "\k<z>XzzX".match(/\k<z>X(z*)X/) is ["k<z>XzzX", "zz"] >+PASS "\k<z>XzzX".match(/\k<z>X(z*)X/u) threw exception SyntaxError: Invalid regular expression: invalid backreference for unicode pattern. > PASS successfullyParsed is true > > TEST COMPLETE >Index: LayoutTests/js/script-tests/regexp-named-capture-groups.js >=================================================================== >--- LayoutTests/js/script-tests/regexp-named-capture-groups.js (revision 235763) >+++ LayoutTests/js/script-tests/regexp-named-capture-groups.js (working copy) >@@ -104,3 +104,13 @@ shouldThrow('let r = new RegExp("/(?<g\u > shouldThrow('let r = new RegExp("/(?<\u200cgroupName1>abc)/u")', '"SyntaxError: Invalid regular expression: invalid group specifier name"'); > shouldThrow('let r = new RegExp("/(?<\u200dgroupName1>abc)/u")', '"SyntaxError: Invalid regular expression: invalid group specifier name"'); > >+// Check the named forward references work >+shouldBe('"XzzXzz".match(/\\\k<z>X(?<z>z*)X\\\k<z>/)', '["XzzXzz", "zz"]'); >+shouldBe('"XzzXzz".match(/\\\k<z>X(?<z>z*)X\\\k<z>/u)', '["XzzXzz", "zz"]'); >+shouldBe('"1122332211".match(/\\\k<ones>\\\k<twos>\\\k<threes>(?<ones>1*)(?<twos>2*)(?<threes>3*)\\\k<threes>\\\k<twos>\\\k<ones>/)', '["1122332211", "11", "22", "3"]'); >+shouldBe('"1122332211".match(/\\\k<ones>\\\k<twos>\\\k<threes>(?<ones>1*)(?<twos>2*)(?<threes>3*)\\\k<threes>\\\k<twos>\\\k<ones>/u)', '["1122332211", "11", "22", "3"]'); >+ >+// Check that a named forward reference for a non-existent named capture >+// matches for non-unicode patterns and throws for unicode patterns. >+shouldBe('"\\\k<z>XzzX".match(/\\\k<z>X(z*)X/)', '["k<z>XzzX", "zz"]'); >+shouldThrow('"\\\k<z>XzzX".match(/\\\k<z>X(z*)X/u)', '"SyntaxError: Invalid regular expression: invalid backreference for unicode pattern"');
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189407
:
349187
|
349212
|
349355