From 277ac8de1b1965250ddbe98b1788ee43afd038e0 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Sat, 19 Feb 2022 11:32:13 +0100 Subject: [PATCH] Phpstan level7 (#2) * Simplify array structure * Remove array merges in favor of explicit options * Split allowAll options * Tweak array type * Revert "Tweak array type" This reverts commit 4738baf14e10d5654f63e87fedaffc76347f9158. * Add explicit cast * Skip check on lowest symfony 4 * Skip check on lowest symfony 4 * Skip check on lowest symfony 4 * SKip * SKip --- .github/workflows/run-tests.yml | 2 +- composer.json | 2 +- src/CorsService.php | 109 +++++++++++++------------------- tests/CorsServiceTest.php | 2 +- tests/CorsTest.php | 18 ++---- tests/MockApp.php | 4 +- 6 files changed, 55 insertions(+), 82 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 64ee087..81756ab 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -46,7 +46,7 @@ jobs: - name: Analyse with PHPStan run: composer analyse - if: matrix.os == 'ubuntu-latest' + if: matrix.os == 'ubuntu-latest' && matrix.symfony != '4.x' - name: Check PSR-12 Codestyle run: composer test diff --git a/composer.json b/composer.json index c5d53d7..e821e29 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ }, "scripts": { "test": "phpunit", - "analyse": "phpstan analyse src tests --level=6", + "analyse": "phpstan analyse src tests --level=7", "check-style": "phpcs -p --standard=PSR12 --exclude=Generic.Files.LineLength --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 src tests", "fix-style": "phpcbf -p --standard=PSR12 --exclude=Generic.Files.LineLength --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 src tests" }, diff --git a/src/CorsService.php b/src/CorsService.php index c191fea..2005517 100644 --- a/src/CorsService.php +++ b/src/CorsService.php @@ -18,30 +18,33 @@ /** * @phpstan-type CorsInputOptions array{ - * 'allowedOrigins'?: array{string}|array{}, - * 'allowedOriginsPatterns'?: array{string}|array{}, + * 'allowedOrigins'?: string[], + * 'allowedOriginsPatterns'?: string[], * 'supportsCredentials'?: bool, - * 'allowedHeaders'?: array{string}|array{}, - * 'allowedMethods'?: array{string}|array{}, - * 'exposedHeaders'?: array{string}|array{}, + * 'allowedHeaders'?: string[], + * 'allowedMethods'?: string[], + * 'exposedHeaders'?: string[]|false, * 'maxAge'?: int|bool|null, - * 'allowed_origins'?: array{string}|array{}, - * 'allowed_origins_patterns'?: array{string}|array{}, + * 'allowed_origins'?: string[], + * 'allowed_origins_patterns'?: string[], * 'supports_credentials'?: bool, - * 'allowed_headers'?: array{string}|array{}, - * 'allowed_methods'?: array{string}|array{}, - * 'exposed_headers'?: array{string}|array{}, + * 'allowed_headers'?: string[], + * 'allowed_methods'?: string[], + * 'exposed_headers'?: string[]|false, * 'max_age'?: int|bool|null * } * * @phpstan-type CorsNormalizedOptions array{ - * 'allowedOrigins': array{string}|array{}|true, - * 'allowedOriginsPatterns': array{string}|array{}, + * 'allowedOrigins': string[], + * 'allowedOriginsPatterns': string[], * 'supportsCredentials': bool, - * 'allowedHeaders': array{string}|array{}|bool, - * 'allowedMethods': array{string}|array{}|bool, - * 'exposedHeaders': array{string}|array{}, - * 'maxAge': int|bool|null + * 'allowedHeaders': string[], + * 'allowedMethods': string[], + * 'exposedHeaders': string[], + * 'maxAge': int|bool|null, + * 'allowAllOrigins': bool, + * 'allowAllHeaders': bool, + * 'allowAllMethods': bool, * } */ class CorsService @@ -63,34 +66,18 @@ public function __construct(array $options = []) */ private function normalizeOptions(array $options = []): array { - $aliases = [ - 'supports_credentials' => 'supportsCredentials', - 'allowed_origins' => 'allowedOrigins', - 'allowed_origins_patterns' => 'allowedOriginsPatterns', - 'allowed_headers' => 'allowedHeaders', - 'allowed_methods' => 'allowedMethods', - 'exposed_headers' => 'exposedHeaders', - 'max_age' => 'maxAge', - ]; - - // Normalize underscores - foreach ($aliases as $alias => $option) { - if (isset($options[$alias])) { - $options[$option] = $options[$alias]; - unset($options[$alias]); - } + $options['allowedOrigins'] = $options['allowedOrigins'] ?? $options['allowed_origins'] ?? []; + $options['allowedOriginsPatterns'] = + $options['allowedOriginsPatterns'] ?? $options['allowed_origins_patterns'] ?? []; + $options['allowedMethods'] = $options['allowedMethods'] ?? $options['allowed_methods'] ?? []; + $options['allowedHeaders'] = $options['allowedHeaders'] ?? $options['allowed_headers'] ?? []; + $options['exposedHeaders'] = $options['exposedHeaders'] ?? $options['exposed_headers'] ?? []; + $options['supportsCredentials'] = $options['supportsCredentials'] ?? $options['supports_credentials'] ?? false; + + if (!array_key_exists('maxAge', $options)) { + $options['maxAge'] = array_key_exists('max_age', $options) ? $options['max_age'] : 0; } - $options += [ - 'allowedOrigins' => [], - 'allowedOriginsPatterns' => [], - 'supportsCredentials' => false, - 'allowedHeaders' => [], - 'exposedHeaders' => [], - 'allowedMethods' => [], - 'maxAge' => 0, - ]; - if ($options['exposedHeaders'] === false) { $options['exposedHeaders'] = []; } @@ -115,21 +102,14 @@ private function normalizeOptions(array $options = []): array } } - // normalize array('*') to true - if (in_array('*', $options['allowedOrigins'])) { - $options['allowedOrigins'] = true; - } - if (in_array('*', $options['allowedHeaders'])) { - $options['allowedHeaders'] = true; - } else { - $options['allowedHeaders'] = array_map('strtolower', $options['allowedHeaders']); - } + // Normalize case + $options['allowedHeaders'] = array_map('strtolower', $options['allowedHeaders']); + $options['allowedMethods'] = array_map('strtoupper', $options['allowedMethods']); - if (in_array('*', $options['allowedMethods'])) { - $options['allowedMethods'] = true; - } else { - $options['allowedMethods'] = array_map('strtoupper', $options['allowedMethods']); - } + // Normalize ['*'] to true + $options['allowAllOrigins'] = in_array('*', $options['allowedOrigins']); + $options['allowAllHeaders'] = in_array('*', $options['allowedHeaders']); + $options['allowAllMethods'] = in_array('*', $options['allowedMethods']); return $options; } @@ -191,7 +171,7 @@ public function addPreflightRequestHeaders(Response $response, Request $request) public function isOriginAllowed(Request $request): bool { - if ($this->options['allowedOrigins'] === true) { + if ($this->options['allowAllOrigins'] === true) { return true; } @@ -205,6 +185,7 @@ public function isOriginAllowed(Request $request): bool return true; } + /** @var string $pattern */ foreach ($this->options['allowedOriginsPatterns'] as $pattern) { if (preg_match($pattern, $origin)) { return true; @@ -229,7 +210,7 @@ public function addActualRequestHeaders(Response $response, Request $request): R private function configureAllowedOrigin(Response $response, Request $request): void { - if ($this->options['allowedOrigins'] === true && !$this->options['supportsCredentials']) { + if ($this->options['allowAllOrigins'] === true && !$this->options['supportsCredentials']) { // Safe+cacheable, allow everything $response->headers->set('Access-Control-Allow-Origin', '*'); } elseif ($this->isSingleOriginAllowed()) { @@ -247,7 +228,7 @@ private function configureAllowedOrigin(Response $response, Request $request): v private function isSingleOriginAllowed(): bool { - if ($this->options['allowedOrigins'] === true || count($this->options['allowedOriginsPatterns']) > 0) { + if ($this->options['allowAllOrigins'] === true || count($this->options['allowedOriginsPatterns']) > 0) { return false; } @@ -256,8 +237,8 @@ private function isSingleOriginAllowed(): bool private function configureAllowedMethods(Response $response, Request $request): void { - if ($this->options['allowedMethods'] === true) { - $allowMethods = strtoupper($request->headers->get('Access-Control-Request-Method')); + if ($this->options['allowAllMethods'] === true) { + $allowMethods = strtoupper((string) $request->headers->get('Access-Control-Request-Method')); $this->varyHeader($response, 'Access-Control-Request-Method'); } else { $allowMethods = implode(', ', $this->options['allowedMethods']); @@ -268,7 +249,7 @@ private function configureAllowedMethods(Response $response, Request $request): private function configureAllowedHeaders(Response $response, Request $request): void { - if ($this->options['allowedHeaders'] === true) { + if ($this->options['allowAllHeaders'] === true) { $allowHeaders = $request->headers->get('Access-Control-Request-Headers'); $this->varyHeader($response, 'Access-Control-Request-Headers'); } else { @@ -302,8 +283,8 @@ public function varyHeader(Response $response, string $header): Response { if (!$response->headers->has('Vary')) { $response->headers->set('Vary', $header); - } elseif (!in_array($header, explode(', ', $response->headers->get('Vary')))) { - $response->headers->set('Vary', $response->headers->get('Vary') . ', ' . $header); + } elseif (!in_array($header, explode(', ', (string) $response->headers->get('Vary')))) { + $response->headers->set('Vary', ((string) $response->headers->get('Vary')) . ', ' . $header); } return $response; diff --git a/tests/CorsServiceTest.php b/tests/CorsServiceTest.php index 9b7f5e8..589dcc5 100644 --- a/tests/CorsServiceTest.php +++ b/tests/CorsServiceTest.php @@ -83,7 +83,7 @@ public function itNormalizesWildcardOrigins(): void $service = new CorsService(['allowedOrigins' => $origins]); $this->assertInstanceOf(CorsService::class, $service); - $this->assertEquals(true, $this->getOptionsFromService($service)['allowedOrigins']); + $this->assertTrue($this->getOptionsFromService($service)['allowAllOrigins']); } /** diff --git a/tests/CorsTest.php b/tests/CorsTest.php index 11ddd6c..8838a32 100644 --- a/tests/CorsTest.php +++ b/tests/CorsTest.php @@ -555,23 +555,15 @@ private function createValidPreflightRequest(): Request /** * @param CorsInputOptions $options - * @param array{'Vary'?: string} $responseHeaders + * @param string[] $responseHeaders * @return MockApp */ private function createStackedApp(array $options = array(), array $responseHeaders = array()): MockApp { - $passedOptions = array_merge( - array( - 'allowedHeaders' => array('x-allowed-header', 'x-other-allowed-header'), - 'allowedMethods' => array('delete', 'get', 'post', 'put'), - 'allowedOrigins' => array('http://localhost'), - 'exposedHeaders' => false, - 'maxAge' => false, - 'supportsCredentials' => false, - ), - $options - ); + $options['allowedHeaders'] = $options['allowedHeaders'] ?? ['x-allowed-header', 'x-other-allowed-header']; + $options['allowedMethods'] = $options['allowedMethods'] ?? ['delete', 'get', 'post', 'put']; + $options['allowedOrigins'] = $options['allowedOrigins'] ?? ['http://localhost']; - return new MockApp($responseHeaders, $passedOptions); + return new MockApp($responseHeaders, $options); } } diff --git a/tests/MockApp.php b/tests/MockApp.php index d87dcd1..79e4ff3 100644 --- a/tests/MockApp.php +++ b/tests/MockApp.php @@ -21,7 +21,7 @@ */ class MockApp { - /** @var array{'Vary'?: string} */ + /** @var string[] */ private $responseHeaders; /** @@ -30,7 +30,7 @@ class MockApp private $cors; /** - * @param array{'Vary'?: string} $responseHeaders + * @param string[] $responseHeaders * @param CorsInputOptions $options */ public function __construct(array $responseHeaders, array $options = [])