From d85b9fd5a09e451c5faa7922c3f8ab2661f0706e Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 11 May 2021 22:47:14 +0100 Subject: [PATCH] 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. --- Userland/Libraries/LibJS/Runtime/ErrorTypes.h | 2 ++ .../Libraries/LibJS/Runtime/RegExpObject.cpp | 28 ++++++++++++++++--- .../LibJS/Tests/builtins/RegExp/RegExp.js | 23 +++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/ErrorTypes.h b/Userland/Libraries/LibJS/Runtime/ErrorTypes.h index 693cf4a6b9..f4fcf82b2a 100644 --- a/Userland/Libraries/LibJS/Runtime/ErrorTypes.h +++ b/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") \ diff --git a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp b/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp index ea669aecb4..f2734ff8d6 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp +++ b/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(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(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(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(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(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(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(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) { diff --git a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.js b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.js index 9ae8ab97c0..0a65d2a25b 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.js +++ b/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("/(?:)/");