diff --git a/Classes/Dispatcher/Bootstrap.php b/Classes/Dispatcher/Bootstrap.php index ba400877..d8ffdd61 100644 --- a/Classes/Dispatcher/Bootstrap.php +++ b/Classes/Dispatcher/Bootstrap.php @@ -7,11 +7,16 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use SourceBroker\T3api\Domain\Repository\ApiResourceRepository; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\Exception\ExceptionInterface; +use SourceBroker\T3api\Exception\InternalServerErrorException; use SourceBroker\T3api\Serializer\ContextBuilder\DeserializationContextBuilder; use SourceBroker\T3api\Serializer\ContextBuilder\SerializationContextBuilder; use SourceBroker\T3api\Service\OperationResponseCache; +use SourceBroker\T3api\Service\RequestLanguageService; use SourceBroker\T3api\Service\RouteService; use SourceBroker\T3api\Service\SerializerService; use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; @@ -24,8 +29,10 @@ use TYPO3\CMS\Core\Http\Response; use TYPO3\CMS\Core\Utility\GeneralUtility; -class Bootstrap extends AbstractDispatcher +class Bootstrap extends AbstractDispatcher implements LoggerAwareInterface { + use LoggerAwareTrait; + protected ResponseInterface $response; protected HttpFoundationFactory $httpFoundationFactory; @@ -59,6 +66,10 @@ public function process(ServerRequestInterface $inputRequest): ResponseInterface $request = $this->httpFoundationFactory->createRequest($inputRequest); $context = (new RequestContext())->fromRequest($request); $this->callProcessors($request, $this->response); + $languageHeaderException = $inputRequest->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION); + if ($languageHeaderException instanceof BadRequestException) { + throw $languageHeaderException; + } if ($this->isMainEndpointResponseClassDefined() && $this->isContextMatchingMainEndpointRoute($context)) { $output = $this->processMainEndpoint(); } else { @@ -68,8 +79,16 @@ public function process(ServerRequestInterface $inputRequest): ResponseInterface $output = $this->serializerService->serialize($exception); $this->response = $this->response->withStatus($exception->getStatusCode(), $exception->getTitle()); } catch (\Throwable $throwable) { + $eventId = bin2hex(random_bytes(8)); + $this->logger?->error( + 't3api: unexpected exception, event ID {eventId}', + ['eventId' => $eventId, 'exception' => $throwable] + ); try { - $output = $this->serializerService->serialize($throwable); + // The message of an unexpected exception may contain paths, database details etc. + $output = $this->serializerService->serialize( + SerializerService::isDebugMode() ? $throwable : new InternalServerErrorException($eventId) + ); $this->response = $this->response->withStatus( SymfonyResponse::HTTP_INTERNAL_SERVER_ERROR, SymfonyResponse::$statusTexts[SymfonyResponse::HTTP_INTERNAL_SERVER_ERROR] diff --git a/Classes/Exception/BadRequestException.php b/Classes/Exception/BadRequestException.php new file mode 100644 index 00000000..776cabbf --- /dev/null +++ b/Classes/Exception/BadRequestException.php @@ -0,0 +1,37 @@ +statusCode(SymfonyResponse::HTTP_BAD_REQUEST) + ->description(self::translate('exception.bad_request.title')); + } + + /** + * @param string $descriptionKey Key of the description in `locallang.xlf`, without the `exception.bad_request.` prefix + */ + public function __construct(string $descriptionKey, array $arguments, int $code) + { + $this->title = self::translate('exception.bad_request.title'); + parent::__construct(self::translate('exception.bad_request.' . $descriptionKey, $arguments), $code); + } + + public function getStatusCode(): int + { + return Response::HTTP_BAD_REQUEST; + } +} diff --git a/Classes/Exception/InternalServerErrorException.php b/Classes/Exception/InternalServerErrorException.php new file mode 100644 index 00000000..bd4e47c6 --- /dev/null +++ b/Classes/Exception/InternalServerErrorException.php @@ -0,0 +1,22 @@ +title = self::translate('exception.internal_server_error.title'); + parent::__construct( + self::translate('exception.internal_server_error.description', [$eventId]), + 1793285051 + ); + } +} diff --git a/Classes/Filter/DistanceFilter.php b/Classes/Filter/DistanceFilter.php index fb14e549..ac55dea6 100644 --- a/Classes/Filter/DistanceFilter.php +++ b/Classes/Filter/DistanceFilter.php @@ -5,6 +5,7 @@ namespace SourceBroker\T3api\Filter; use SourceBroker\T3api\Domain\Model\ApiFilter; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\OpenApi\Objects\Parameter; use SourceBroker\T3api\OpenApi\Objects\Schema; use TYPO3\CMS\Core\Database\ConnectionPool; @@ -142,19 +143,15 @@ public function filterProperty( } /** - * @throws \InvalidArgumentException + * @throws BadRequestException */ protected function getLatLangParameterValues(array $values, ApiFilter $apiFilter): array { if (!isset($values[self::PARAMETER_LATITUDE], $values[self::PARAMETER_LONGITUDE])) { - throw new \InvalidArgumentException( - sprintf( - 'Parameters `%s[%s]` and %s[%s] are required to use distance filter', - $apiFilter->getParameterName(), - self::PARAMETER_LATITUDE, - $apiFilter->getParameterName(), - self::PARAMETER_LONGITUDE - ) + throw new BadRequestException( + 'distance_parameters', + [$apiFilter->getParameterName(), self::PARAMETER_LATITUDE, self::PARAMETER_LONGITUDE], + 1793641752 ); } diff --git a/Classes/Filter/OrderFilter.php b/Classes/Filter/OrderFilter.php index 47f839ad..81b9a02c 100644 --- a/Classes/Filter/OrderFilter.php +++ b/Classes/Filter/OrderFilter.php @@ -5,6 +5,7 @@ namespace SourceBroker\T3api\Filter; use SourceBroker\T3api\Domain\Model\ApiFilter; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\OpenApi\Objects\Parameter; use SourceBroker\T3api\OpenApi\Objects\Schema; use TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface; @@ -45,6 +46,10 @@ public function filterProperty( return null; } + if (!is_string($values[$property])) { + throw new BadRequestException('order_direction', [gettype($values[$property])], 1560890654236); + } + $defaultDirection = $apiFilter->getStrategy()->getName(); $direction = strtoupper($values[$property] !== '' ? $values[$property] : $defaultDirection); @@ -53,7 +58,7 @@ public function filterProperty( } if (!in_array($direction, [QueryInterface::ORDER_ASCENDING, QueryInterface::ORDER_DESCENDING], true)) { - throw new \InvalidArgumentException(sprintf('Unknown order direction `%s`', $direction), 1560890654236); + throw new BadRequestException('order_direction', [$values[$property]], 1560890654236); } $query->setOrderings(array_merge($query->getOrderings(), [$property => $direction])); diff --git a/Classes/OperationHandler/AbstractOperationHandler.php b/Classes/OperationHandler/AbstractOperationHandler.php index eb8171c3..a73a5c95 100644 --- a/Classes/OperationHandler/AbstractOperationHandler.php +++ b/Classes/OperationHandler/AbstractOperationHandler.php @@ -8,6 +8,7 @@ use SourceBroker\T3api\Domain\Model\OperationInterface; use SourceBroker\T3api\Domain\Repository\CommonRepository; use SourceBroker\T3api\Event\AfterDeserializeOperationEvent; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\Exception\OperationNotAllowedException; use SourceBroker\T3api\Security\OperationAccessChecker; use SourceBroker\T3api\Serializer\ContextBuilder\DeserializationContextBuilder; @@ -56,6 +57,7 @@ protected function getRepositoryForOperation(OperationInterface $operation): Com } /** + * @throws BadRequestException * @throws OperationNotAllowedException */ protected function deserializeOperation( @@ -63,8 +65,11 @@ protected function deserializeOperation( Request $request, ?AbstractDomainObject $targetObject = null ): AbstractDomainObject { + $content = $request->getContent(); + $this->assertJsonObject($content); + $object = $this->serializerService->deserialize( - $request->getContent(), + $content, $operation->getApiResource()->getEntity(), $this->deserializationContextBuilder->createFromOperation($operation, $request, $targetObject) ); @@ -81,4 +86,22 @@ protected function deserializeOperation( return $afterDeserializeOperationEvent->getObject(); } + + /** + * Checked before deserialization, as the serializer reports malformed JSON with a generic runtime exception. + * + * @throws BadRequestException + */ + protected function assertJsonObject(string $content): void + { + try { + $data = json_decode($content, false, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException $exception) { + throw new BadRequestException('invalid_json', [$exception->getMessage()], 1793641753); + } + + if (!$data instanceof \stdClass) { + throw new BadRequestException('json_object', [], 1793641755); + } + } } diff --git a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php index b71cf55d..c2db52d5 100644 --- a/Classes/Serializer/Handler/AbstractDomainObjectHandler.php +++ b/Classes/Serializer/Handler/AbstractDomainObjectHandler.php @@ -8,6 +8,7 @@ use JMS\Serializer\Metadata\PropertyMetadata; use JMS\Serializer\Visitor\DeserializationVisitorInterface; use SourceBroker\T3api\Annotation\ORM\Cascade; +use SourceBroker\T3api\Exception\BadRequestException; use SourceBroker\T3api\Service\PropertyInfoService; use SourceBroker\T3api\Service\SerializerService; use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject; @@ -61,8 +62,9 @@ public function deserialize( return $this->processCascadePersistence($data, $targetObjectType, $context); } - throw new \InvalidArgumentException( - sprintf('It was not possible to deserialize %s into %s', gettype($data), AbstractDomainObject::class), + throw new BadRequestException( + 'invalid_relation', + [gettype($data), implode('.', $context->getCurrentPath())], 1584866997736 ); } diff --git a/Classes/Serializer/Handler/ObjectStorageHandler.php b/Classes/Serializer/Handler/ObjectStorageHandler.php index 2ff90318..054e6803 100644 --- a/Classes/Serializer/Handler/ObjectStorageHandler.php +++ b/Classes/Serializer/Handler/ObjectStorageHandler.php @@ -8,6 +8,7 @@ use JMS\Serializer\SerializationContext; use JMS\Serializer\Visitor\DeserializationVisitorInterface; use JMS\Serializer\Visitor\SerializationVisitorInterface; +use SourceBroker\T3api\Exception\BadRequestException; use TYPO3\CMS\Extbase\Persistence\Generic\LazyObjectStorage; use TYPO3\CMS\Extbase\Persistence\ObjectStorage; @@ -55,13 +56,9 @@ public function deserialize( } if (!is_array($data)) { - throw new \InvalidArgumentException( - sprintf( - 'Data of type `%s` can not be converted to %s in path `%s`', - gettype($data), - ObjectStorage::class, - implode('.', $context->getCurrentPath()) - ), + throw new BadRequestException( + 'invalid_collection', + [gettype($data), implode('.', $context->getCurrentPath())], 1570805126535 ); } diff --git a/Classes/Service/RequestLanguageService.php b/Classes/Service/RequestLanguageService.php index e3ce58c6..ed49a6c4 100644 --- a/Classes/Service/RequestLanguageService.php +++ b/Classes/Service/RequestLanguageService.php @@ -5,6 +5,8 @@ namespace SourceBroker\T3api\Service; use Psr\Http\Message\ServerRequestInterface; +use SourceBroker\T3api\Exception\BadRequestException; +use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; use TYPO3\CMS\Core\Context\Context; use TYPO3\CMS\Core\Context\LanguageAspectFactory; use TYPO3\CMS\Core\Site\Entity\Site; @@ -13,9 +15,16 @@ class RequestLanguageService { + /** + * Request attribute with the exception for an invalid language header. It is thrown in + * `Bootstrap::process()`, so the client gets a regular API error response. + */ + public const ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION = 't3apiLanguageHeaderException'; + public function withLanguageFromHeader(ServerRequestInterface $request): ServerRequestInterface { - $languageHeader = $request->getHeader($GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['languageHeader']); + $headerName = $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['languageHeader']; + $languageHeader = $request->getHeader($headerName); if ($languageHeader === []) { return $request; } @@ -26,12 +35,23 @@ public function withLanguageFromHeader(ServerRequestInterface $request): ServerR return $request; } - $languageUid = (int)array_shift($languageHeader); - if ($language->getLanguageId() === $languageUid) { + $headerValue = trim((string)array_shift($languageHeader)); + if ($headerValue === '') { + return $request; + } + + $targetLanguage = preg_match('/^\d+$/', $headerValue) === 1 ? $this->getAvailableLanguage($site, (int)$headerValue) : null; + if ($targetLanguage === null) { + return $request->withAttribute( + self::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION, + new BadRequestException('language_header', [$headerName, $headerValue], 1793641754) + ); + } + + if ($language->getLanguageId() === $targetLanguage->getLanguageId()) { return $request; } - $targetLanguage = $site->getLanguageById($languageUid); GeneralUtility::makeInstance(Context::class)->setAspect( 'language', LanguageAspectFactory::createFromSiteLanguage($targetLanguage) @@ -41,4 +61,22 @@ public function withLanguageFromHeader(ServerRequestInterface $request): ServerR ->withAttribute('t3apiHeaderLanguageRequest', true) ->withAttribute('language', $targetLanguage); } + + /** + * Same rule as for the language in URL (see TYPO3 `SiteBaseRedirectResolver`): a disabled language + * is available only for a backend user with access to it. + */ + private function getAvailableLanguage(Site $site, int $languageId): ?SiteLanguage + { + $language = $site->getAllLanguages()[$languageId] ?? null; + if ($language === null || $language->isEnabled()) { + return $language; + } + + $backendUser = $GLOBALS['BE_USER'] ?? null; + + return $backendUser instanceof BackendUserAuthentication && $backendUser->checkLanguageAccess($languageId) + ? $language + : null; + } } diff --git a/Configuration/RequestMiddlewares.php b/Configuration/RequestMiddlewares.php index 6427a17a..61da3ffb 100644 --- a/Configuration/RequestMiddlewares.php +++ b/Configuration/RequestMiddlewares.php @@ -46,6 +46,8 @@ 'target' => \SourceBroker\T3api\Middleware\T3apiRequestLanguageResolver::class, 'after' => [ 'typo3/cms-frontend/site', + // `$GLOBALS['BE_USER']` is needed to allow disabled languages for backend users + 'typo3/cms-frontend/backend-user-authentication', ], 'before' => [ 'typo3/cms-frontend/tsfe', diff --git a/Documentation/Miscellaneous/Changelog/Index.rst b/Documentation/Miscellaneous/Changelog/Index.rst index b2b99bbe..26ad9e9b 100644 --- a/Documentation/Miscellaneous/Changelog/Index.rst +++ b/Documentation/Miscellaneous/Changelog/Index.rst @@ -7,6 +7,10 @@ Changelog 5.0.0 ===== - Fix response cache invalidation when the page cache is cleared from the backend (cache tags passed as a list were ignored, numeric tags caused errors). +- Log unexpected exceptions with an event ID. Outside of ``Development`` context HTTP 500 response contains a generic message with this event ID instead of the exception message. See :ref:`ThrowableSubscriber `. +- Return HTTP 500 for PHP errors (e.g. ``TypeError``) as API response with CORS headers instead of rethrowing them to TYPO3. [`issue `__] +- [!!!] Return ``400 Bad Request`` (``SourceBroker\T3api\Exception\BadRequestException``) instead of HTTP 500 for invalid client input: malformed JSON or non-object request body, invalid value of a relation, unknown order direction, missing coordinates of distance filter. +- [!!!] Validate language header (``X-Locale``): unknown language identifier returns ``400 Bad Request`` instead of HTTP 500, non-numeric value returns ``400 Bad Request`` instead of default language. Disabled language is available only for backend users with access to it. See :ref:`multilanguage`. - Refactor ``CurrentFeUser`` handling. See :ref:`current user assignment `. - [!!!] CORS refactor: unify response handling, normalize configuration lists and improve preflight and cache behavior. Origin patterns now match complete origins. Review ``allowOrigin`` configuration and custom ``OPTIONS`` response handling when upgrading. See :ref:`cors`. - [!!!] Embed ``goldspecdigital/oooas`` library (PHP 8.5 compatibility). The package is no longer a dependency, namespace ``GoldSpecDigital\ObjectOrientedOAS`` is renamed to ``SourceBroker\T3api\OpenApi``. Update imports in custom filters implementing ``OpenApiSupportingFilterInterface`` and custom :ref:`collection response classes `. [`commit `__] diff --git a/Documentation/Multilanguage/Index.rst b/Documentation/Multilanguage/Index.rst index c39b0100..add6e1fc 100644 --- a/Documentation/Multilanguage/Index.rst +++ b/Documentation/Multilanguage/Index.rst @@ -36,6 +36,10 @@ from your site's ``config.yaml``. curl -H "X-Locale: 1" https://14.t3api.ddev.site/_api/news/news +An empty header is ignored. A value which is not the identifier of an available language (unknown identifier, +not a number) results in ``400 Bad Request``. A disabled language (``enabled: false``) is available only for a +logged in backend user with access to this language - same as for the language prefix in URL. + Here is a part of ``config.yaml`` used for the examples (site configuration of testing instance): .. code-block:: yaml diff --git a/Documentation/Serialization/Subscribers/Index.rst b/Documentation/Serialization/Subscribers/Index.rst index 1c18d738..6f259b4d 100644 --- a/Documentation/Serialization/Subscribers/Index.rst +++ b/Documentation/Serialization/Subscribers/Index.rst @@ -98,3 +98,8 @@ ThrowableSubscriber This subscriber listens to the ``POST_SERIALIZE`` event. If the object is an instance of ``Throwable``, it adds ``hydra:description`` (exception message) and - in ``Development`` application context only - ``hydra:debug`` (stack trace) to the serialized object. + +Unexpected exceptions (not implementing ``SourceBroker\T3api\Exception\ExceptionInterface``) are logged with an event ID +and result in HTTP 500. Outside of ``Development`` application context the response contains a generic +``SourceBroker\T3api\Exception\InternalServerErrorException`` with this event ID instead of the original exception, so its +message is not returned to the client. Use the event ID to find the original exception in the TYPO3 log. diff --git a/Resources/Private/Language/locallang.xlf b/Resources/Private/Language/locallang.xlf index 22fc2378..6e5718be 100644 --- a/Resources/Private/Language/locallang.xlf +++ b/Resources/Private/Language/locallang.xlf @@ -45,6 +45,36 @@ Parameter `%s` can not be greater than 1 if parameter `%s` is 0 + + Bad request + + + Request body is not valid JSON: %s + + + Request body has to be a JSON object + + + Value of type `%s` in path `%s` has to be an identifier or an object + + + Value of type `%s` in path `%s` has to be an array + + + Unknown order direction `%s`, expected `asc` or `desc` + + + Parameters `%1$s[%2$s]` and `%1$s[%3$s]` are required to use distance filter + + + Header `%s` has to contain the ID of an available language, `%s` given + + + Internal Server Error + + + An internal error occurred. Event ID: %s + diff --git a/Resources/Private/Serializer/Metadata/Throwable.yml b/Resources/Private/Serializer/Metadata/Throwable.yml index 474972e7..8014a502 100644 --- a/Resources/Private/Serializer/Metadata/Throwable.yml +++ b/Resources/Private/Serializer/Metadata/Throwable.yml @@ -1,4 +1,4 @@ -Exception: +Exception: &throwable properties: message: serialized_name: 'hydra:title' @@ -14,3 +14,6 @@ Exception: exclude: true previous: exclude: true + +# `Error` (e.g. `TypeError`) does not extend `Exception` - without metadata it can not be serialized. +Error: *throwable diff --git a/Tests/Functional/Dispatcher/BadRequestDispatcherTest.php b/Tests/Functional/Dispatcher/BadRequestDispatcherTest.php new file mode 100644 index 00000000..e2b3bfda --- /dev/null +++ b/Tests/Functional/Dispatcher/BadRequestDispatcherTest.php @@ -0,0 +1,172 @@ +dispatch($this->createRequest('POST', '/_api/articles', $body)); + + $this->assertBadRequest($response, $description); + self::assertSame(0, $this->getConnectionPool()->getConnectionForTable('tx_functionaltest_domain_model_article') + ->count('*', 'tx_functionaltest_domain_model_article', [])); + } + + public static function invalidBodyProvider(): array + { + return [ + 'malformed JSON' => ['{"title": ', 'Request body is not valid JSON: Syntax error'], + 'empty body' => ['', 'Request body is not valid JSON: Syntax error'], + 'JSON array' => ['[{"title": "Article"}]', 'Request body has to be a JSON object'], + 'JSON string' => ['"Article"', 'Request body has to be a JSON object'], + ]; + } + + #[Test] + #[DataProvider('invalidOrderProvider')] + public function invalidOrderDirectionIsRejected(string $query, string $description): void + { + $response = $this->dispatch($this->createRequest('GET', '/_api/products?' . $query)); + + $this->assertBadRequest($response, $description); + } + + public static function invalidOrderProvider(): array + { + return [ + 'unknown direction' => ['order[title]=sideways', 'Unknown order direction `sideways`, expected `asc` or `desc`'], + 'array instead of direction' => ['order[title][]=asc', 'Unknown order direction `array`, expected `asc` or `desc`'], + ]; + } + + #[Test] + public function unavailableLanguageInHeaderIsRejectedWithCorsHeaders(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowOrigin'] = ['https://client.example']; + $request = $this->createRequest('GET', '/_api/products') + ->withHeader('X-Locale', '99') + ->withHeader('Origin', 'https://client.example'); + + $response = $this->dispatch($this->get(RequestLanguageService::class)->withLanguageFromHeader($request)); + + $this->assertBadRequest($response, 'Header `X-Locale` has to contain the ID of an available language, `99` given'); + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + } + + #[Test] + #[DataProvider('invalidRelationProvider')] + public function invalidRelationValueIsRejected(string $body, int $code, string $description): void + { + try { + $this->get(SerializerService::class)->deserialize($body, Product::class); + self::fail('BadRequestException expected'); + } catch (BadRequestException $exception) { + self::assertSame($code, $exception->getCode()); + self::assertSame($description, $exception->getDescription()); + } + } + + public static function invalidRelationProvider(): array + { + return [ + 'single relation' => [ + '{"pid": 0, "category": true}', + 1584866997736, + 'Value of type `boolean` in path `category` has to be an identifier or an object', + ], + 'collection relation' => [ + '{"pid": 0, "tags": "first"}', + 1570805126535, + 'Value of type `string` in path `tags` has to be an array', + ], + ]; + } + + #[Test] + public function distanceFilterWithoutCoordinatesIsRejected(): void + { + $this->expectException(BadRequestException::class); + $this->expectExceptionCode(1793641752); + $this->expectExceptionMessage('Parameters `position[lat]` and `position[lng]` are required to use distance filter'); + + GeneralUtility::makeInstance(DistanceFilter::class)->filterProperty( + 'position', + ['lat' => '50.06'], + self::createStub(QueryInterface::class), + new ApiFilter(DistanceFilter::class, 'position', '', ['parameterName' => 'position']) + ); + } + + private function assertBadRequest(ResponseInterface $response, string $description): void + { + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(400, $response->getStatusCode(), $body); + self::assertSame('Bad request', $json['hydra:title']); + self::assertSame($description, $json['hydra:description']); + } + + private function createRequest(string $method, string $path, ?string $body = null): ServerRequest + { + $stream = new Stream('php://temp', 'rw'); + $stream->write($body ?? ''); + $stream->rewind(); + + return (new ServerRequest('https://example.com' . $path, $method, $stream, ['Content-Type' => 'application/json'])) + ->withQueryParams($this->parseQuery($path)) + ->withAttribute('site', $GLOBALS['TYPO3_REQUEST']->getAttribute('site')) + ->withAttribute('language', $GLOBALS['TYPO3_REQUEST']->getAttribute('language')); + } + + private function parseQuery(string $path): array + { + parse_str((string)parse_url($path, PHP_URL_QUERY), $query); + + return $query; + } + + private function dispatch(ServerRequestInterface $request): ResponseInterface + { + $dispatcher = new Bootstrap( + $this->get(SerializerService::class), + $this->get(ApiResourceRepository::class), + $this->get(SerializationContextBuilder::class), + $this->get(DeserializationContextBuilder::class), + $this->get(EventDispatcherInterface::class), + $this->get(OperationResponseCache::class) + ); + + return $dispatcher->process($request); + } +} diff --git a/Tests/Functional/Dispatcher/InternalErrorDispatcherTest.php b/Tests/Functional/Dispatcher/InternalErrorDispatcherTest.php new file mode 100644 index 00000000..5322c2a9 --- /dev/null +++ b/Tests/Functional/Dispatcher/InternalErrorDispatcherTest.php @@ -0,0 +1,162 @@ +originalContext = Environment::getContext(); + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowOrigin'] = ['https://client.example']; + } + + protected function tearDown(): void + { + $this->setApplicationContext($this->originalContext); + parent::tearDown(); + } + + #[Test] + public function unexpectedExceptionMessageIsReplacedOutsideOfDevelopmentContext(): void + { + $this->registerFailingProcessor(); + $logged = []; + $logger = $this->createMock(LoggerInterface::class); + $logger->expects(self::once()) + ->method('error') + ->willReturnCallback(static function ($message, array $context) use (&$logged): void { + $logged = $context; + }); + + $response = $this->dispatch($logger); + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + + self::assertFalse(SerializerService::isDebugMode()); + self::assertSame(500, $response->getStatusCode(), $body); + self::assertStringNotContainsString('db_user', $body); + self::assertStringNotContainsString('settings.php', $body); + self::assertSame('Internal Server Error', $json['hydra:title']); + self::assertSame('An internal error occurred. Event ID: ' . $logged['eventId'], $json['hydra:description']); + self::assertArrayNotHasKey('hydra:debug', $json); + self::assertMatchesRegularExpression('/^[0-9a-f]{16}$/', $logged['eventId']); + self::assertInstanceOf(\RuntimeException::class, $logged['exception']); + self::assertSame(FailingProcessor::MESSAGE, $logged['exception']->getMessage()); + // https://github.com/sourcebroker/t3api/issues/38 + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + } + + #[Test] + public function unexpectedExceptionMessageIsReturnedInDevelopmentContext(): void + { + $this->registerFailingProcessor(); + $this->setApplicationContext(new ApplicationContext('Development')); + $logger = $this->createMock(LoggerInterface::class); + $logger->expects(self::once())->method('error'); + + $response = $this->dispatch($logger); + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(500, $response->getStatusCode(), $body); + self::assertSame(FailingProcessor::MESSAGE, $json['hydra:description']); + self::assertArrayHasKey('hydra:debug', $json); + } + + /** + * https://github.com/sourcebroker/t3api/issues/38 - an `Error` could not be serialized, so it was rethrown + * and rendered by TYPO3 without CORS headers. + */ + #[Test] + public function errorIsReturnedAsApiResponseWithCorsHeadersInDevelopmentContext(): void + { + $this->registerFailingProcessor(FailingWithErrorProcessor::class); + $this->setApplicationContext(new ApplicationContext('Development')); + + $response = $this->dispatch(self::createStub(LoggerInterface::class)); + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(500, $response->getStatusCode(), $body); + self::assertSame(FailingWithErrorProcessor::MESSAGE, $json['hydra:description']); + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + } + + #[Test] + public function controlledExceptionMessageIsReturnedOutsideOfDevelopmentContext(): void + { + $logger = $this->createMock(LoggerInterface::class); + $logger->expects(self::never())->method('error'); + + $response = $this->dispatch($logger, '/_api/articles/999'); + $body = (string)$response->getBody(); + $json = json_decode($body, true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(404, $response->getStatusCode(), $body); + self::assertStringContainsString('999', $json['hydra:description']); + } + + #[Test] + public function loggerIsInjectedIntoBootstrapService(): void + { + $logger = (new \ReflectionProperty(Bootstrap::class, 'logger'))->getValue($this->get(Bootstrap::class)); + + self::assertInstanceOf(LoggerInterface::class, $logger); + } + + private function dispatch(LoggerInterface $logger, string $path = '/_api/articles/1'): ResponseInterface + { + $dispatcher = new Bootstrap( + $this->get(SerializerService::class), + $this->get(ApiResourceRepository::class), + $this->get(SerializationContextBuilder::class), + $this->get(DeserializationContextBuilder::class), + $this->get(EventDispatcherInterface::class), + $this->get(OperationResponseCache::class) + ); + $dispatcher->setLogger($logger); + + return $dispatcher->process( + (new ServerRequest('https://example.com' . $path, 'GET'))->withHeader('Origin', 'https://client.example') + ); + } + + /** + * Lower priority than CorsProcessor (100) - the exception is thrown after CORS headers are set, + * like an exception thrown by an operation handler. + */ + private function registerFailingProcessor(string $processorClass = FailingProcessor::class): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['processors'][$processorClass] = 50; + } + + private function setApplicationContext(ApplicationContext $context): void + { + (new \ReflectionProperty(Environment::class, 'context'))->setValue(null, $context); + } +} diff --git a/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Processor/FailingProcessor.php b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Processor/FailingProcessor.php new file mode 100644 index 00000000..43f898a2 --- /dev/null +++ b/Tests/Functional/Fixtures/Extensions/t3api_functional_test/Classes/Processor/FailingProcessor.php @@ -0,0 +1,23 @@ +site = new Site('test', 1, [ + 'base' => 'https://example.com/', + 'languages' => [ + ['languageId' => 0, 'title' => 'English', 'base' => '/', 'locale' => 'en_US.UTF-8'], + ['languageId' => 1, 'title' => 'German', 'base' => '/de/', 'locale' => 'de_DE.UTF-8'], + ['languageId' => 2, 'title' => 'Polish', 'base' => '/pl/', 'locale' => 'pl_PL.UTF-8', 'enabled' => false], + ], + ]); + } + + protected function tearDown(): void + { + unset($GLOBALS['BE_USER']); + parent::tearDown(); + } + + #[Test] + #[DataProvider('validHeaderProvider')] + public function headerWithAvailableLanguageSwitchesLanguage(string $header, int $expectedLanguageId): void + { + $request = (new RequestLanguageService())->withLanguageFromHeader($this->createRequest($header)); + + self::assertNull($request->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION)); + self::assertSame($expectedLanguageId, $request->getAttribute('language')->getLanguageId()); + self::assertSame( + $expectedLanguageId, + GeneralUtility::makeInstance(Context::class)->getPropertyFromAspect('language', 'id') + ); + } + + public static function validHeaderProvider(): array + { + return [ + 'enabled language' => ['1', 1], + 'surrounding whitespace' => [' 1 ', 1], + 'language from URL' => ['0', 0], + ]; + } + + #[Test] + #[DataProvider('invalidHeaderProvider')] + public function headerWithUnavailableLanguageIsRejected(string $header): void + { + $request = (new RequestLanguageService())->withLanguageFromHeader($this->createRequest($header)); + + $exception = $request->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION); + self::assertInstanceOf(BadRequestException::class, $exception); + self::assertSame(1793641754, $exception->getCode()); + self::assertSame(0, $request->getAttribute('language')->getLanguageId()); + } + + public static function invalidHeaderProvider(): array + { + return [ + 'not existing language' => ['99'], + 'negative ID' => ['-1'], + 'not a number' => ['abc'], + 'number with suffix' => ['1abc'], + 'disabled language' => ['2'], + ]; + } + + #[Test] + public function disabledLanguageIsAvailableForBackendUserWithAccess(): void + { + $backendUser = $this->createMock(BackendUserAuthentication::class); + $backendUser->expects(self::once())->method('checkLanguageAccess')->with(2)->willReturn(true); + $GLOBALS['BE_USER'] = $backendUser; + + $request = (new RequestLanguageService())->withLanguageFromHeader($this->createRequest('2')); + + self::assertNull($request->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION)); + self::assertSame(2, $request->getAttribute('language')->getLanguageId()); + } + + #[Test] + public function disabledLanguageIsRejectedForBackendUserWithoutAccess(): void + { + $backendUser = $this->createMock(BackendUserAuthentication::class); + $backendUser->expects(self::once())->method('checkLanguageAccess')->with(2)->willReturn(false); + $GLOBALS['BE_USER'] = $backendUser; + + $request = (new RequestLanguageService())->withLanguageFromHeader($this->createRequest('2')); + + self::assertInstanceOf( + BadRequestException::class, + $request->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION) + ); + } + + #[Test] + public function requestWithoutHeaderIsNotChanged(): void + { + $request = (new ServerRequest('https://example.com/_api/items')) + ->withAttribute('site', $this->site) + ->withAttribute('language', $this->site->getDefaultLanguage()); + + self::assertSame($request, (new RequestLanguageService())->withLanguageFromHeader($request)); + } + + #[Test] + public function requestWithEmptyHeaderIsNotChanged(): void + { + $request = $this->createRequest(' '); + + self::assertSame($request, (new RequestLanguageService())->withLanguageFromHeader($request)); + } + + private function createRequest(string $header): ServerRequest + { + return (new ServerRequest('https://example.com/_api/items')) + ->withHeader('X-Locale', $header) + ->withAttribute('site', $this->site) + ->withAttribute('language', $this->site->getDefaultLanguage()); + } +}