Fix incorrect IMAP-style sequence-set element handling

src/PhpImap/Imap.php

@@ -1156,20 +1156,35 @@ private static function EnsureRange(
             throw new InvalidArgumentException('Argument 1 passed to '.__METHOD__.'() must be an integer or a string!');
         }
 
-        $regex = '/^\d+:\d+$/';
-        $suffix = '() did not appear to be a valid message id range!';
+        if (\is_int($msg_number) || \preg_match('/^\d+$/', $msg_number)) {
+            return \sprintf('%1$s:%1$s', $msg_number);
+        }
 
         if ($allow_sequence) {
-            $regex = '/^\d+(?:(?:,\d+)+|:\d+)$/';
-            $suffix = '() did not appear to be a valid message id range or sequence!';
+            if (!self::IsValidSequenceSet($msg_number)) {
+                throw new InvalidArgumentException('Argument '.(string) $argument.' passed to '.$method.'() did not appear to be a valid message id range or sequence!');
+            }
+
+            return $msg_number;
         }
 
-        if (\is_int($msg_number) || \preg_match('/^\d+$/', $msg_number)) {
-            return \sprintf('%1$s:%1$s', $msg_number);
-        } elseif (1 !== \preg_match($regex, $msg_number)) {
-            throw new InvalidArgumentException('Argument '.(string) $argument.' passed to '.$method.$suffix);
+        if (1 !== \preg_match('/^\d+:\d+$/', $msg_number)) {
+            throw new InvalidArgumentException('Argument '.(string) $argument.' passed to '.$method.'() did not appear to be a valid message id range!');
         }
 
         return $msg_number;
     }
+
+    /**
+     * RFC 3501 sequence-set elements are message numbers or "*", optionally as ranges, comma-separated.
+     *
+     * @psalm-pure
+     */
+    private static function IsValidSequenceSet(string $msg_number): bool
+    {
+        return 1 === \preg_match(
+            '/^(?:\*|\d+)(?::(?:\*|\d+))?(?:,(?:\*|\d+)(?::(?:\*|\d+))?)*$/',
+            $msg_number
+        );
+    }
 }

tests/unit/ImapSequenceSetTest.php

@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpImap;
+
+use InvalidArgumentException;
+use PHPUnit\Framework\TestCase;
+
+final class ImapSequenceSetTest extends TestCase
+{
+    /**
+     * @return array<string, array{0:string}>
+     */
+    public function validSequenceSetProvider(): array
+    {
+        return [
+            'wildcard only' => ['*'],
+            'numeric range' => ['1:5'],
+            'range ending with wildcard' => ['1:*'],
+            'range starting with wildcard' => ['*:5'],
+            'wildcard range' => ['*:*'],
+            'comma separated ids' => ['4,5,6'],
+            'comma separated mixed sequence set' => ['2,4:7,9,12:*'],
+            'wildcard as comma item' => ['1,*'],
+            'wildcard range in sequence set' => ['1,3:*,5'],
+        ];
+    }
+
+    /**
+     * @dataProvider validSequenceSetProvider
+     */
+    public function testEnsureRangeAcceptsValidSequenceSetsWhenAllowed(string $msgNumber): void
+    {
+        $this->assertSame($msgNumber, $this->ensureRangeForTests($msgNumber, true));
+    }
+
+    public function testEnsureRangeNormalizesSingleMessageIdsWhenSequenceSetsAreAllowed(): void
+    {
+        $this->assertSame('123:123', $this->ensureRangeForTests('123', true));
+    }
+
+    /**
+     * @return array<string, array{0:string}>
+     */
+    public function invalidSequenceSetProvider(): array
+    {
+        return [
+            'empty string' => [''],
+            'leading comma' => [',1:5'],
+            'trailing comma' => ['1:5,'],
+            'double comma' => ['1,,5'],
+            'double colon' => ['1::5'],
+            'non numeric token' => ['foo'],
+            'wildcard in malformed position' => ['2,4:7,9,12:**'],
+        ];
+    }
+
+    /**
+     * @dataProvider invalidSequenceSetProvider
+     */
+    public function testEnsureRangeRejectsInvalidSequenceSetsWhenAllowed(string $msgNumber): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('did not appear to be a valid message id range or sequence');
+
+        $this->ensureRangeForTests($msgNumber, true);
+    }
+
+    public function testEnsureRangeRejectsWildcardsWhenOnlyRangesAreAllowed(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('did not appear to be a valid message id range');
+
+        $this->ensureRangeForTests('1:*');
+    }
+
+    private function ensureRangeForTests(int|string $msgNumber, bool $allowSequence = false): string
+    {
+        $ensureRange = \Closure::bind(
+            static function (int|string $msgNumber, bool $allowSequence): string {
+                return Imap::EnsureRange($msgNumber, __METHOD__, 1, $allowSequence);
+            },
+            null,
+            Imap::class
+        );
+
+        if (!$ensureRange instanceof \Closure) {
+            throw new \RuntimeException('Could not bind EnsureRange() test helper.');
+        }
+
+        return $ensureRange($msgNumber, $allowSequence);
+    }
+}