From adc95eec560a57847526e37d98cd0126a6d206c6 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Sat, 3 Mar 2018 13:47:09 +0000 Subject: [PATCH] Issue #2935193 by Sutharsan, alexpott: Fix broken exceptions in Gettext component --- .../lib/Drupal/Component/Gettext/PoHeader.php | 4 +- .../Component/Gettext/PoStreamReader.php | 6 +- .../Component/Gettext/PoStreamWriter.php | 14 +-- .../Component/Gettext/PoStreamWriterTest.php | 118 ++++++++++++++++++ 4 files changed, 131 insertions(+), 11 deletions(-) create mode 100644 core/tests/Drupal/Tests/Component/Gettext/PoStreamWriterTest.php diff --git a/core/lib/Drupal/Component/Gettext/PoHeader.php b/core/lib/Drupal/Component/Gettext/PoHeader.php index 5dd13a5d5b9d..96793b5e9562 100644 --- a/core/lib/Drupal/Component/Gettext/PoHeader.php +++ b/core/lib/Drupal/Component/Gettext/PoHeader.php @@ -188,7 +188,7 @@ public function __toString() { * - 'nplurals': The number of plural forms defined by the plural formula. * - 'plurals': Array of plural positions keyed by plural value. * - * @throws Exception + * @throws \Exception */ public function parsePluralForms($pluralforms) { $plurals = []; @@ -473,7 +473,7 @@ private function tokenizeFormula($formula) { * Number of the plural string to be used for the given plural value. * * @see parseArithmetic() - * @throws Exception + * @throws \Exception */ protected function evaluatePlural($element_stack, $n) { $count = count($element_stack); diff --git a/core/lib/Drupal/Component/Gettext/PoStreamReader.php b/core/lib/Drupal/Component/Gettext/PoStreamReader.php index f84d2514c0c8..9548ea7ea225 100644 --- a/core/lib/Drupal/Component/Gettext/PoStreamReader.php +++ b/core/lib/Drupal/Component/Gettext/PoStreamReader.php @@ -146,7 +146,7 @@ public function setURI($uri) { * Opens the stream and reads the header. The stream is ready for reading * items after. * - * @throws Exception + * @throws \Exception * If the URI is not yet set. */ public function open() { @@ -162,7 +162,7 @@ public function open() { /** * Implements Drupal\Component\Gettext\PoStreamInterface::close(). * - * @throws Exception + * @throws \Exception * If the stream is not open. */ public function close() { @@ -506,6 +506,8 @@ private function readLine() { $this->_errors[] = SafeMarkup::format('The translation stream %uri ended unexpectedly at line %line.', $log_vars); return FALSE; } + + return; } /** diff --git a/core/lib/Drupal/Component/Gettext/PoStreamWriter.php b/core/lib/Drupal/Component/Gettext/PoStreamWriter.php index 5419b210aafb..1e3f3871e399 100644 --- a/core/lib/Drupal/Component/Gettext/PoStreamWriter.php +++ b/core/lib/Drupal/Component/Gettext/PoStreamWriter.php @@ -81,7 +81,7 @@ public function open() { /** * Implements Drupal\Component\Gettext\PoStreamInterface::close(). * - * @throws Exception + * @throws \Exception * If the stream is not open. */ public function close() { @@ -89,7 +89,7 @@ public function close() { fclose($this->_fd); } else { - throw new Exception('Cannot close stream that is not open.'); + throw new \Exception('Cannot close stream that is not open.'); } } @@ -100,13 +100,13 @@ public function close() { * Piece of string to write to the stream. If the value is not directly a * string, casting will happen in writing. * - * @throws Exception + * @throws \Exception * If writing the data is not possible. */ private function write($data) { $result = fwrite($this->_fd, $data); - if ($result === FALSE) { - throw new Exception('Unable to write data: ' . substr($data, 0, 20)); + if ($result === FALSE || $result != strlen($data)) { + throw new \Exception('Unable to write data: ' . substr($data, 0, 20)); } } @@ -137,12 +137,12 @@ public function writeItems(PoReaderInterface $reader, $count = -1) { /** * Implements Drupal\Component\Gettext\PoStreamInterface::getURI(). * - * @throws Exception + * @throws \Exception * If the URI is not set. */ public function getURI() { if (empty($this->_uri)) { - throw new Exception('No URI set.'); + throw new \Exception('No URI set.'); } return $this->_uri; } diff --git a/core/tests/Drupal/Tests/Component/Gettext/PoStreamWriterTest.php b/core/tests/Drupal/Tests/Component/Gettext/PoStreamWriterTest.php new file mode 100644 index 000000000000..7f636c10866f --- /dev/null +++ b/core/tests/Drupal/Tests/Component/Gettext/PoStreamWriterTest.php @@ -0,0 +1,118 @@ +<?php + +namespace Drupal\Tests\Component\Gettext; + +use Drupal\Component\Gettext\PoItem; +use Drupal\Component\Gettext\PoStreamWriter; +use org\bovigo\vfs\vfsStream; +use org\bovigo\vfs\vfsStreamFile; +use PHPUnit\Framework\TestCase; + +/** + * @coversDefaultClass \Drupal\Component\Gettext\PoStreamWriter + * @group Gettext + */ +class PoStreamWriterTest extends TestCase { + + /** + * The PO writer object under test. + * + * @var \Drupal\Component\Gettext\PoStreamWriter + */ + protected $poWriter; + + /** + * The mock po file. + * + * @var \org\bovigo\vfs\vfsStreamFile + */ + protected $poFile; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $this->poWriter = new PoStreamWriter(); + + $root = vfsStream::setup(); + $this->poFile = new vfsStreamFile('powriter.po'); + $root->addChild($this->poFile); + } + + /** + * @covers ::getURI + */ + public function testGetUriException() { + if (method_exists($this, 'expectException')) { + $this->expectException(\Exception::class, 'No URI set.'); + } + else { + $this->setExpectedException(\Exception::class, 'No URI set.'); + } + + $this->poWriter->getURI(); + } + + /** + * @covers ::writeItem + * @dataProvider providerWriteData + */ + public function testWriteItem($poContent, $expected, $long) { + if ($long) { + if (method_exists($this, 'expectException')) { + $this->expectException(\Exception::class, 'Unable to write data:'); + } + else { + $this->setExpectedException(\Exception::class, 'Unable to write data:'); + } + } + + // Limit the file system quota to make the write fail on long strings. + vfsStream::setQuota(10); + + $this->poWriter->setURI($this->poFile->url()); + $this->poWriter->open(); + + $poItem = $this->prophesize(PoItem::class); + $poItem->__toString()->willReturn($poContent); + + $this->poWriter->writeItem($poItem->reveal()); + $this->poWriter->close(); + $this->assertEquals(file_get_contents($this->poFile->url()), $expected); + } + + /** + * @return array + * - Content to write. + * - Written content. + * - Content longer than 10 bytes. + */ + public function providerWriteData() { + return [ + ['', '', FALSE], + ["\r\n", "\r\n", FALSE], + ['write this if you can', 'write this', TRUE], + ['éáíó>&', 'éáíó>&', FALSE], + ['éáíó>&<', 'éáíó>&', TRUE], + ['中文 890', '中文 890', FALSE], + ['中文 89012', '中文 890', TRUE], + ]; + } + + /** + * @covers ::close + */ + public function testCloseException() { + if (method_exists($this, 'expectException')) { + $this->expectException(\Exception::class, 'Cannot close stream that is not open.'); + } + else { + $this->setExpectedException(\Exception::class, 'Cannot close stream that is not open.'); + } + + $this->poWriter->close(); + } + +} -- GitLab