From 3c3c8562097d78ddc9d3569ba1e46735fbaf8ea2 Mon Sep 17 00:00:00 2001 From: Maciej Kobus Date: Tue, 18 Feb 2020 09:36:09 +0100 Subject: [PATCH] Backported #2945 "EZP-31298: Made non verbose exception messages in REST responses" --- .travis.yml | 2 +- .../translations/repository_exceptions.en.xlf | 5 + .../Output/ValueObjectVisitor/Exception.php | 13 ++- .../ValueObjectVisitor/ExceptionTest.php | 103 +++++++++++++++--- 4 files changed, 102 insertions(+), 21 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8212bccca36..8da48f5bad9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,7 +50,7 @@ matrix: - php: 7.1 env: PHP_IMAGE=ezsystems/php:7.1-v1 REST_TEST_CONFIG="phpunit-functional-rest.xml" RUN_INSTALL=1 SYMFONY_ENV=behat SYMFONY_DEBUG=1 SF_CMD="ez:behat:create-language 'pol-PL' 'Polish (polski)'" - php: 7.1 - env: PHP_IMAGE=ezsystems/php:7.1-v1 BEHAT_OPTS="--profile=rest --tags=~@broken --suite=fullJson" RUN_INSTALL=1 SYMFONY_ENV=behat + env: PHP_IMAGE=ezsystems/php:7.1-v1 BEHAT_OPTS="--profile=rest --tags=~@broken --suite=fullJson" RUN_INSTALL=1 SYMFONY_ENV=behat SYMFONY_DEBUG=1 - php: 7.1 env: TEST_CONFIG="phpunit-integration-legacy.xml" DB="mysql" DATABASE="mysql://root@localhost/testdb" CUSTOM_CACHE_POOL="singleredis" REDIS_ENABLE_LZF="true" REDIS_ENABLE_IGBINARY="true" SYMFONY_VERSION="~3.4.26" # 7.2 diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/translations/repository_exceptions.en.xlf b/eZ/Bundle/EzPublishCoreBundle/Resources/translations/repository_exceptions.en.xlf index 24c34390f0e..eaac1bf9fae 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/translations/repository_exceptions.en.xlf +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/translations/repository_exceptions.en.xlf @@ -233,6 +233,11 @@ expected value to be of type '%expectedType%', got '%actualType%' key: expected value to be of type '%expectedType%', got '%actualType%' + + An error has occurred. Please try again later or contact your Administrator. + An error has occurred. Please try again later or contact your Administrator. + key: non_verbose_error + diff --git a/eZ/Publish/Core/REST/Server/Output/ValueObjectVisitor/Exception.php b/eZ/Publish/Core/REST/Server/Output/ValueObjectVisitor/Exception.php index a3f4c7ac737..0123eed081f 100644 --- a/eZ/Publish/Core/REST/Server/Output/ValueObjectVisitor/Exception.php +++ b/eZ/Publish/Core/REST/Server/Output/ValueObjectVisitor/Exception.php @@ -117,12 +117,17 @@ public function visit(Visitor $visitor, Generator $generator, $data) $generator->startValueElement('errorMessage', $this->httpStatusCodes[$statusCode]); $generator->endValueElement('errorMessage'); - if ($data instanceof Translatable && $this->translator) { - /** @Ignore */ - $errorDescription = $this->translator->trans($data->getMessageTemplate(), $data->getParameters(), 'repository_exceptions'); + if ($this->debug || $statusCode < 500) { + $errorDescription = $data instanceof Translatable && $this->translator + ? /** @Ignore */ $this->translator->trans($data->getMessageTemplate(), $data->getParameters(), 'repository_exceptions') + : $data->getMessage(); } else { - $errorDescription = $data->getMessage(); + // Do not leak any file paths and sensitive data on production environments + $errorDescription = $this->translator + ? /** @Desc("An error has occurred. Please try again later or contact your Administrator.") */ $this->translator->trans('non_verbose_error', [], 'repository_exceptions') + : 'An error has occurred. Please try again later or contact your Administrator.'; } + $generator->startValueElement('errorDescription', $errorDescription); $generator->endValueElement('errorDescription'); diff --git a/eZ/Publish/Core/REST/Server/Tests/Output/ValueObjectVisitor/ExceptionTest.php b/eZ/Publish/Core/REST/Server/Tests/Output/ValueObjectVisitor/ExceptionTest.php index b55bffb9b69..25260370164 100644 --- a/eZ/Publish/Core/REST/Server/Tests/Output/ValueObjectVisitor/ExceptionTest.php +++ b/eZ/Publish/Core/REST/Server/Tests/Output/ValueObjectVisitor/ExceptionTest.php @@ -8,11 +8,21 @@ */ namespace eZ\Publish\Core\REST\Server\Tests\Output\ValueObjectVisitor; +use DOMDocument; +use DOMXPath; +use eZ\Publish\Core\REST\Common\Output\Generator; +use eZ\Publish\Core\REST\Common\Output\ValueObjectVisitor; +use eZ\Publish\Core\REST\Server\Output\ValueObjectVisitor\Exception as ExceptionValueObjectVisitor; use eZ\Publish\Core\REST\Common\Tests\Output\ValueObjectVisitorBaseTest; -use eZ\Publish\Core\REST\Server\Output\ValueObjectVisitor; +use Symfony\Component\Translation\TranslatorInterface; class ExceptionTest extends ValueObjectVisitorBaseTest { + const NON_VERBOSE_ERROR_DESCRIPTION = 'An error has occurred. Please try again later or contact your Administrator.'; + + /** @var \Symfony\Component\Translation\TranslatorInterface|\PHPUnit\Framework\MockObject\MockObject */ + private $translatorMock; + /** * Test the Exception visitor. * @@ -23,24 +33,27 @@ public function testVisit() $visitor = $this->getVisitor(); $generator = $this->getGenerator(); - $generator->startDocument(null); + $result = $this->generateDocument($generator, $visitor); - $previousException = new \Exception('Sub-test'); - $exception = new \Exception('Test', 0, $previousException); + $this->assertNotNull($result); - $this - ->getVisitorMock() - ->expects($this->once()) - ->method('visitValueObject') - ->with($previousException); + return $result; + } - $visitor->visit( - $this->getVisitorMock(), - $generator, - $exception - ); + public function testVisitNonVerbose() + { + $this->getTranslatorMock()->method('trans') + ->with('non_verbose_error', [], 'repository_exceptions') + ->willReturn(self::NON_VERBOSE_ERROR_DESCRIPTION); + + $visitor = $this->internalGetNonDebugVisitor(); + $visitor->setRequestParser($this->getRequestParser()); + $visitor->setRouter($this->getRouterMock()); + $visitor->setTemplateRouter($this->getTemplatedRouterMock()); - $result = $generator->endDocument(null); + $generator = $this->getGenerator(); + + $result = $this->generateDocument($generator, $visitor); $this->assertNotNull($result); @@ -112,6 +125,21 @@ public function testResultContainsErrorDescription($result) ); } + /** + * @depends testVisitNonVerbose + */ + public function testNonVerboseErrorDescription($result) + { + $document = new DOMDocument(); + $document->loadXML($result); + $xpath = new DOMXPath($document); + + $nodeList = $xpath->query('//ErrorMessage/errorDescription'); + $errorDescriptionNode = $nodeList->item(0); + + $this->assertEquals(self::NON_VERBOSE_ERROR_DESCRIPTION, $errorDescriptionNode->textContent); + } + /** * Test if ErrorMessage element contains required attributes. * @@ -186,6 +214,49 @@ protected function getException() */ protected function internalGetVisitor() { - return new ValueObjectVisitor\Exception(); + return new ExceptionValueObjectVisitor(true, $this->getTranslatorMock()); + } + + /** + * Gets the exception visitor. + * + * @return \eZ\Publish\Core\REST\Server\Output\ValueObjectVisitor\Exception + */ + protected function internalGetNonDebugVisitor() + { + return new ExceptionValueObjectVisitor(false, $this->getTranslatorMock()); + } + + protected function getTranslatorMock() + { + if (!isset($this->translatorMock)) { + $this->translatorMock = $this->getMockBuilder(TranslatorInterface::class) + ->disableOriginalConstructor() + ->getMock(); + } + + return $this->translatorMock; + } + + private function generateDocument(Generator\Xml $generator, ValueObjectVisitor $visitor) + { + $generator->startDocument(null); + + $previousException = new \Exception('Sub-test'); + $exception = new \Exception('Test', 0, $previousException); + + $this + ->getVisitorMock() + ->expects($this->once()) + ->method('visitValueObject') + ->with($previousException); + + $visitor->visit( + $this->getVisitorMock(), + $generator, + $exception + ); + + return $generator->endDocument(null); } }