LibRegex: Reject invalid /v class set expressions in negated classes

Some string literals were incorrectly accepted inside negated classes
because the negated class check computed the actual result string set
instead of applying the structural rules.

This commit replaces class_set_expression_strings with a structural
class_set_expression_may_contain_strings check. Also removes some
incorrect tests.

Author: aplefull

Libraries/LibRegex/Rust/src/parser.rs

@@ -1182,7 +1182,7 @@ impl Parser {
                 self.parse_class_set_expression()?
             };
             self.in_negated_class = saved_negated;
-            if negated && !Self::class_set_expression_strings(&expr).is_empty() {
+            if negated && Self::class_set_expression_may_contain_strings(&expr) {
                 return Err(Error::InvalidCharacterClass);
             }
             self.expect(']')?;
@@ -1413,6 +1413,9 @@ impl Parser {
                 self.in_negated_class = false;
                 while self.peek_pair() == Some(('&', '&')) {
                     self.pos += 2; // consume '&&'
+                    if self.peek() == Some('&') {
+                        return Err(Error::InvalidCharacterClass);
+                    }
                     operands.push(self.parse_class_set_operand()?);
                 }
                 self.in_negated_class = saved_negated;
@@ -1454,93 +1457,38 @@ impl Parser {
         Ok(())
     }
 
-    fn get_string_property_strings(name: &str) -> std::collections::BTreeSet<Vec<u32>> {
-        let buf = libunicode_rust::character_types::get_string_property_data(name);
-        if buf.is_empty() {
-            return std::collections::BTreeSet::new();
-        }
-
-        let count = buf[0] as usize;
-        let mut strings = std::collections::BTreeSet::new();
-        let mut offset = 1;
-        for _ in 0..count {
-            if offset >= buf.len() {
-                break;
-            }
-            let len = buf[offset] as usize;
-            offset += 1;
-            if offset + len > buf.len() {
-                break;
-            }
-            if len > 1 {
-                strings.insert(buf[offset..offset + len].to_vec());
-            }
-            offset += len;
-        }
-        strings
-    }
-
-    fn class_set_expression_strings(expr: &ClassSetExpression) -> std::collections::BTreeSet<Vec<u32>> {
+    /// - Union: may contain strings if ANY operand may contain strings
+    /// - Intersection: may contain strings if ALL operands may contain strings
+    ///   (i.e., no operand is "purely characters")
+    /// - Subtraction: may contain strings if the leftmost operand may contain strings
+    fn class_set_expression_may_contain_strings(expr: &ClassSetExpression) -> bool {
         match expr {
-            ClassSetExpression::Union(operands) => {
-                operands
-                    .iter()
-                    .fold(std::collections::BTreeSet::new(), |mut strings, operand| {
-                        strings.extend(Self::class_set_operand_strings(operand));
-                        strings
-                    })
-            }
+            ClassSetExpression::Union(operands) => operands.iter().any(Self::class_set_operand_may_contain_strings),
             ClassSetExpression::Intersection(operands) => {
-                let Some((first, rest)) = operands.split_first() else {
-                    return std::collections::BTreeSet::new();
-                };
-                let mut strings = Self::class_set_operand_strings(first);
-                for operand in rest {
-                    let operand_strings = Self::class_set_operand_strings(operand);
-                    strings.retain(|string| operand_strings.contains(string));
-                }
-                strings
-            }
-            ClassSetExpression::Subtraction(operands) => {
-                let Some((first, rest)) = operands.split_first() else {
-                    return std::collections::BTreeSet::new();
-                };
-                let mut strings = Self::class_set_operand_strings(first);
-                for operand in rest {
-                    let operand_strings = Self::class_set_operand_strings(operand);
-                    strings.retain(|string| !operand_strings.contains(string));
-                }
-                strings
+                operands.iter().all(Self::class_set_operand_may_contain_strings)
             }
+            ClassSetExpression::Subtraction(operands) => operands
+                .first()
+                .is_some_and(Self::class_set_operand_may_contain_strings),
         }
     }
 
-    fn class_set_operand_strings(operand: &ClassSetOperand) -> std::collections::BTreeSet<Vec<u32>> {
+    fn class_set_operand_may_contain_strings(operand: &ClassSetOperand) -> bool {
         match operand {
             ClassSetOperand::NestedClass(class) => {
                 if class.negated {
-                    return std::collections::BTreeSet::new();
+                    return false;
                 }
                 match &class.body {
-                    CharacterClassBody::Ranges(_) => std::collections::BTreeSet::new(),
-                    CharacterClassBody::UnicodeSet(expr) => Self::class_set_expression_strings(expr),
+                    CharacterClassBody::Ranges(_) => false,
+                    CharacterClassBody::UnicodeSet(expr) => Self::class_set_expression_may_contain_strings(expr),
                 }
             }
             ClassSetOperand::UnicodeProperty(property) => {
-                if !property.negated && property.value.is_none() && Self::is_string_property(&property.name) {
-                    return Self::get_string_property_strings(&property.name);
-                }
-                std::collections::BTreeSet::new()
-            }
-            ClassSetOperand::StringLiteral(chars) => {
-                if chars.len() > 1 {
-                    return [chars.iter().map(|ch| *ch as u32).collect()].into_iter().collect();
-                }
-                std::collections::BTreeSet::new()
-            }
-            ClassSetOperand::Char(_) | ClassSetOperand::Range(_, _) | ClassSetOperand::BuiltinClass(_) => {
-                std::collections::BTreeSet::new()
+                !property.negated && property.value.is_none() && Self::is_string_property(&property.name)
             }
+            ClassSetOperand::StringLiteral(chars) => chars.len() != 1,
+            ClassSetOperand::Char(_) | ClassSetOperand::Range(_, _) | ClassSetOperand::BuiltinClass(_) => false,
         }
     }
 

Tests/LibJS/Runtime/builtins/RegExp/RegExp.js

@@ -36,12 +36,7 @@ describe("errors", () => {
     });
 
     test("valid pattern (negated v-mode class set ops can eliminate strings)", () => {
-        for (const pattern of [
-            "[^[[a-z]--[\\q{ab}]]]",
-            "[^[[\\q{ab}]&&[a-z]]]",
-            "[^[[\\q{ab}]--[\\q{ab}]]]",
-            "[^[[\\q{ab}]&&[\\q{cd}]]]",
-        ]) {
+        for (const pattern of ["[^[[a-z]--[\\q{ab}]]]", "[^[[\\q{ab}]&&[a-z]]]"]) {
             expect(() => {
                 RegExp(pattern, "v");
             }).not.toThrow();

Tests/LibJS/Runtime/regexp-unicode-sets-negated-class.js

@@ -0,0 +1,32 @@
+test("triple & in unicode sets class is a syntax error", () => {
+    expect(() => eval("/[a&&&]/v")).toThrow(SyntaxError);
+    expect(() => eval("/[a&&b&&&c]/v")).toThrow(SyntaxError);
+});
+
+test("empty \\q{} in negated unicode sets class is a syntax error", () => {
+    expect(() => eval("/[^\\q{}]/v")).toThrow(SyntaxError);
+});
+
+test("multi-char string intersection where all operands contain strings is a syntax error in negated class", () => {
+    expect(() => eval("/[^\\q{foo}&&\\q{bar}]/v")).toThrow(SyntaxError);
+    expect(() => eval("/[^\\q{foo}&&\\q{foo}]/v")).toThrow(SyntaxError);
+});
+
+test("multi-char string in negated unicode sets class union is a syntax error", () => {
+    expect(() => eval("/[^\\q{ab}]/v")).toThrow(SyntaxError);
+    expect(() => eval("/[^\\q{foo}]/v")).toThrow(SyntaxError);
+});
+
+test("subtraction with string-valued left operand is a syntax error in negated class", () => {
+    expect(() => eval("/[^\\q{foo}--[a-z]]/v")).toThrow(SyntaxError);
+});
+
+test("valid unicode sets class set operations in negated class", () => {
+    expect(() => eval("/[^\\q{a}]/v")).not.toThrow();
+    expect(() => eval("/[^\\q{foo}&&[a]]/v")).not.toThrow();
+    expect(() => eval("/[^\\q{a}&&\\q{b}]/v")).not.toThrow();
+    expect(() => eval("/[^\\q{a}&&[a-z]]/v")).not.toThrow();
+    expect(() => eval("/[^[a-z]--\\q{foo}]/v")).not.toThrow();
+    expect(() => eval("/[a&&b]/v")).not.toThrow();
+    expect(() => eval("/[a&&b&&c]/v")).not.toThrow();
+});

Tests/LibJS/Runtime/syntax/regex-literal-errors.js

@@ -89,12 +89,7 @@ test("negated v-mode classes containing nested strings are syntax errors", () =>
 });
 
 test("negated v-mode class set ops that eliminate strings are valid", () => {
-    for (const source of [
-        "/[^[[a-z]--[\\q{ab}]]]/v",
-        "/[^[[\\q{ab}]&&[a-z]]]/v",
-        "/[^[[\\q{ab}]--[\\q{ab}]]]/v",
-        "/[^[[\\q{ab}]&&[\\q{cd}]]]/v",
-    ]) {
+    for (const source of ["/[^[[a-z]--[\\q{ab}]]]/v", "/[^[[\\q{ab}]&&[a-z]]]/v"]) {
         expect(source).toEval();
         expect(() => eval(source)).not.toThrow();
         expect(() => new Function(source)).not.toThrow();