Add phpstan, fix types

nelmio · Jun 26, 2024 · a4be623 · a4be623
1 parent 3a526fe
commit a4be623
Show file tree

Hide file tree

Showing 19 changed files with 301 additions and 164 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -1,3 +1,4 @@
 .* export-ignore
 Tests export-ignore
 phpunit.xml.dist export-ignore
+phpstan.neon.dist export-ignore
diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml
@@ -0,0 +1,47 @@
+name: "PHPStan"
+
+on:
+  - push
+  - pull_request
+
+permissions:
+  contents: read
+
+jobs:
+  tests:
+    name: "PHPStan"
+
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        php-version:
+          - "8.3"
+
+    steps:
+      - name: "Checkout"
+        uses: "actions/checkout@v4"
+
+      - name: "Install PHP"
+        uses: "shivammathur/setup-php@v2"
+        with:
+          coverage: "none"
+          php-version: "${{ matrix.php-version }}"
+
+      - name: Get composer cache directory
+        id: composercache
+        run: echo "::set-output name=dir::$(composer config cache-files-dir)"
+
+      - name: Cache dependencies
+        uses: actions/cache@v2
+        with:
+          path: ${{ steps.composercache.outputs.dir }}
+          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
+          restore-keys: ${{ runner.os }}-composer-
+
+      - name: "Install latest dependencies"
+        run: "composer update"
+
+      - name: Run PHPStan
+        run: |
+          vendor/bin/phpstan analyse
diff --git a/DependencyInjection/Compiler/CorsConfigurationProviderPass.php b/DependencyInjection/Compiler/CorsConfigurationProviderPass.php
@@ -29,7 +29,7 @@ public function process(ContainerBuilder $container): void
         $optionsProvidersByPriority = [];
         foreach ($container->findTaggedServiceIds('nelmio_cors.options_provider') as $taggedServiceId => $tagAttributes) {
             foreach ($tagAttributes as $attribute) {
-                $priority = isset($attribute['priority']) ? $attribute['priority'] : 0;
+                $priority = isset($attribute['priority']) ? (int) $attribute['priority'] : 0;
                 $optionsProvidersByPriority[$priority][] = new Reference($taggedServiceId);
             }
         }
@@ -43,7 +43,7 @@ public function process(ContainerBuilder $container): void
 
     /**
      * Transforms a two-dimensions array of providers, indexed by priority, into a flat array of Reference objects
-     * @param  array       $providersByPriority
+     * @param  array<int, list<Reference>> $providersByPriority
      * @return Reference[]
      */
     protected function sortProviders(array $providersByPriority): array

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -29,13 +29,7 @@ public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('nelmio_cors');
 
-        if (method_exists($treeBuilder, 'getRootNode')) {
-            $rootNode = $treeBuilder->getRootNode();
-        } else {
-            // BC for symfony/config < 4.2
-            $rootNode = $treeBuilder->root('nelmio_cors');
-        }
-
+        $rootNode = $treeBuilder->getRootNode();
         $rootNode
             ->children()
                 ->arrayNode('defaults')

