fix(core): seal group collision loopholes and harden varargs validation

Mirkoddd · Mirkoddd · commit b6a96489f5be · 2026-02-26T17:39:19.000+01:00
Finalized architectural safety before v1.6.0 release:

- Implemented stealth metadata extraction for nested SiftPattern injections in addPattern and addNamedCapture to prevent group name collisions.
- Added pre-emptive fail-fast null validation for followedBy varargs to prevent corrupted builder states.
- Achieved 100% Jacoco coverage with extensive nested group collision and happy-path scenarios.
diff --git a/sift-core/src/main/java/com/mirkoddd/sift/core/PatternAssembler.java b/sift-core/src/main/java/com/mirkoddd/sift/core/PatternAssembler.java
@@ -44,6 +44,10 @@ class PatternAssembler {
         mainPattern.append(RegexSyntax.GROUP_CLOSE);
     }
 
+    Set<String> getRegisteredGroups() {
+        return registeredGroups;
+    }
+
     void setQuantifier(String quantifier) {
         this.currentQuantifier = quantifier;
     }
@@ -59,28 +63,39 @@ void addClassRange(String range) {
     }
 
     void addClassInclusion(char c, char... additionalExtras) {
-            RegexEscaper.escapeInsideBrackets(c, pendingClass);
-            for (char extra : additionalExtras) {
-                RegexEscaper.escapeInsideBrackets(extra, pendingClass);
-            }
+        RegexEscaper.escapeInsideBrackets(c, pendingClass);
+        for (char extra : additionalExtras) {
+            RegexEscaper.escapeInsideBrackets(extra, pendingClass);
+        }
     }
 
     void addClassExclusion(char excluded, char... additionalExcluded) {
-            pendingClass.append(RegexSyntax.CLASS_INTERSECTION_NEGATION);
-            RegexEscaper.escapeInsideBrackets(excluded, pendingClass);
-            for (char c : additionalExcluded) {
-                RegexEscaper.escapeInsideBrackets(c, pendingClass);
-            }
-            pendingClass.append(RegexSyntax.CLASS_CLOSE);
+        pendingClass.append(RegexSyntax.CLASS_INTERSECTION_NEGATION);
+        RegexEscaper.escapeInsideBrackets(excluded, pendingClass);
+        for (char c : additionalExcluded) {
+            RegexEscaper.escapeInsideBrackets(c, pendingClass);
+        }
+        pendingClass.append(RegexSyntax.CLASS_CLOSE);
     }
 
     void addNamedCapture(NamedCapture group) {
         boolean isAdded = registeredGroups.add(group.getName());
-        if (!isAdded){
+        if (!isAdded) {
             throw new IllegalStateException("A capturing group with the name '" + group.getName() +
                     "' has already been defined. Each group name must be unique.");
         }
 
+        if (group.getPattern() instanceof SiftBuilder) {
+            SiftBuilder subBuilder = (SiftBuilder) group.getPattern();
+            for (String incomingGroup : subBuilder.getRegisteredGroupNames()) {
+                if (!registeredGroups.add(incomingGroup)) {
+                    throw new IllegalStateException("Collision detected: The pattern inside capturing group '" +
+                            group.getName() + "' contains a nested group named '" + incomingGroup +
+                            "' which has already been defined in the current builder.");
+                }
+            }
+        }
+
         mainPattern.append(RegexSyntax.NAMED_GROUP_OPEN)
                 .append(group.getName())
                 .append(RegexSyntax.NAMED_GROUP_NAME_CLOSE)
@@ -116,6 +131,18 @@ void addCharacter(char literal) {
 
     void addPattern(SiftPattern pattern) {
         flush();
+
+        if (pattern instanceof SiftBuilder) {
+            SiftBuilder subBuilder = (SiftBuilder) pattern;
+            for (String incomingGroup : subBuilder.getRegisteredGroupNames()) {
+                boolean isAdded = registeredGroups.add(incomingGroup);
+                if (!isAdded) {
+                    throw new IllegalStateException("Collision detected: The sub-pattern contains a capturing group named '" +
+                            incomingGroup + "' which has already been defined in the current pattern.");
+                }
+            }
+        }
+
         if (!currentQuantifier.isEmpty()) {
             mainPattern.append(RegexSyntax.NON_CAPTURING_GROUP_OPEN)
                     .append(pattern.shake())
diff --git a/sift-core/src/main/java/com/mirkoddd/sift/core/SiftBuilder.java b/sift-core/src/main/java/com/mirkoddd/sift/core/SiftBuilder.java
@@ -18,6 +18,7 @@
 import com.mirkoddd.sift.core.dsl.*;
 
 import java.util.Objects;
+import java.util.Set;
 
 /**
  * Internal implementation of the State Machine.
@@ -52,13 +53,12 @@ public SiftBuilder anchorStart() {
     // QUANTIFIERS
     // ===================================================================================
 
-    @SuppressWarnings("unchecked")
     @Override
     public TypeStep<ConnectorStep, CharacterClassConnectorStep> exactly(int n) {
         if (n < 0) throw new IllegalArgumentException("Quantity cannot be negative: " + n);
         assembler.setQuantifier((n == 1) ? RegexSyntax.EMPTY :
                 RegexSyntax.QUANTIFIER_OPEN + n + RegexSyntax.QUANTIFIER_CLOSE);
-        return (TypeStep<ConnectorStep, CharacterClassConnectorStep>) (Object) this;
+        return this;
     }
 
     @SuppressWarnings("unchecked")
@@ -306,6 +306,10 @@ public ConnectorStep followedBy(SiftPattern pattern, SiftPattern... additionalPa
         Objects.requireNonNull(pattern, "First SiftPattern cannot be null");
         Objects.requireNonNull(additionalPatterns, "Additional SiftPatterns array cannot be null");
 
+        for (int i = 0; i < additionalPatterns.length; i++) {
+            Objects.requireNonNull(additionalPatterns[i], "SiftPattern at index " + i + " cannot be null");
+        }
+
         ConnectorStep current = this.then().exactly(1).pattern(pattern);
 
         for (SiftPattern p : additionalPatterns) {
@@ -355,4 +359,8 @@ public SiftPattern andNothingElse() {
     public String shake() {
         return assembler.build();
     }
+
+    Set<String> getRegisteredGroupNames() {
+        return assembler.getRegisteredGroups();
+    }
 }
diff --git a/sift-core/src/main/java/com/mirkoddd/sift/core/dsl/SiftPattern.java b/sift-core/src/main/java/com/mirkoddd/sift/core/dsl/SiftPattern.java
@@ -15,6 +15,9 @@
  */
 package com.mirkoddd.sift.core.dsl;
 
+import java.util.Collections;
+import java.util.Set;
+
 /**
  * Represents a component that can be converted into a valid Regex string.
  * <p>
diff --git a/sift-core/src/test/java/com/mirkoddd/sift/core/SiftTest.java b/sift-core/src/test/java/com/mirkoddd/sift/core/SiftTest.java
@@ -384,6 +384,17 @@ void throwOnDuplicateGroupNames() {
             assertTrue(exception.getMessage().contains(duplicateName));
         }
 
+        @Test
+        @DisplayName("Should throw NullPointerException when a null pattern is passed in followedBy varargs")
+        void followedByNullVarargs() {
+            SiftPattern validPattern = SiftPatterns.literal("valid");
+
+            NullPointerException exception = assertThrows(NullPointerException.class, () ->
+                    Sift.fromAnywhere().letters().followedBy(validPattern, validPattern, null, validPattern)
+            );
+
+            assertTrue(exception.getMessage().contains("cannot be null"));
+        }
     }
 
     @Nested
@@ -1037,20 +1048,81 @@ void negativeLookbehindTest() {
             assertRegexMatches(regex, "tomcat");
             assertRegexMatches(regex, "cat");
             assertRegexDoesNotMatch(regex, "supercat");
+        }
+    }
 
-            String rege = fromStart()
-                    .exactly(1).unicodeLettersUppercaseOnly() // Must start with an uppercase letter
-                    .then()
-                    .between(3, 15).unicodeWordCharacters().withoutBacktracking() // Secure against ReDoS
-                    .then()
-                    .optional().digits() // May end with an ASCII number
-                    .andNothingElse()
-                    .shake();
+    @Test
+    @DisplayName("Should detect group name collisions when injecting nested patterns")
+    void nestedPatternGroupCollision() {
+        SiftPattern nestedModule = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("id", literal("foo")))
+                .andNothingElse();
+
+        IllegalStateException exception = assertThrows(IllegalStateException.class, () ->
+                Sift.fromAnywhere()
+                        .namedCapture(SiftPatterns.capture("id", literal("bar")))
+                        .then()
+                        .pattern(nestedModule) // FAIL!
+                        .shake()
+        );
+
+        assertTrue(exception.getMessage().contains("Collision detected"));
+        assertTrue(exception.getMessage().contains("id"));
+    }
 
-            // Result from README:    ^\p{Lu}[\p{L}\p{Nd}_]{3,15}+[0-9]?$
-            // Real result from test: ^[\p{Lu}][\p{L}\p{Nd}_]{3,15}+[0-9]?$
-            // Result:                ^[\p{Lu}][\p{L}\p{Nd}_]{3,15}+[0-9]?$
-            System.out.println(rege);
-        }
+    @Test
+    @DisplayName("Should successfully merge nested patterns with non-colliding groups")
+    void nestedPatternGroupMergeSuccess() {
+        SiftPattern nestedModule = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("nestedId", fromAnywhere().oneOrMore().digits()))
+                .andNothingElse();
+
+        String regex = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("mainId", fromAnywhere().oneOrMore().letters()))
+                .then()
+                .pattern(nestedModule)
+                .shake();
+
+        assertEquals("(?<mainId>[a-zA-Z]+)(?<nestedId>[0-9]+)$", regex);
+    }
+
+    @Test
+    @DisplayName("Should detect collisions when a NamedCapture contains nested groups")
+    void namedCaptureNestedGroupCollision() {
+        SiftPattern innerPattern = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("sharedId", Sift.fromAnywhere().oneOrMore().digits()))
+                .andNothingElse();
+
+        NamedCapture wrapperGroup = SiftPatterns.capture("wrapper", innerPattern);
+
+        IllegalStateException exception = assertThrows(IllegalStateException.class, () ->
+                Sift.fromAnywhere()
+                        .namedCapture(SiftPatterns.capture("sharedId", Sift.fromAnywhere().oneOrMore().letters()))
+                        .then()
+                        .namedCapture(wrapperGroup) // FAIL!
+                        .shake()
+        );
+
+        assertTrue(exception.getMessage().contains("Collision detected"));
+        assertTrue(exception.getMessage().contains("wrapper"));
+        assertTrue(exception.getMessage().contains("sharedId"));
+    }
+
+    @Test
+    @DisplayName("Should successfully merge NamedCapture containing non-colliding nested groups")
+    void namedCaptureNestedGroupMergeSuccess() {
+        SiftPattern innerPattern = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("innerId", Sift.fromAnywhere().oneOrMore().digits()))
+                .andNothingElse();
+
+        NamedCapture wrapperGroup = SiftPatterns.capture("wrapper", innerPattern);
+
+        String regex = Sift.fromAnywhere()
+                .namedCapture(SiftPatterns.capture("mainId", Sift.fromAnywhere().oneOrMore().letters()))
+                .then()
+                .namedCapture(wrapperGroup)
+                .shake();
+
+        assertEquals("(?<mainId>[a-zA-Z]+)(?<wrapper>(?<innerId>[0-9]+)$)", regex);
     }
 }
