Skip to content

Commit

Permalink
Backported #2945 "EZP-31298: Made non verbose exception messages in R…
Browse files Browse the repository at this point in the history
…EST responses" (#2947)
  • Loading branch information
webhdx authored Feb 19, 2020
1 parent 24c3443 commit c22b671
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@
<target>expected value to be of type '%expectedType%', got '%actualType%'</target>
<note>key: expected value to be of type '%expectedType%', got '%actualType%'</note>
</trans-unit>
<trans-unit id="9307ee9a550b5c318fbbd6f5750e95d1de1d3483" resname="non_verbose_error">
<source>An error has occurred. Please try again later or contact your Administrator.</source>
<target>An error has occurred. Please try again later or contact your Administrator.</target>
<note>key: non_verbose_error</note>
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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);

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);
}
}

0 comments on commit c22b671

Please sign in to comment.