diff --git a/DependencyInjection/NelmioCorsExtension.php b/DependencyInjection/NelmioCorsExtension.php
@@ -21,6 +21,19 @@
  */
 class NelmioCorsExtension extends Extension
 {
+    public const DEFAULTS = [
+        'allow_origin' => [],
+        'allow_credentials' => false,
+        'allow_headers' => [],
+        'allow_private_network' => false,
+        'expose_headers' => [],
+        'allow_methods' => [],
+        'max_age' => 0,
+        'hosts' => [],
+        'origin_regex' => false,
+        'skip_same_as_origin' => true,
+    ];
+
     /**
      * {@inheritDoc}
      */
@@ -29,20 +42,7 @@ public function load(array $configs, ContainerBuilder $container): void
         $configuration = new Configuration();
         $config = $this->processConfiguration($configuration, $configs);
 
-        $defaults = array_merge(
-            [
-                'allow_origin' => [],
-                'allow_credentials' => false,
-                'allow_headers' => [],
-                'allow_private_network' => false,
-                'expose_headers' => [],
-                'allow_methods' => [],
-                'max_age' => 0,
-                'hosts' => [],
-                'origin_regex' => false,
-            ],
-            $config['defaults']
-        );
+        $defaults = array_merge(self::DEFAULTS, $config['defaults']);
 
         if ($defaults['allow_credentials'] && in_array('*', $defaults['expose_headers'], true)) {
             throw new \UnexpectedValueException('nelmio_cors expose_headers cannot contain a wildcard (*) when allow_credentials is enabled.');

diff --git a/EventListener/CacheableResponseVaryListener.php b/EventListener/CacheableResponseVaryListener.php
@@ -9,7 +9,7 @@
  */
 final class CacheableResponseVaryListener
 {
-    public function onResponse(ResponseEvent $event)
+    public function onResponse(ResponseEvent $event): void
     {
         $response = $event->getResponse();
 

diff --git a/EventListener/CorsListener.php b/EventListener/CorsListener.php
@@ -12,6 +12,7 @@
 namespace Nelmio\CorsBundle\EventListener;
 
 use Nelmio\CorsBundle\Options\ResolverInterface;
+use Nelmio\CorsBundle\Options\ProviderInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Symfony\Component\HttpFoundation\Request;
@@ -24,16 +25,21 @@
  * Adds CORS headers and handles pre-flight requests
  *
  * @author Jordi Boggiano <j.boggiano@seld.be>
+ * @phpstan-import-type CorsCompleteOptions from ProviderInterface
  */
 class CorsListener
 {
-    const SHOULD_ALLOW_ORIGIN_ATTR = '_nelmio_cors_should_allow_origin';
-    const SHOULD_FORCE_ORIGIN_ATTR = '_nelmio_cors_should_force_origin';
+    public const SHOULD_ALLOW_ORIGIN_ATTR = '_nelmio_cors_should_allow_origin';
+    public const SHOULD_FORCE_ORIGIN_ATTR = '_nelmio_cors_should_force_origin';
 
     /**
      * Simple headers as defined in the spec should always be accepted
+     * @var list<string>
+     * @deprecated
      */
-    protected static $simpleHeaders = [
+    protected static $simpleHeaders = self::SIMPLE_HEADERS;
+
+    protected const SIMPLE_HEADERS = [
         'accept',
         'accept-language',
         'content-language',
@@ -66,6 +72,7 @@ public function onKernelRequest(RequestEvent $event): void
 
         $request = $event->getRequest();
 
+        // @phpstan-ignore booleanNot.alwaysFalse (an invalid overridden configuration resolver may not be trustworthy)
         if (!$options = $this->configurationResolver->getOptions($request)) {
             $this->logger->debug('Could not get options for request, skipping CORS checks.');
             return;
@@ -136,6 +143,7 @@ public function onKernelResponse(ResponseEvent $event): void
             return;
         }
 
+        // @phpstan-ignore booleanNot.alwaysFalse (an invalid overridden configuration resolver may not be trustworthy)
         if (!$options = $this->configurationResolver->getOptions($request)) {
             $this->logger->debug("Could not resolve options for request, skip adding CORS response headers.");
 
@@ -166,12 +174,16 @@ public function onKernelResponse(ResponseEvent $event): void
         }
 
         if ($shouldForceOrigin) {
+            assert(isset($options['forced_allow_origin_value']));
             $this->logger->debug(sprintf("Setting 'Access-Control-Allow-Origin' response header to '%s'.", $options['forced_allow_origin_value']));
 
             $event->getResponse()->headers->set('Access-Control-Allow-Origin', $options['forced_allow_origin_value']);
         }
     }
 
+    /**
+     * @phpstan-param CorsCompleteOptions $options
+     */
     protected function getPreflightResponse(Request $request, array $options): Response
     {
         $response = new Response();
@@ -192,7 +204,7 @@ protected function getPreflightResponse(Request $request, array $options): Respo
         if ($options['allow_headers']) {
             $headers = $this->isWildcard($options, 'allow_headers')
                 ? $request->headers->get('Access-Control-Request-Headers')
-                : implode(', ', $options['allow_headers']);
+                : implode(', ', $options['allow_headers']); // @phpstan-ignore argument.type (isWildcard guarantees this is an array but PHPStan does not know)
 
             if ($headers) {
                 $this->logger->debug(sprintf("Setting 'Access-Control-Allow-Headers' response header to '%s'.", $headers));
@@ -203,7 +215,7 @@ protected function getPreflightResponse(Request $request, array $options): Respo
         if ($options['max_age']) {
             $this->logger->debug(sprintf("Setting 'Access-Control-Max-Age' response header to '%d'.", $options['max_age']));
 
-            $response->headers->set('Access-Control-Max-Age', $options['max_age']);
+            $response->headers->set('Access-Control-Max-Age', (string) $options['max_age']);
         }
 
         if (!$this->checkOrigin($request, $options)) {
@@ -222,7 +234,7 @@ protected function getPreflightResponse(Request $request, array $options): Respo
 
         // check private network access
         if ($request->headers->has('Access-Control-Request-Private-Network')
-            && strtolower($request->headers->get('Access-Control-Request-Private-Network')) === 'true'
+            && strtolower((string) $request->headers->get('Access-Control-Request-Private-Network')) === 'true'
         ) {
             if ($options['allow_private_network']) {
                 $this->logger->debug("Setting 'Access-Control-Allow-Private-Network' response header to 'true'.");
@@ -235,7 +247,7 @@ protected function getPreflightResponse(Request $request, array $options): Respo
         }
 
         // check request method
-        $method = strtoupper($request->headers->get('Access-Control-Request-Method'));
+        $method = strtoupper((string) $request->headers->get('Access-Control-Request-Method'));
         if (!in_array($method, $options['allow_methods'], true)) {
             $this->logger->debug(sprintf("Method '%s' is not allowed.", $method));
 
@@ -250,19 +262,23 @@ protected function getPreflightResponse(Request $request, array $options): Respo
          * request.
          */
         if (!in_array($request->headers->get('Access-Control-Request-Method'), $options['allow_methods'], true)) {
-            $options['allow_methods'][] = $request->headers->get('Access-Control-Request-Method');
+            $options['allow_methods'][] = (string) $request->headers->get('Access-Control-Request-Method');
             $response->headers->set('Access-Control-Allow-Methods', implode(', ', $options['allow_methods']));
         }
 
         // check request headers
         $headers = $request->headers->get('Access-Control-Request-Headers');
         if ($headers && !$this->isWildcard($options, 'allow_headers')) {
             $headers = strtolower(trim($headers));
-            foreach (preg_split('{, *}', $headers) as $header) {
-                if (in_array($header, self::$simpleHeaders, true)) {
+            $splitHeaders = preg_split('{, *}', $headers);
+            if (false === $splitHeaders) {
+                throw new \RuntimeException('Failed splitting '.$headers);
+            }
+            foreach ($splitHeaders as $header) {
+                if (in_array($header, self::SIMPLE_HEADERS, true)) {
                     continue;
                 }
-                if (!in_array($header, $options['allow_headers'], true)) {
+                if (!in_array($header, $options['allow_headers'], true)) { // @phpstan-ignore argument.type (isWildcard guarantees this is an array but PHPStan does not know)
                     $sanitizedMessage = htmlentities('Unauthorized header '.$header, ENT_QUOTES, 'UTF-8');
                     $response->setStatusCode(400);
                     $response->setContent($sanitizedMessage);
@@ -274,18 +290,21 @@ protected function getPreflightResponse(Request $request, array $options): Respo
         return $response;
     }
 
+    /**
+     * @param CorsCompleteOptions $options
+     */
     protected function checkOrigin(Request $request, array $options): bool
     {
         // check origin
-        $origin = $request->headers->get('Origin');
+        $origin = (string) $request->headers->get('Origin');
 
         if ($this->isWildcard($options, 'allow_origin')) {
             return true;
         }
 
         if ($options['origin_regex'] === true) {
             // origin regex matching
-            foreach ($options['allow_origin'] as $originRegexp) {
+            foreach ($options['allow_origin'] as $originRegexp) { // @phpstan-ignore foreach.nonIterable (isWildcard guarantees this is an array but PHPStan does not know)
                 $this->logger->debug(sprintf("Matching origin regex '%s' to origin '%s'.", $originRegexp, $origin));
 
                 if (preg_match('{'.$originRegexp.'}i', $origin)) {
@@ -296,7 +315,7 @@ protected function checkOrigin(Request $request, array $options): bool
             }
         } else {
             // old origin matching
-            if (in_array($origin, $options['allow_origin'])) {
+            if (in_array($origin, $options['allow_origin'], true)) { // @phpstan-ignore argument.type (isWildcard guarantees this is an array but PHPStan does not know)
                 $this->logger->debug(sprintf("Origin '%s' is allowed.", $origin));
 
                 return true;
@@ -308,9 +327,13 @@ protected function checkOrigin(Request $request, array $options): bool
         return false;
     }
 
+    /**
+     * @phpstan-param CorsCompleteOptions $options
+     * @phpstan-param key-of<CorsCompleteOptions> $option
+     */
     private function isWildcard(array $options, string $option): bool
     {
-        $result = $options[$option] === true || (is_array($options[$option]) && in_array('*', $options[$option]));
+        $result = $options[$option] === true || (is_array($options[$option]) && in_array('*', $options[$option], true));
 
         $this->logger->debug(sprintf("Option '%s' is %s a wildcard.", $option, $result ? '' : 'not'));
 

diff --git a/Options/ConfigProvider.php b/Options/ConfigProvider.php
@@ -11,21 +11,37 @@
 namespace Nelmio\CorsBundle\Options;
 
 use Symfony\Component\HttpFoundation\Request;
+use Nelmio\CorsBundle\DependencyInjection\NelmioCorsExtension;
 
 /**
  * Default CORS configuration provider.
  *
  * Uses the bundle's semantic configuration.
  * Default settings are the lowest priority one, and can be relied upon.
+ *
+ * @phpstan-import-type CorsCompleteOptions from ProviderInterface
+ * @phpstan-import-type CorsOptionsPerPath from ProviderInterface
  */
 class ConfigProvider implements ProviderInterface
 {
+    /**
+     * @var CorsOptionsPerPath
+     */
     protected $paths;
+    /**
+     * @var array<string, bool|array<string>|int>
+     * @phpstan-var CorsCompleteOptions
+     */
     protected $defaults;
 
-    public function __construct(array $paths, array $defaults = [])
+    /**
+     * @param CorsOptionsPerPath $paths
+     * @param array<string, bool|array<string>|int> $defaults
+     * @phpstan-param CorsCompleteOptions $defaults
+     */
+    public function __construct(array $paths, ?array $defaults = null)
     {
-        $this->defaults = $defaults;
+        $this->defaults = $defaults === null ? NelmioCorsExtension::DEFAULTS : $defaults;
         $this->paths = $paths;
     }
 

diff --git a/Options/ProviderInterface.php b/Options/ProviderInterface.php
@@ -16,6 +16,10 @@
  * CORS configuration provider interface.
  *
  * Can override CORS options for a particular path.
+ *
+ * @phpstan-type CorsOptions array{hosts?: list<string>, allow_credentials?: bool, allow_origin?: bool|list<string>, allow_headers?: bool|list<string>, allow_private_network?: bool, origin_regex?: bool, allow_methods?: list<string>, expose_headers?: list<string>, max_age?: int, forced_allow_origin_value?: string, skip_same_as_origin?: bool}
+ * @phpstan-type CorsCompleteOptions array{hosts: list<string>, allow_credentials: bool, allow_origin: bool|list<string>, allow_headers: bool|list<string>, allow_private_network: bool, origin_regex: bool, allow_methods: list<string>, expose_headers: list<string>, max_age: int, forced_allow_origin_value?: string, skip_same_as_origin: bool}
+ * @phpstan-type CorsOptionsPerPath array<string, CorsOptions>
  */
 interface ProviderInterface
 {
@@ -35,7 +39,8 @@ interface ProviderInterface
      * - array expose_headers
      * - int max_age
      *
-     * @return array CORS options
+     * @return array<string, bool|array<string>|int> CORS options
+     * @phpstan-return CorsOptions
      */
     public function getOptions(Request $request): array;
 }