From 690d9cd45fb38d42380ce4a05e20fe58620cf91c Mon Sep 17 00:00:00 2001
From: Jaapio <jaap@ijaap.nl>
Date: Tue, 21 Jul 2020 22:14:17 +0200
Subject: [PATCH] Introduce safe preg_split

Removes the need for extra assertions which makes the code
more readable, and reduces the overhead of an is_array check.
---
 src/DocBlock/DescriptionFactory.php        |  8 +--
 src/DocBlock/Tags/Covers.php               |  5 +-
 src/DocBlock/Tags/Link.php                 |  5 +-
 src/DocBlock/Tags/Param.php                |  5 +-
 src/DocBlock/Tags/Property.php             |  5 +-
 src/DocBlock/Tags/PropertyRead.php         |  5 +-
 src/DocBlock/Tags/PropertyWrite.php        |  5 +-
 src/DocBlock/Tags/See.php                  |  5 +-
 src/DocBlock/Tags/Uses.php                 |  6 +-
 src/DocBlock/Tags/Var_.php                 |  5 +-
 src/Exception/PcreException.php            | 38 +++++++++++++
 src/Utils.php                              | 57 +++++++++++++++++++
 tests/unit/Exception/PcreExceptionTest.php | 66 ++++++++++++++++++++++
 tests/unit/PregSplitTest.php               | 48 ++++++++++++++++
 14 files changed, 229 insertions(+), 34 deletions(-)
 create mode 100644 src/Exception/PcreException.php
 create mode 100644 src/Utils.php
 create mode 100644 tests/unit/Exception/PcreExceptionTest.php
 create mode 100644 tests/unit/PregSplitTest.php

diff --git a/src/DocBlock/DescriptionFactory.php b/src/DocBlock/DescriptionFactory.php
index 0501c3c..c27d2a0 100644
--- a/src/DocBlock/DescriptionFactory.php
+++ b/src/DocBlock/DescriptionFactory.php
@@ -14,13 +14,12 @@ declare(strict_types=1);
 namespace phpDocumentor\Reflection\DocBlock;
 
 use phpDocumentor\Reflection\Types\Context as TypeContext;
-use Webmozart\Assert\Assert;
+use phpDocumentor\Reflection\Utils;
 use function count;
 use function explode;
 use function implode;
 use function ltrim;
 use function min;
-use function preg_split;
 use function str_replace;
 use function strlen;
 use function strpos;
@@ -98,7 +97,7 @@ class DescriptionFactory
             return [$contents];
         }
 
-        $parts =  preg_split(
+        return Utils::pregSplit(
             '/\{
                 # "{@}" is not a valid inline tag. This ensures that we do not treat it as one, but treat it literally.
                 (?!@\})
@@ -127,9 +126,6 @@ class DescriptionFactory
             0,
             PREG_SPLIT_DELIM_CAPTURE
         );
