浏览代码

LibJS: Bring back runtime validation of RegExp flags

This is a partial revert of commit 60064e2, which removed the validation
of RegExp flags during runtime and expected the parser to do that
exclusively - however this was not taking into account the RegExp()
constructor, which was subsequently crashing on invalid flags.

Also adds test for these constructor error cases, which were obviously
missing before.

Fixes #7042.
Linus Groh 4 年之前
父节点
当前提交
d85b9fd5a0

+ 2 - 0
Userland/Libraries/LibJS/Runtime/ErrorTypes.h

@@ -138,6 +138,8 @@
     M(ReflectBadNewTarget, "Optional third argument of Reflect.construct() must be a constructor")                                      \
     M(ReflectBadDescriptorArgument, "Descriptor argument is not an object")                                                             \
     M(RegExpCompileError, "RegExp compile error: {}")                                                                                   \
+    M(RegExpObjectBadFlag, "Invalid RegExp flag '{}'")                                                                                  \
+    M(RegExpObjectRepeatedFlag, "Repeated RegExp flag '{}'")                                                                            \
     M(StringRawCannotConvert, "Cannot convert property 'raw' to object from {}")                                                        \
     M(StringRepeatCountMustBe, "repeat count must be a {} number")                                                                      \
     M(ThisHasNotBeenInitialized, "|this| has not been initialized")                                                                     \

+ 24 - 4
Userland/Libraries/LibJS/Runtime/RegExpObject.cpp

@@ -13,8 +13,10 @@
 
 namespace JS {
 
-static Flags options_from(const String& flags)
+static Flags options_from(GlobalObject& global_object, const String& flags)
 {
+    auto& vm = global_object.vm();
+    bool g = false, i = false, m = false, s = false, u = false, y = false;
     Flags options {
         // JS regexps are all 'global' by default as per our definition, but the "global" flag enables "stateful".
         // FIXME: Enable 'BrowserExtended' only if in a browser context.
@@ -25,26 +27,44 @@ static Flags options_from(const String& flags)
     for (auto ch : flags) {
         switch (ch) {
         case 'g':
+            if (g)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            g = true;
             options.effective_flags |= regex::ECMAScriptFlags::Global;
             options.declared_flags |= regex::ECMAScriptFlags::Global;
             break;
         case 'i':
+            if (i)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            i = true;
             options.effective_flags |= regex::ECMAScriptFlags::Insensitive;
             options.declared_flags |= regex::ECMAScriptFlags::Insensitive;
             break;
         case 'm':
+            if (m)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            m = true;
             options.effective_flags |= regex::ECMAScriptFlags::Multiline;
             options.declared_flags |= regex::ECMAScriptFlags::Multiline;
             break;
         case 's':
+            if (s)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            s = true;
             options.effective_flags |= regex::ECMAScriptFlags::SingleLine;
             options.declared_flags |= regex::ECMAScriptFlags::SingleLine;
             break;
         case 'u':
+            if (u)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            u = true;
             options.effective_flags |= regex::ECMAScriptFlags::Unicode;
             options.declared_flags |= regex::ECMAScriptFlags::Unicode;
             break;
         case 'y':
+            if (y)
+                vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectRepeatedFlag, ch);
+            y = true;
             // Now for the more interesting flag, 'sticky' actually unsets 'global', part of which is the default.
             options.effective_flags.reset_flag(regex::ECMAScriptFlags::Global);
             // "What's the difference between sticky and global, then", that's simple.
@@ -55,8 +75,8 @@ static Flags options_from(const String& flags)
             options.declared_flags |= regex::ECMAScriptFlags::Sticky;
             break;
         default:
-            // RegExp flags must be validated by the parser.
-            VERIFY_NOT_REACHED();
+            vm.throw_exception<SyntaxError>(global_object, ErrorType::RegExpObjectBadFlag, ch);
+            return options;
         }
     }
 
@@ -72,7 +92,7 @@ RegExpObject::RegExpObject(String pattern, String flags, Object& prototype)
     : Object(prototype)
     , m_pattern(pattern)
     , m_flags(flags)
-    , m_active_flags(options_from(m_flags))
+    , m_active_flags(options_from(global_object(), m_flags))
     , m_regex(pattern, m_active_flags.effective_flags)
 {
     if (m_regex.parser_result.error != regex::Error::NoError) {

+ 23 - 0
Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.js

@@ -1,3 +1,26 @@
+describe("errors", () => {
+    test("invalid pattern", () => {
+        expect(() => {
+            RegExp("[");
+        }).toThrowWithMessage(
+            SyntaxError,
+            "RegExp compile error: Error during parsing of regular expression:"
+        );
+    });
+
+    test("invalid flag", () => {
+        expect(() => {
+            RegExp("", "x");
+        }).toThrowWithMessage(SyntaxError, "Invalid RegExp flag 'x'");
+    });
+
+    test("repeated flag", () => {
+        expect(() => {
+            RegExp("", "gg");
+        }).toThrowWithMessage(SyntaxError, "Repeated RegExp flag 'g'");
+    });
+});
+
 test("basic functionality", () => {
     expect(RegExp().toString()).toBe("/(?:)/");
     expect(RegExp(undefined).toString()).toBe("/(?:)/");