diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e7b4305..5f8889f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,12 +7,13 @@ jobs: runs-on: ubuntu-latest strategy: + fail-fast: false matrix: - TYPO3: ['11' , '12', '13'] + TYPO3: ['13', '14'] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Set up PHP Version uses: shivammathur/setup-php@v2 @@ -20,29 +21,30 @@ jobs: php-version: 8.3 tools: composer:v2 - - name: Validate composer.json and composer.lock - run: composer validate + - name: Validate composer.json + run: composer validate --no-check-lock - name: Cache dependencies - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: ~/.composer/cache - key: dependencies-composer-${{ hashFiles('composer.json') }} + key: dependencies-composer-${{ matrix.TYPO3 }}-${{ hashFiles('composer.json') }} + + - name: Install composer dependencies TYPO3 14 + if: matrix.TYPO3 == '14' + run: | + composer require typo3/cms-core:^14.0 typo3/cms-frontend:^14.0 --no-progress --no-interaction -W - name: Install composer dependencies TYPO3 13 if: matrix.TYPO3 == '13' run: | - composer install --no-progress --no-interaction + composer require typo3/cms-core:^13.4 typo3/cms-frontend:^13.4 --no-progress --no-interaction -W + + - name: Unit Tests + run: ./vendor/bin/phpunit -c Build/phpunit/UnitTests.xml - - name: Install composer dependencies TYPO3 12 - if: matrix.TYPO3 == '12' - run: | - composer require typo3/cms-core:^12.4 --no-progress --no-interaction --dev -W - - name: Install composer dependencies TYPO3 11 - if: matrix.TYPO3 == '11' - run: | - composer require typo3/cms-core:^11.5 --no-progress --no-interaction --dev -W - name: Phpstan run: ./vendor/bin/phpstan analyze -c phpstan.neon + - name: Phpcsfix run: ./vendor/bin/php-cs-fixer fix --config=php-cs-fixer.php --dry-run --stop-on-violation --using-cache=no diff --git a/.gitignore b/.gitignore index 796aece..cc7b784 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /public/ /vendor/ /.php-cs-fixer.cache +/Build/phpunit/.phpunit.cache/ diff --git a/Build/phpunit/UnitTests.xml b/Build/phpunit/UnitTests.xml new file mode 100644 index 0000000..6383fba --- /dev/null +++ b/Build/phpunit/UnitTests.xml @@ -0,0 +1,24 @@ + + + + + ../../Tests/Unit/ + + + + + ../../src/ + + + diff --git a/Build/phpunit/UnitTestsBootstrap.php b/Build/phpunit/UnitTestsBootstrap.php new file mode 100644 index 0000000..3b028a3 --- /dev/null +++ b/Build/phpunit/UnitTestsBootstrap.php @@ -0,0 +1,17 @@ +shouldBeApplied()); + } + + #[Test] + public function doNotApplyPreventsMiddlewareFromBeingApplied(): void + { + $event = new BeforeMiddlewareIsAppliedEvent(new ServerRequest('https://example.com/Foo')); + + $event->doNotApply(); + + self::assertFalse($event->shouldBeApplied()); + } + + #[Test] + public function serverRequestIsExposedToListeners(): void + { + $request = new ServerRequest('https://example.com/Foo'); + $event = new BeforeMiddlewareIsAppliedEvent($request); + + self::assertSame($request, $event->getServerRequest()); + } +} diff --git a/Tests/Unit/Middleware/Fixtures/RoutableSite.php b/Tests/Unit/Middleware/Fixtures/RoutableSite.php new file mode 100644 index 0000000..61360b0 --- /dev/null +++ b/Tests/Unit/Middleware/Fixtures/RoutableSite.php @@ -0,0 +1,76 @@ +matchingPaths) implements RouterInterface { + /** + * @param string[] $matchingPaths + */ + public function __construct( + private readonly Site $site, + private readonly array $matchingPaths + ) { + } + + public function matchRequest( + ServerRequestInterface $request, + ?RouteResultInterface $previousResult = null + ): RouteResultInterface { + if (!in_array($request->getUri()->getPath(), $this->matchingPaths, true)) { + throw new RouteNotFoundException('No route found', 1579000000); + } + return new SiteRouteResult($request->getUri(), $this->site, null); + } + + public function generateUri( + $route, + array $parameters = [], + string $fragment = '', + string $type = self::ABSOLUTE_URL + ): UriInterface { + throw new \BadMethodCallException('Not needed in these tests', 1579000001); + } + }; + } +} diff --git a/Tests/Unit/Middleware/LowerCaseUriTest.php b/Tests/Unit/Middleware/LowerCaseUriTest.php new file mode 100644 index 0000000..a512ed6 --- /dev/null +++ b/Tests/Unit/Middleware/LowerCaseUriTest.php @@ -0,0 +1,331 @@ +handledRequest) implements RequestHandlerInterface { + public function __construct(private ?ServerRequestInterface &$captured) + { + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + $this->captured = $request; + return new Response(); + } + }; + } + + /** + * The middleware type-hints the concrete EventDispatcher, so a stub subclass is + * used rather than a mock to avoid constructing a ListenerProvider container. + */ + private function eventDispatcher(bool $apply = true): EventDispatcher + { + return new class($apply) extends EventDispatcher { + public function __construct(private readonly bool $apply) + { + // Intentionally not calling parent::__construct(): no listener + // provider is needed, dispatch() is fully overridden below. + } + + public function dispatch(object $event): object + { + if (!$this->apply && $event instanceof BeforeMiddlewareIsAppliedEvent) { + $event->doNotApply(); + } + return $event; + } + }; + } + + private function subject(bool $apply = true): LowerCaseUri + { + return new LowerCaseUri($this->eventDispatcher($apply)); + } + + private function siteLanguage(array $configuration = []): SiteLanguage + { + return new SiteLanguage(0, 'en-US', new \TYPO3\CMS\Core\Http\Uri('/'), $configuration); + } + + #[Test] + public function upperCasePathIsLoweredForFurtherProcessing(): void + { + $request = new ServerRequest('https://example.com/Lets-Connect'); + + $this->subject()->process($request, $this->handler()); + + self::assertSame('/lets-connect', $this->handledRequest->getUri()->getPath()); + } + + #[Test] + public function alreadyLowerCasePathIsPassedThroughUntouched(): void + { + $request = new ServerRequest('https://example.com/lets-connect'); + + $this->subject()->process($request, $this->handler()); + + self::assertSame($request, $this->handledRequest); + } + + #[Test] + public function queryStringCaseIsPreserved(): void + { + $request = new ServerRequest('https://example.com/Lets-Connect?tx_foo[Bar]=BaZ'); + + $this->subject()->process($request, $this->handler()); + + // Only the path is lowered; argument names and values survive verbatim + // (brackets are percent-encoded by Uri itself). + self::assertSame('/lets-connect', $this->handledRequest->getUri()->getPath()); + self::assertSame('tx_foo%5BBar%5D=BaZ', $this->handledRequest->getUri()->getQuery()); + } + + #[Test] + public function hostIsNotAffectedByPathLowering(): void + { + $request = new ServerRequest('https://example.com/Lets-Connect'); + + $this->subject()->process($request, $this->handler()); + + self::assertSame('/lets-connect', $this->handledRequest->getUri()->getPath()); + self::assertSame('example.com', $this->handledRequest->getUri()->getHost()); + } + + /** + * Documents a known limitation rather than desired behaviour. + * + * mb_strtolower() is applied to the still percent-encoded path, so it lowers + * the hex digits of an encoded octet instead of the character they represent. + * "/%C3%9CBER-UNS" becomes "/%c3%9cber-uns", which decodes back to "/Uber-uns" + * with a still-uppercase umlaut. Pure ASCII paths are unaffected, which is why + * this has gone unnoticed. + */ + #[Test] + public function encodedNonAsciiCharactersAreNotActuallyLowered(): void + { + $request = new ServerRequest('https://example.com/%C3%9CBER-UNS'); + + $this->subject()->process($request, $this->handler()); + + $loweredPath = $this->handledRequest->getUri()->getPath(); + self::assertSame('/%c3%9cber-uns', $loweredPath); + // The ASCII part is lowered correctly ... + self::assertStringContainsString('ber-uns', $loweredPath); + // ... but the umlaut is still uppercase once decoded. + self::assertSame("/\u{00DC}ber-uns", rawurldecode($loweredPath)); + } + + #[Test] + public function middlewareIsSkippedWhenEventListenerPreventsIt(): void + { + $request = new ServerRequest('https://example.com/Lets-Connect'); + + $this->subject(apply: false)->process($request, $this->handler()); + + self::assertSame($request, $this->handledRequest); + self::assertSame('/Lets-Connect', $this->handledRequest->getUri()->getPath()); + } + + #[Test] + public function requestIsPassedThroughWhenOriginalUriAlreadyResolves(): void + { + $site = new RoutableSite('main', 1, ['base' => 'https://example.com/'], ['/Order/paymentForm']); + $request = (new ServerRequest('https://example.com/Order/paymentForm')) + ->withAttribute('site', $site); + + $this->subject()->process($request, $this->handler()); + + // Camel-case route enhancers must keep working untouched. + self::assertSame('/Order/paymentForm', $this->handledRequest->getUri()->getPath()); + } + + #[Test] + public function routingAttributeIsRebuiltWithLoweredUriAndTail(): void + { + $site = new RoutableSite('main', 1, ['base' => 'https://example.com/']); + $language = $this->siteLanguage(); + $uri = new \TYPO3\CMS\Core\Http\Uri('https://example.com/Lets-Connect/SubPage'); + $routeResult = new SiteRouteResult($uri, $site, $language, 'SubPage'); + + $request = (new ServerRequest($uri)) + ->withAttribute('site', $site) + ->withAttribute('language', $language) + ->withAttribute('routing', $routeResult); + + $this->subject()->process($request, $this->handler()); + + $result = $this->handledRequest->getAttribute('routing'); + self::assertInstanceOf(SiteRouteResult::class, $result); + self::assertSame('/lets-connect/subpage', $result->getUri()->getPath()); + self::assertSame('subpage', $result->getTail()); + self::assertSame($site, $result->getSite()); + self::assertSame($language, $result->getLanguage()); + } + + #[Test] + public function redirectIsIssuedWhenEnabledOnSiteLevel(): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => true], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage()); + + $response = $this->subject()->process($request, $this->handler()); + + self::assertSame(307, $response->getStatusCode()); + self::assertSame('https://example.com/lets-connect', $response->getHeaderLine('location')); + self::assertNull($this->handledRequest, 'Handler must not be called on redirect'); + } + + #[Test] + public function redirectStatusCodeIsConfigurableOnSiteLevel(): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => true, 'redirectStatusCode' => 303], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage()); + + $response = $this->subject()->process($request, $this->handler()); + + self::assertSame(303, $response->getStatusCode()); + } + + #[Test] + public function redirectIsIssuedWhenEnabledOnLanguageLevel(): void + { + $site = new RoutableSite('main', 1, ['base' => 'https://example.com/']); + $request = (new ServerRequest('https://example.com/Lets-Connect')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage([ + 'redirectOnUpperCase' => true, + 'redirectStatusCode' => 308, + ])); + + $response = $this->subject()->process($request, $this->handler()); + + self::assertSame(308, $response->getStatusCode()); + } + + #[Test] + public function siteSettingsTakePrecedenceOverLanguageSettings(): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => true, 'redirectStatusCode' => 301], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage(['redirectStatusCode' => 302])); + + $response = $this->subject()->process($request, $this->handler()); + + self::assertSame(301, $response->getStatusCode()); + } + + #[Test] + public function noRedirectHappensWhenDisabled(): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => false], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage()); + + $this->subject()->process($request, $this->handler()); + + self::assertNotNull($this->handledRequest); + self::assertSame('/lets-connect', $this->handledRequest->getUri()->getPath()); + } + + /** + * @return array + */ + public static function nonRedirectableMethodsDataProvider(): array + { + return [ + 'POST' => ['POST'], + 'PUT' => ['PUT'], + 'DELETE' => ['DELETE'], + 'PATCH' => ['PATCH'], + ]; + } + + #[Test] + #[DataProvider('nonRedirectableMethodsDataProvider')] + public function redirectIsSkippedForNonGetOrHeadRequests(string $method): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => true], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect', $method)) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage()); + + $this->subject()->process($request, $this->handler()); + + // The request must be rewritten and forwarded rather than redirected, + // because a redirect would drop the request body. + self::assertNotNull($this->handledRequest); + self::assertSame('/lets-connect', $this->handledRequest->getUri()->getPath()); + } + + #[Test] + public function headRequestIsRedirected(): void + { + $site = new RoutableSite('main', 1, [ + 'base' => 'https://example.com/', + 'settings' => ['redirectOnUpperCase' => true], + ]); + $request = (new ServerRequest('https://example.com/Lets-Connect', 'HEAD')) + ->withAttribute('site', $site) + ->withAttribute('language', $this->siteLanguage()); + + $response = $this->subject()->process($request, $this->handler()); + + self::assertSame(307, $response->getStatusCode()); + } +} diff --git a/composer.json b/composer.json index aef502a..11df285 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,8 @@ "description": "With incoming URLs, it does not matter if they are upper/lowercase, they just work.", "license": "GPL-2.0-or-later", "require": { - "typo3/cms-core": "^11.5 || ^12.4 || ^13.1" + "typo3/cms-core": "^13.4 || ^14.0", + "typo3/cms-frontend": "^13.4 || ^14.0" }, "replace": { "typo3-ter/justincase": "self.version" @@ -19,9 +20,15 @@ "B13\\JustInCase\\": "src/" } }, + "autoload-dev": { + "psr-4": { + "B13\\JustInCase\\Tests\\": "Tests/" + } + }, "require-dev": { + "phpunit/phpunit": "^11.5 || ^12.0", + "saschaegerer/phpstan-typo3": "^3.0", "typo3/coding-standards": "^0.5.5", - "saschaegerer/phpstan-typo3": "^1.8", "typo3/tailor": "^1.1" }, "config": { @@ -33,6 +40,7 @@ }, "scripts": { "php:cs": "@composer exec 'php-cs-fixer fix --config=php-cs-fixer.php'", - "php:phpstan": "@composer exec 'phpstan analyse -c phpstan.neon'" + "php:phpstan": "@composer exec 'phpstan analyse -c phpstan.neon'", + "php:unit": "@composer exec 'phpunit -c Build/phpunit/UnitTests.xml'" } } diff --git a/ext_emconf.php b/ext_emconf.php index 63c6c8d..49754b6 100644 --- a/ext_emconf.php +++ b/ext_emconf.php @@ -8,10 +8,10 @@ 'author' => 'b13', 'author_email' => 'typo3@b13.com', 'author_company' => 'b13 GmbH', - 'version' => '2.0.0', + 'version' => '3.0.0', 'constraints' => [ 'depends' => [ - 'typo3' => '11.5.0-13.4.99', + 'typo3' => '13.4.0-14.99.99', ], 'conflicts' => [], 'suggests' => [], diff --git a/src/Middleware/LowerCaseUri.php b/src/Middleware/LowerCaseUri.php index 306eb72..729da7a 100644 --- a/src/Middleware/LowerCaseUri.php +++ b/src/Middleware/LowerCaseUri.php @@ -45,7 +45,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface if ($beforeMiddlewareIsAppliedEvent->shouldBeApplied() === false) { return $handler->handle($request); } - /** @var Site $site */ $site = $request->getAttribute('site'); if ($site instanceof Site) {