-        Assert::isArray($parts);
-
-        return $parts;
     }
 
     /**
diff --git a/src/DocBlock/Tags/Covers.php b/src/DocBlock/Tags/Covers.php
index 77edf4a..820d595 100644
--- a/src/DocBlock/Tags/Covers.php
+++ b/src/DocBlock/Tags/Covers.php
@@ -18,8 +18,8 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Fqsen;
 use phpDocumentor\Reflection\FqsenResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
-use function preg_split;
 
 /**
  * Reflection class for a @covers tag in a Docblock.
@@ -51,8 +51,7 @@ final class Covers extends BaseTag implements Factory\StaticMethod
         Assert::notNull($descriptionFactory);
         Assert::notNull($resolver);
 
-        $parts = preg_split('/\s+/Su', $body, 2);
-        Assert::isArray($parts);
+        $parts = Utils::pregSplit('/\s+/Su', $body, 2);
 
         return new static(
             $resolver->resolve($parts[0], $context),
diff --git a/src/DocBlock/Tags/Link.php b/src/DocBlock/Tags/Link.php
index 0588f72..7cee2d6 100644
--- a/src/DocBlock/Tags/Link.php
+++ b/src/DocBlock/Tags/Link.php
@@ -16,8 +16,8 @@ namespace phpDocumentor\Reflection\DocBlock\Tags;
 use phpDocumentor\Reflection\DocBlock\Description;
 use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
-use function preg_split;
 
 /**
  * Reflection class for a @link tag in a Docblock.
@@ -46,8 +46,7 @@ final class Link extends BaseTag implements Factory\StaticMethod
     ) : self {
         Assert::notNull($descriptionFactory);
 
-        $parts = preg_split('/\s+/Su', $body, 2);
-        Assert::isArray($parts);
+        $parts = Utils::pregSplit('/\s+/Su', $body, 2);
         $description = isset($parts[1]) ? $descriptionFactory->create($parts[1], $context) : null;
 
         return new static($parts[0], $description);
diff --git a/src/DocBlock/Tags/Param.php b/src/DocBlock/Tags/Param.php
index 7f94361..2e5daf7 100644
--- a/src/DocBlock/Tags/Param.php
+++ b/src/DocBlock/Tags/Param.php
@@ -18,11 +18,11 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\TypeResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function array_shift;
 use function array_unshift;
 use function implode;
-use function preg_split;
 use function strpos;
 use function substr;
 use const PREG_SPLIT_DELIM_CAPTURE;
@@ -64,8 +64,7 @@ final class Param extends TagWithType implements Factory\StaticMethod
         [$firstPart, $body] = self::extractTypeFromBody($body);
 
         $type         = null;
-        $parts        = preg_split('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
-        Assert::isArray($parts);
+        $parts        = Utils::pregSplit('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
         $variableName = '';
         $isVariadic   = false;
 
diff --git a/src/DocBlock/Tags/Property.php b/src/DocBlock/Tags/Property.php
index 0da0233..d8b83b6 100644
--- a/src/DocBlock/Tags/Property.php
+++ b/src/DocBlock/Tags/Property.php
@@ -18,11 +18,11 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\TypeResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function array_shift;
 use function array_unshift;
 use function implode;
-use function preg_split;
 use function strpos;
 use function substr;
 use const PREG_SPLIT_DELIM_CAPTURE;
@@ -57,8 +57,7 @@ final class Property extends TagWithType implements Factory\StaticMethod
 
         [$firstPart, $body] = self::extractTypeFromBody($body);
         $type               = null;
-        $parts              = preg_split('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
-        Assert::isArray($parts);
+        $parts              = Utils::pregSplit('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
         $variableName = '';
 
         // if the first item that is encountered is not a variable; it is a type
diff --git a/src/DocBlock/Tags/PropertyRead.php b/src/DocBlock/Tags/PropertyRead.php
index af768fb..087803c 100644
--- a/src/DocBlock/Tags/PropertyRead.php
+++ b/src/DocBlock/Tags/PropertyRead.php
@@ -18,11 +18,11 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\TypeResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function array_shift;
 use function array_unshift;
 use function implode;
-use function preg_split;
 use function strpos;
 use function substr;
 use const PREG_SPLIT_DELIM_CAPTURE;
@@ -57,8 +57,7 @@ final class PropertyRead extends TagWithType implements Factory\StaticMethod
 
         [$firstPart, $body] = self::extractTypeFromBody($body);
         $type               = null;
-        $parts              = preg_split('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
-        Assert::isArray($parts);
+        $parts              = Utils::pregSplit('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
         $variableName = '';
 
         // if the first item that is encountered is not a variable; it is a type
diff --git a/src/DocBlock/Tags/PropertyWrite.php b/src/DocBlock/Tags/PropertyWrite.php
index 34cd75f..176b63d 100644
--- a/src/DocBlock/Tags/PropertyWrite.php
+++ b/src/DocBlock/Tags/PropertyWrite.php
@@ -18,11 +18,11 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\TypeResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function array_shift;
 use function array_unshift;
 use function implode;
-use function preg_split;
 use function strpos;
 use function substr;
 use const PREG_SPLIT_DELIM_CAPTURE;
@@ -57,8 +57,7 @@ final class PropertyWrite extends TagWithType implements Factory\StaticMethod
 
         [$firstPart, $body] = self::extractTypeFromBody($body);
         $type               = null;
-        $parts              = preg_split('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
-        Assert::isArray($parts);
+        $parts              = Utils::pregSplit('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
         $variableName = '';
 
         // if the first item that is encountered is not a variable; it is a type
diff --git a/src/DocBlock/Tags/See.php b/src/DocBlock/Tags/See.php
index 190973d..e71401d 100644
--- a/src/DocBlock/Tags/See.php
+++ b/src/DocBlock/Tags/See.php
@@ -20,9 +20,9 @@ use phpDocumentor\Reflection\DocBlock\Tags\Reference\Reference;
 use phpDocumentor\Reflection\DocBlock\Tags\Reference\Url;
 use phpDocumentor\Reflection\FqsenResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function preg_match;
-use function preg_split;
 
 /**
  * Reflection class for an {@}see tag in a Docblock.
@@ -53,8 +53,7 @@ final class See extends BaseTag implements Factory\StaticMethod
         Assert::notNull($typeResolver);
         Assert::notNull($descriptionFactory);
 
-        $parts = preg_split('/\s+/Su', $body, 2);
-        Assert::isArray($parts);
+        $parts = Utils::pregSplit('/\s+/Su', $body, 2);
         $description = isset($parts[1]) ? $descriptionFactory->create($parts[1], $context) : null;
 
         // https://tools.ietf.org/html/rfc2396#section-3
diff --git a/src/DocBlock/Tags/Uses.php b/src/DocBlock/Tags/Uses.php
index 1be71f6..57fb290 100644
--- a/src/DocBlock/Tags/Uses.php
+++ b/src/DocBlock/Tags/Uses.php
@@ -18,8 +18,8 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Fqsen;
 use phpDocumentor\Reflection\FqsenResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
-use function preg_split;
 
 /**
  * Reflection class for a {@}uses tag in a Docblock.
@@ -50,9 +50,7 @@ final class Uses extends BaseTag implements Factory\StaticMethod
         Assert::notNull($resolver);
         Assert::notNull($descriptionFactory);
 
-        $parts = preg_split('/\s+/Su', $body, 2);
-        Assert::isArray($parts);
-        Assert::allString($parts);
+        $parts = Utils::pregSplit('/\s+/Su', $body, 2);
 
         return new static(
             $resolver->resolve($parts[0], $context),
diff --git a/src/DocBlock/Tags/Var_.php b/src/DocBlock/Tags/Var_.php
index e03f994..ac1c438 100644
--- a/src/DocBlock/Tags/Var_.php
+++ b/src/DocBlock/Tags/Var_.php
@@ -18,11 +18,11 @@ use phpDocumentor\Reflection\DocBlock\DescriptionFactory;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\TypeResolver;
 use phpDocumentor\Reflection\Types\Context as TypeContext;
+use phpDocumentor\Reflection\Utils;
 use Webmozart\Assert\Assert;
 use function array_shift;
 use function array_unshift;
 use function implode;
-use function preg_split;
 use function strpos;
 use function substr;
 use const PREG_SPLIT_DELIM_CAPTURE;
@@ -57,8 +57,7 @@ final class Var_ extends TagWithType implements Factory\StaticMethod
 
         [$firstPart, $body] = self::extractTypeFromBody($body);
 
-        $parts = preg_split('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
-        Assert::isArray($parts);
+        $parts = Utils::pregSplit('/(\s+)/Su', $body, 2, PREG_SPLIT_DELIM_CAPTURE);
         $type         = null;
         $variableName = '';
 
diff --git a/src/Exception/PcreException.php b/src/Exception/PcreException.php
new file mode 100644
index 0000000..77aa40e
--- /dev/null
+++ b/src/Exception/PcreException.php
@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection\Exception;
+
+use InvalidArgumentException;
+use const PREG_BACKTRACK_LIMIT_ERROR;
+use const PREG_BAD_UTF8_ERROR;
+use const PREG_BAD_UTF8_OFFSET_ERROR;
+use const PREG_INTERNAL_ERROR;
+use const PREG_JIT_STACKLIMIT_ERROR;
+use const PREG_NO_ERROR;
+use const PREG_RECURSION_LIMIT_ERROR;
+
+final class PcreException extends InvalidArgumentException
+{
+    public static function createFromPhpError(int $errorCode) : self
+    {
+        switch ($errorCode) {
+            case PREG_BACKTRACK_LIMIT_ERROR:
+                return new self('Backtrack limit error');
+            case PREG_RECURSION_LIMIT_ERROR:
+                return new self('Recursion limit error');
+            case PREG_BAD_UTF8_ERROR:
+                return new self('Bad UTF8 error');
+            case PREG_BAD_UTF8_OFFSET_ERROR:
+                return new self('Bad UTF8 offset error');
+            case PREG_JIT_STACKLIMIT_ERROR:
+                return new self('Jit stacklimit error');
+            case PREG_NO_ERROR:
+            case PREG_INTERNAL_ERROR:
+            default:
+        }
+
+        return new self('Unknown Pcre error');
+    }
+}
diff --git a/src/Utils.php b/src/Utils.php
new file mode 100644
index 0000000..d9b790b
--- /dev/null
+++ b/src/Utils.php
@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of phpDocumentor.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @link      http://phpdoc.org
+ */
+
+namespace phpDocumentor\Reflection;
+
+use phpDocumentor\Reflection\Exception\PcreException;
+use function preg_last_error;
+use function preg_split as php_preg_split;
+
+abstract class Utils
+{
+    /**
+     * Wrapper function for phps preg_split
+     *
+     * This function is inspired by {@link https://github.com/thecodingmachine/safe/blob/master/generated/pcre.php}. But
+     * since this library is all about performance we decided to strip everything we don't need. Reducing the amount
+     * of files that have to be loaded, ect.
+     *
+     * @param string $pattern The pattern to search for, as a string.
+     * @param string $subject The input string.
+     * @param int|null $limit If specified, then only substrings up to limit are returned with the
+     *      rest of the string being placed in the last substring. A limit of -1 or 0 means "no limit".
+     * @param int $flags flags can be any combination of the following flags (combined with the | bitwise operator):
+     * *PREG_SPLIT_NO_EMPTY*
+     *      If this flag is set, only non-empty pieces will be returned by preg_split().
+     * *PREG_SPLIT_DELIM_CAPTURE*
+     *      If this flag is set, parenthesized expression in the delimiter pattern will be captured
+     *      and returned as well.
+     * *PREG_SPLIT_OFFSET_CAPTURE*
+     *      If this flag is set, for every occurring match the appendant string offset will also be returned.
+     *      Note that this changes the return value in an array where every element is an array consisting of the
+     *      matched string at offset 0 and its string offset into subject at offset 1.
+     *
+     * @return string[] Returns an array containing substrings of subject split along boundaries matched by pattern
+     *
+     * @throws PcreException
+     */
+    public static function pregSplit(string $pattern, string $subject, ?int $limit = -1, int $flags = 0) : array
+    {
+        $parts = php_preg_split($pattern, $subject, $limit, $flags);
+        if ($parts === false) {
+            throw PcreException::createFromPhpError(preg_last_error());
+        }
+
+        return $parts;
+    }
+}
diff --git a/tests/unit/Exception/PcreExceptionTest.php b/tests/unit/Exception/PcreExceptionTest.php
new file mode 100644
index 0000000..f583300
--- /dev/null
+++ b/tests/unit/Exception/PcreExceptionTest.php
@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection\Exception;
+
+use PHPUnit\Framework\TestCase;
+use const PREG_BACKTRACK_LIMIT_ERROR;
+use const PREG_BAD_UTF8_ERROR;
+use const PREG_BAD_UTF8_OFFSET_ERROR;
+use const PREG_INTERNAL_ERROR;
+use const PREG_JIT_STACKLIMIT_ERROR;
+use const PREG_NO_ERROR;
+use const PREG_RECURSION_LIMIT_ERROR;
+
+/**
+ * @coversDefaultClass \phpDocumentor\Reflection\Exception\PcreException
+ */
+final class PcreExceptionTest extends TestCase
+{
+    /**
+     * @covers ::createFromPhpError
+     * @dataProvider errorCodeProvider
+     */
+    public function testErrorConversion(int $errorCode, string $message) : void
+    {
+        $this->assertSame($message, PcreException::createFromPhpError($errorCode)->getMessage());
+    }
+
+    /**
+     * @return array<int, (string|int)[]>
+     */
+    public function errorCodeProvider() : array
+    {
+        return [
+            [
+                PREG_BACKTRACK_LIMIT_ERROR,
+                'Backtrack limit error',
+            ],
+            [
+                PREG_RECURSION_LIMIT_ERROR,
+                'Recursion limit error',
+            ],
+            [
+                PREG_BAD_UTF8_ERROR,
+                'Bad UTF8 error',
+            ],
+            [
+                PREG_BAD_UTF8_OFFSET_ERROR,
+                'Bad UTF8 offset error',
+            ],
+            [
+                PREG_JIT_STACKLIMIT_ERROR,
+                'Jit stacklimit error',
+            ],
+            [
+                PREG_NO_ERROR,
+                'Unknown Pcre error',
+            ],
+            [
+                PREG_INTERNAL_ERROR,
+                'Unknown Pcre error',
+            ],
+        ];
+    }
+}
diff --git a/tests/unit/PregSplitTest.php b/tests/unit/PregSplitTest.php
new file mode 100644
index 0000000..ef7a458
--- /dev/null
+++ b/tests/unit/PregSplitTest.php
@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection;
+
+use phpDocumentor\Reflection\Exception\PcreException;
+use PHPUnit\Framework\TestCase;
+use function set_error_handler;
+use const E_WARNING;
+
+final class PregSplitTest extends TestCase
+{
+    /** @var callable|null */
+    private $errorHandler = null;
+
+    protected function tearDown() : void
+    {
+        if ($this->errorHandler === null) {
+            return;
+        }
+
+        set_error_handler($this->errorHandler, E_WARNING);
+    }
+
+    /**
+     * @covers \phpDocumentor\Reflection\Utils::pregSplit
+     */
+    public function testSimplePregSplit() : void
+    {
+        $result = Utils::pregSplit('/\s/', 'word split');
+
+        $this->assertSame(['word', 'split'], $result);
+    }
+
+    /**
+     * @covers \phpDocumentor\Reflection\Utils::pregSplit
+     */
+    public function testPregSplitThrowsOnError() : void
+    {
+        //We need to disable the error handler for phpunit... because we expect some errors here
+        $this->errorHandler = set_error_handler(static function () : void {
+        }, E_WARNING);
+
+        $this->expectException(PcreException::class);
+        Utils::pregSplit('~InvalidRegular)Expression~', 'some word');
+    }
+}