From a4be6237da7c04c52b4f4b6510e5079af8ecb8bc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 26 Jun 2024 13:06:16 +0200 Subject: [PATCH] Add phpstan, fix types --- .gitattributes | 1 + .github/workflows/phpstan.yml | 47 +++++ .../CorsConfigurationProviderPass.php | 4 +- DependencyInjection/Configuration.php | 8 +- DependencyInjection/NelmioCorsExtension.php | 28 +-- .../CacheableResponseVaryListener.php | 2 +- EventListener/CorsListener.php | 53 ++++-- Options/ConfigProvider.php | 20 ++- Options/ProviderInterface.php | 7 +- Options/Resolver.php | 7 +- Options/ResolverInterface.php | 8 +- Tests/CorsListenerTest.php | 161 +++++++++--------- .../CacheableResponseVaryListenerTest.php | 9 +- Tests/Fixtures/ProviderMock.php | 7 +- Tests/Options/ConfigProviderTest.php | 32 +++- Tests/Options/ResolverTest.php | 33 ++-- Tests/bootstrap.php | 8 +- composer.json | 11 +- phpstan.neon.dist | 19 +++ 19 files changed, 301 insertions(+), 164 deletions(-) create mode 100644 .github/workflows/phpstan.yml create mode 100644 phpstan.neon.dist diff --git a/.gitattributes b/.gitattributes index 85f410e..ef2ac25 100644 --- 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 new file mode 100644 index 0000000..f8d5a7a --- /dev/null +++ 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 index c075401..39babcb 100644 --- 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> $providersByPriority * @return Reference[] */ protected function sortProviders(array $providersByPriority): array diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 900fb12..ffb612b 100644 --- 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 index d1d9584..e9f9290 100644 --- 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 index d1e1ef8..aa01d62 100644 --- 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 index 06dcac5..0f996d0 100644 --- 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 + * @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 + * @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,7 +262,7 @@ 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'])); } @@ -258,11 +270,15 @@ protected function getPreflightResponse(Request $request, array $options): Respo $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,10 +290,13 @@ 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; @@ -285,7 +304,7 @@ protected function checkOrigin(Request $request, array $options): bool 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 $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 index 73d8943..b5b2c1d 100644 --- 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|int> + * @phpstan-var CorsCompleteOptions + */ protected $defaults; - public function __construct(array $paths, array $defaults = []) + /** + * @param CorsOptionsPerPath $paths + * @param array|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 index 0d5fc28..0d32b2f 100644 --- 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, allow_credentials?: bool, allow_origin?: bool|list, allow_headers?: bool|list, allow_private_network?: bool, origin_regex?: bool, allow_methods?: list, expose_headers?: list, max_age?: int, forced_allow_origin_value?: string, skip_same_as_origin?: bool} + * @phpstan-type CorsCompleteOptions array{hosts: list, allow_credentials: bool, allow_origin: bool|list, allow_headers: bool|list, allow_private_network: bool, origin_regex: bool, allow_methods: list, expose_headers: list, max_age: int, forced_allow_origin_value?: string, skip_same_as_origin: bool} + * @phpstan-type CorsOptionsPerPath array */ interface ProviderInterface { @@ -35,7 +39,8 @@ interface ProviderInterface * - array expose_headers * - int max_age * - * @return array CORS options + * @return array|int> CORS options + * @phpstan-return CorsOptions */ public function getOptions(Request $request): array; } diff --git a/Options/Resolver.php b/Options/Resolver.php index 3b2d203..f23bd30 100644 --- a/Options/Resolver.php +++ b/Options/Resolver.php @@ -20,12 +20,12 @@ class Resolver implements ResolverInterface { /** * CORS configuration providers, indexed by numerical priority - * @var ProviderInterface[][] + * @var list */ private $providers; /** - * @param $providers ProviderInterface[] + * @param list $providers */ public function __construct(array $providers = []) { @@ -34,8 +34,6 @@ public function __construct(array $providers = []) /** * Resolves the options for $request based on {@see $providers} data - * - * @return array CORS options */ public function getOptions(Request $request): array { @@ -44,6 +42,7 @@ public function getOptions(Request $request): array $options[] = $provider->getOptions($request); } + // @phpstan-ignore return.type (the default ConfigProvider will ensure default array is always setting every key) return array_merge(...$options); } } diff --git a/Options/ResolverInterface.php b/Options/ResolverInterface.php index cdd5a23..2e10c08 100644 --- a/Options/ResolverInterface.php +++ b/Options/ResolverInterface.php @@ -12,12 +12,16 @@ use Symfony\Component\HttpFoundation\Request; +/** + * @phpstan-import-type CorsCompleteOptions from ProviderInterface + */ interface ResolverInterface { /** - * Returns CORS options for $path + * Returns CORS options for a request's path * - * @internal param string $path + * @return array|int> CORS options + * @phpstan-return CorsCompleteOptions */ public function getOptions(Request $request): array; } diff --git a/Tests/CorsListenerTest.php b/Tests/CorsListenerTest.php index d3a943c..01189fb 100644 --- a/Tests/CorsListenerTest.php +++ b/Tests/CorsListenerTest.php @@ -11,8 +11,8 @@ namespace Nelmio\CorsBundle\Tests; -use Mockery as m; use Nelmio\CorsBundle\EventListener\CorsListener; +use Nelmio\CorsBundle\Options\ProviderInterface; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -20,13 +20,14 @@ use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; +/** + * @phpstan-import-type CorsOptions from ProviderInterface + */ class CorsListenerTest extends TestCase { - public function tearDown(): void - { - m::close(); - } - + /** + * @param CorsOptions $options + */ public function getListener(array $options = []): CorsListener { $mergedOptions = array_merge( @@ -46,8 +47,10 @@ public function getListener(array $options = []): CorsListener $options ); - $resolver = m::mock('Nelmio\CorsBundle\Options\ResolverInterface'); - $resolver->shouldReceive('getOptions')->andReturn($mergedOptions); + $resolver = $this->getMockBuilder('Nelmio\CorsBundle\Options\ResolverInterface')->getMock(); + $resolver->expects($this->once()) + ->method('getOptions') + ->willReturn($mergedOptions); return new CorsListener($resolver); } @@ -55,7 +58,7 @@ public function getListener(array $options = []): CorsListener public function testPreflightedRequest(): void { $options = [ - 'allow_origin' => [true], + 'allow_origin' => true, 'allow_headers' => ['foo', 'bar'], 'allow_methods' => ['POST', 'PUT'], ]; @@ -66,15 +69,15 @@ public function testPreflightedRequest(): void $req->headers->set('Access-Control-Request-Method', 'POST'); $req->headers->set('Access-Control-Request-Headers', 'Foo, BAR'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals('POST, PUT', $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals('foo, bar', $resp->headers->get('Access-Control-Allow-Headers')); - $this->assertEquals(['Origin'], $resp->getVary()); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals('POST, PUT', $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals('foo, bar', $resp->headers->get('Access-Control-Allow-Headers')); + self::assertEquals(['Origin'], $resp->getVary()); // actual request $req = Request::create('/foo', 'POST'); @@ -82,22 +85,21 @@ public function testPreflightedRequest(): void $req->headers->set('Foo', 'huh'); $req->headers->set('BAR', 'lala'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Headers')); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals(null, $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals(null, $resp->headers->get('Access-Control-Allow-Headers')); } public function testPreflightedRequestLinkFirefox(): void { $options = [ - 'allow_origin' => [true], + 'allow_origin' => true, 'allow_methods' => ['LINK', 'PUT'], ]; @@ -106,14 +108,14 @@ public function testPreflightedRequestLinkFirefox(): void $req->headers->set('Origin', 'http://example.com'); $req->headers->set('Access-Control-Request-Method', 'Link'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals('LINK, PUT, Link', $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals(['Origin'], $resp->getVary()); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals('LINK, PUT, Link', $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals(['Origin'], $resp->getVary()); } public function testPreflightedRequestWithForcedAllowOriginValue(): void @@ -121,7 +123,7 @@ public function testPreflightedRequestWithForcedAllowOriginValue(): void // allow_origin matches origin header // => 'Access-Control-Allow-Origin' should be equal to "forced_allow_origin_value" (i.e. '*') $options = [ - 'allow_origin' => [true], + 'allow_origin' => true, 'allow_methods' => ['GET'], 'forced_allow_origin_value' => '*', ]; @@ -130,16 +132,16 @@ public function testPreflightedRequestWithForcedAllowOriginValue(): void $req->headers->set('Origin', 'http://example.com'); $req->headers->set('Access-Control-Request-Method', 'GET'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST, $event->getResponse()); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $event->getResponse()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, $event->getResponse()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals(['Origin'], $resp->getVary()); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals(['Origin'], $resp->getVary()); // allow_origin does not match origin header // => 'Access-Control-Allow-Origin' should be equal to "forced_allow_origin_value" (i.e. '*') @@ -153,15 +155,15 @@ public function testPreflightedRequestWithForcedAllowOriginValue(): void $req->headers->set('Origin', 'http://example.com'); $req->headers->set('Access-Control-Request-Method', 'GET'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST, $event->getResponse()); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $event->getResponse()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, $event->getResponse()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods')); } public function testSameHostRequest(): void @@ -177,13 +179,13 @@ public function testSameHostRequest(): void $req->headers->set('Host', 'example.com'); $req->headers->set('Origin', 'http://example.com'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $this->assertNull($event->getResponse()); + self::assertNull($event->getResponse()); } - public function testPreflightedRequestWithOriginButNo() + public function testPreflightedRequestWithOriginButNo(): void { $options = [ 'allow_origin' => [], @@ -195,12 +197,12 @@ public function testPreflightedRequestWithOriginButNo() $req->headers->set('Origin', 'http://evil.com'); $req->headers->set('Access-Control-Request-Method', 'POST'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertNull($resp->headers->get('Access-Control-Allow-Origin')); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); + self::assertEquals(200, $resp->getStatusCode()); + self::assertNull($resp->headers->get('Access-Control-Allow-Origin')); } public function testRequestWithOriginButNo(): void @@ -214,10 +216,10 @@ public function testRequestWithOriginButNo(): void $req->headers->set('Host', 'example.com'); $req->headers->set('Origin', 'http://evil.com'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $this->assertNull($event->getResponse()); + self::assertNull($event->getResponse()); } public function testRequestWithForcedAllowOriginValue(): void @@ -233,14 +235,13 @@ public function testRequestWithForcedAllowOriginValue(): void $req = Request::create('/foo', 'GET'); $req->headers->set('Origin', 'http://example.com'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('http://example.com http://huh-lala.foobar', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('http://example.com http://huh-lala.foobar', $resp->headers->get('Access-Control-Allow-Origin')); // request without "Origin" header // => 'Access-Control-Allow-Origin' should be equal to "forced_allow_origin_value" (i.e. '*') @@ -250,14 +251,13 @@ public function testRequestWithForcedAllowOriginValue(): void $req = Request::create('/foo', 'GET'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin')); } /** @@ -269,7 +269,7 @@ public function testRequestWithForcedAllowOriginValue(): void private function testPreflightedRequestWithPrivateNetworkAccess($option, $header, $expectedHeader, $expectedStatus): void { $options = [ - 'allow_origin' => [true], + 'allow_origin' => true, 'allow_headers' => ['foo', 'bar'], 'allow_methods' => ['POST', 'PUT'], 'allow_private_network' => $option, @@ -284,17 +284,17 @@ private function testPreflightedRequestWithPrivateNetworkAccess($option, $header $req->headers->set('Access-Control-Request-Private-Network', $header); } - $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST); + $dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals($expectedStatus, $resp->getStatusCode()); - $this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals('POST, PUT', $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals('foo, bar', $resp->headers->get('Access-Control-Allow-Headers')); - $this->assertEquals($expectedHeader, $resp->headers->get('Access-Control-Allow-Private-Network')); - $this->assertEquals(['Origin'], $resp->getVary()); + self::assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); + self::assertEquals($expectedStatus, $resp->getStatusCode()); + self::assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals('POST, PUT', $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals('foo, bar', $resp->headers->get('Access-Control-Allow-Headers')); + self::assertEquals($expectedHeader, $resp->headers->get('Access-Control-Allow-Private-Network')); + self::assertEquals(['Origin'], $resp->getVary()); // actual request $req = Request::create('/foo', 'POST'); @@ -302,17 +302,16 @@ private function testPreflightedRequestWithPrivateNetworkAccess($option, $header $req->headers->set('Foo', 'huh'); $req->headers->set('BAR', 'lala'); - $event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST); + $event = new RequestEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST); $this->getListener($options)->onKernelRequest($event); - $event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, new Response()); + $event = new ResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $req, HttpKernelInterface::MAIN_REQUEST, new Response()); $this->getListener($options)->onKernelResponse($event); $resp = $event->getResponse(); - $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp); - $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); - $this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Methods')); - $this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Headers')); - $this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Private-Network')); + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin')); + self::assertEquals(null, $resp->headers->get('Access-Control-Allow-Methods')); + self::assertEquals(null, $resp->headers->get('Access-Control-Allow-Headers')); + self::assertEquals(null, $resp->headers->get('Access-Control-Allow-Private-Network')); } public function testPreflightedRequestWithPrivateNetworkAccessAllowedAndProvided(): void diff --git a/Tests/EventListener/CacheableResponseVaryListenerTest.php b/Tests/EventListener/CacheableResponseVaryListenerTest.php index 5c96f02..bfb98af 100644 --- a/Tests/EventListener/CacheableResponseVaryListenerTest.php +++ b/Tests/EventListener/CacheableResponseVaryListenerTest.php @@ -11,8 +11,11 @@ class CacheableResponseVaryListenerTest extends TestCase { + /** @var CacheableResponseVaryListener */ private $listener; + /** @var ResponseEvent */ private $event; + /** @var Response */ private $response; protected function setUp(): void @@ -26,15 +29,15 @@ protected function setUp(): void $this->listener = new CacheableResponseVaryListener(); } - public function testOriginIsAddedAsVaryHeaderOnCacheableResponse() + public function testOriginIsAddedAsVaryHeaderOnCacheableResponse(): void { $this->response->setTtl(300); $this->listener->onResponse($this->event); - self::assertStringContainsString('Origin', $this->event->getResponse()->headers->get('Vary')); + self::assertStringContainsString('Origin', (string) $this->event->getResponse()->headers->get('Vary')); } - public function testOriginIsNotAddedAsVaryHeaderOnNonCacheableResponse() + public function testOriginIsNotAddedAsVaryHeaderOnNonCacheableResponse(): void { $this->listener->onResponse($this->event); diff --git a/Tests/Fixtures/ProviderMock.php b/Tests/Fixtures/ProviderMock.php index f822227..c47fcac 100644 --- a/Tests/Fixtures/ProviderMock.php +++ b/Tests/Fixtures/ProviderMock.php @@ -11,5 +11,10 @@ final class ProviderMock implements ProviderInterface { public function __construct() {} - public function getOptions(Request $request): array {} + /** + * @return array + */ + public function getOptions(Request $request): array { + return []; + } } diff --git a/Tests/Options/ConfigProviderTest.php b/Tests/Options/ConfigProviderTest.php index f5c6111..7cc7e8a 100644 --- a/Tests/Options/ConfigProviderTest.php +++ b/Tests/Options/ConfigProviderTest.php @@ -10,11 +10,19 @@ namespace Nelmio\CorsBundle\Tests\Options; use Nelmio\CorsBundle\Options\ConfigProvider; +use Nelmio\CorsBundle\Options\ProviderInterface; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +/** + * @phpstan-import-type CorsOptions from ProviderInterface + * @phpstan-import-type CorsCompleteOptions from ProviderInterface + */ class ConfigProviderTest extends TestCase { + /** + * @phpstan-var CorsCompleteOptions + */ protected $defaultOptions = [ 'allow_credentials' => false, 'allow_origin' => ['http://one.example.com'], @@ -24,8 +32,13 @@ class ConfigProviderTest extends TestCase 'expose_headers' => [], 'max_age' => 0, 'hosts' => [], + 'origin_regex' => false, + 'skip_same_as_origin' => true, ]; + /** + * @phpstan-var CorsOptions + */ protected $pathOptions = [ 'allow_credentials' => true, 'allow_origin' => ['http://two.example.com'], @@ -37,6 +50,9 @@ class ConfigProviderTest extends TestCase 'hosts' => [], ]; + /** + * @phpstan-var CorsOptions + */ protected $domainMatchOptions = [ 'allow_credentials' => true, 'allow_origin' => ['http://domainmatch.example.com'], @@ -48,6 +64,9 @@ class ConfigProviderTest extends TestCase 'hosts' => ['^test\.', '\.example\.org$'], ]; + /** + * @phpstan-var CorsOptions + */ protected $noDomainMatchOptions = [ 'allow_credentials' => true, 'allow_origin' => ['http://nomatch.example.com'], @@ -59,6 +78,9 @@ class ConfigProviderTest extends TestCase 'hosts' => ['^nomatch\.'], ]; + /** + * @phpstan-var CorsOptions + */ protected $originRegexOptions = [ 'allow_credentials' => true, 'allow_origin' => ['^http://(.*)\.example\.com'], @@ -86,7 +108,7 @@ public function testGetOptionsForMappedPath(): void $provider = $this->getProvider(); self::assertEquals( - $this->pathOptions, + array_merge($this->defaultOptions, $this->pathOptions), $provider->getOptions(Request::create('/test/abc')) ); } @@ -96,7 +118,7 @@ public function testGetOptionsMatchingDomain(): void $provider = $this->getProvider(); self::assertEquals( - $this->domainMatchOptions, + array_merge($this->defaultOptions, $this->domainMatchOptions), $provider->getOptions(Request::create('/test/match', 'OPTIONS', [], [], [], ['HTTP_HOST' => 'test.example.com'])) ); } @@ -106,7 +128,7 @@ public function testGetOptionsMatchingDomain2(): void $provider = $this->getProvider(); self::assertEquals( - $this->domainMatchOptions, + array_merge($this->defaultOptions, $this->domainMatchOptions), $provider->getOptions(Request::create('/test/match', 'OPTIONS', [], [], [], ['HTTP_HOST' => 'foo.example.org'])) ); } @@ -116,7 +138,7 @@ public function testGetOptionsNotMatchingDomain(): void $provider = $this->getProvider(); self::assertEquals( - $this->pathOptions, + array_merge($this->defaultOptions, $this->pathOptions), $provider->getOptions(Request::create('/test/nomatch', 'OPTIONS', [], [], [], ['HTTP_HOST' => 'example.com'])) ); } @@ -126,7 +148,7 @@ public function testGetOptionsRegexOrigin(): void $provider = $this->getProvider(); self::assertEquals( - $this->originRegexOptions, + array_merge($this->defaultOptions, $this->originRegexOptions), $provider->getOptions(Request::create('/test/regex')) ); } diff --git a/Tests/Options/ResolverTest.php b/Tests/Options/ResolverTest.php index 0e4d986..c74d8b6 100644 --- a/Tests/Options/ResolverTest.php +++ b/Tests/Options/ResolverTest.php @@ -9,7 +9,7 @@ */ namespace Nelmio\CorsBundle\Tests\Options; -use Mockery as m; +use PHPUnit\Framework\MockObject\MockObject; use Nelmio\CorsBundle\Options\ProviderInterface; use Nelmio\CorsBundle\Options\Resolver; use PHPUnit\Framework\TestCase; @@ -28,21 +28,16 @@ class ResolverTest extends TestCase /** * Return value of the default (low priority) provider - * @var array + * @var array */ protected $defaultProviderValue; /** * Return value of the extra (high priority) provider - * @var array + * @var array */ protected $extraProviderValue; - public function tearDown(): void - { - m::close(); - } - public function testGetOptionsForPath(): void { $this->defaultProviderValue = [ @@ -81,38 +76,36 @@ protected function getResolver(): Resolver } /** - * @return m\MockInterface|ProviderInterface + * @return ProviderInterface&MockObject */ protected function getDefaultProviderMock(): ProviderInterface { $mock = $this->getProviderMock(); - $mock - ->shouldReceive('getOptions') - ->once() - ->andReturn($this->defaultProviderValue); + $mock->expects($this->once()) + ->method('getOptions') + ->willReturn($this->defaultProviderValue); return $mock; } /** - * @return m\MockInterface|ProviderInterface + * @return ProviderInterface&MockObject */ protected function getExtraProviderMock(): ProviderInterface { $mock = $this->getProviderMock(); - $mock - ->shouldReceive('getOptions') - ->once() - ->andReturn($this->extraProviderValue); + $mock->expects($this->once()) + ->method('getOptions') + ->willReturn($this->extraProviderValue); return $mock; } /** - * @return m\MockInterface|ProviderInterface + * @return ProviderInterface&MockObject */ protected function getProviderMock(): ProviderInterface { - return m::mock('Nelmio\CorsBundle\Options\ProviderInterface'); + return $this->getMockBuilder('Nelmio\CorsBundle\Options\ProviderInterface')->getMock(); } } diff --git a/Tests/bootstrap.php b/Tests/bootstrap.php index 601e8ce..7dce37c 100644 --- a/Tests/bootstrap.php +++ b/Tests/bootstrap.php @@ -1,10 +1,10 @@