From 5146c02c08aefa8496529de99b0f86f0b490ee46 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 14 Oct 2014 14:45:37 +0700 Subject: [PATCH 01/11] Fixes zendframework/zf2#6730 : close xml reader on fromFile after it open --- src/Reader/Xml.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Reader/Xml.php b/src/Reader/Xml.php index 348354f..cfb04c9 100644 --- a/src/Reader/Xml.php +++ b/src/Reader/Xml.php @@ -76,6 +76,7 @@ function ($error, $message = '', $file = '', $line = 0) use ($filename) { ); $return = $this->process(); restore_error_handler(); + $this->reader->close(); return $return; } From 1c7ebee78ee1f7a5f3ca61146d9e790967bbad42 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 14 Oct 2014 18:46:34 +0700 Subject: [PATCH 02/11] added close() to fromString() too --- src/Reader/Xml.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Reader/Xml.php b/src/Reader/Xml.php index cfb04c9..6c5edaa 100644 --- a/src/Reader/Xml.php +++ b/src/Reader/Xml.php @@ -111,6 +111,7 @@ function ($error, $message = '', $file = '', $line = 0) { ); $return = $this->process(); restore_error_handler(); + $this->reader->close(); return $return; } From ad4ca1fa0cd444426462f0ea529028c097a2fbfa Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 22 Dec 2014 17:39:27 +0700 Subject: [PATCH 03/11] update check setParserProperty to check validity after closed --- src/Reader/Xml.php | 14 ++++++++++- test/Reader/XmlTest.php | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/Reader/Xml.php b/src/Reader/Xml.php index 6c5edaa..fd32a4c 100644 --- a/src/Reader/Xml.php +++ b/src/Reader/Xml.php @@ -43,6 +43,19 @@ class Xml implements ReaderInterface XMLReader::SIGNIFICANT_WHITESPACE ); + /** + * Get reader + * + * @return XMLReader + */ + public function getReader() + { + if ($this->reader === null) { + $this->reader = new XMLReader(); + } + return $this->reader; + } + /** * fromFile(): defined by Reader interface. * @@ -59,7 +72,6 @@ public function fromFile($filename) $filename )); } - $this->reader = new XMLReader(); $this->reader->open($filename, null, LIBXML_XINCLUDE); diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index ee3ffe9..01ae0dd 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -9,6 +9,7 @@ namespace ZendTest\Config\Reader; +use XMLReader; use Zend\Config\Reader\Xml; /** @@ -121,4 +122,54 @@ public function testElementWithBothAttributesAndAStringValueIsProcessedCorrectly $this->assertArrayHasKey('_', $arrayXml['one'][1]); $this->assertEquals('bazbat', $arrayXml['one'][1]['_']); } + + /** + * @group 6761 + * @group 6730 + */ + public function testCloseWhenCallFromFileReaderGetInvalid() + { + $configReader = new Xml(); + $configReader->getReader()->open($this->getTestAssetPath('attributes')); + $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + $this->assertTrue($configReader->getReader()->isValid()); + + $configReader->fromFile($this->getTestAssetPath('attributes')); + $this->setExpectedException('PHPUnit_Framework_Error_Warning'); + $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + } + + /** + * @group 6761 + * @group 6730 + */ + public function testCloseWhenCallFromStringReaderGetInvalid() + { + $xml = << + + foo + baz + foo + + +ECS; + + $configReader = new Xml(); + $configReader->getReader()->xml($xml, null, LIBXML_XINCLUDE); + $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + $this->assertTrue($configReader->getReader()->isValid()); + $xml = << + + foo + baz + foo + + +ECS; + $configReader->fromString($xml); + $this->setExpectedException('PHPUnit_Framework_Error_Warning'); + $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + } } From 06264ca32fe8cb95286437ec7c4ed1e9c7ef31e5 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 22 Dec 2014 17:49:00 +0700 Subject: [PATCH 04/11] remove unused $xml --- test/Reader/XmlTest.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index 01ae0dd..3e9c467 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -159,15 +159,7 @@ public function testCloseWhenCallFromStringReaderGetInvalid() $configReader->getReader()->xml($xml, null, LIBXML_XINCLUDE); $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); $this->assertTrue($configReader->getReader()->isValid()); - $xml = << - - foo - baz - foo - -ECS; $configReader->fromString($xml); $this->setExpectedException('PHPUnit_Framework_Error_Warning'); $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); From 1cf0b93f92fe9368812ae25a52b6c91bf1a3c188 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:17:32 +0100 Subject: [PATCH 05/11] zendframework/zf2#6730 zendframework/zf2#6761 - config reader should disallow opening non existing files --- test/Reader/XmlTest.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index 3e9c467..e93e613 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -14,6 +14,8 @@ /** * @group Zend_Config + * + * @covers \Zend\Config\Reader\Xml */ class XmlTest extends AbstractReaderTestCase { @@ -123,6 +125,28 @@ public function testElementWithBothAttributesAndAStringValueIsProcessedCorrectly $this->assertEquals('bazbat', $arrayXml['one'][1]['_']); } + /** + * @group 6761 + * @group 6730 + */ + public function testReadRemovedFilesWillFailWithException() + { + $configReader = new Xml(); + + $asset = $this->getTestAssetPath('attributes'); + $tmpAssetPath = tempnam(sys_get_temp_dir(), 'attributesTestAsset'); + + copy($asset, $tmpAssetPath); + + $configReader->fromFile($tmpAssetPath); + + unlink($tmpAssetPath); + + $this->setExpectedException('Zend\Config\Exception\RuntimeException'); + + $configReader->fromFile($tmpAssetPath); + } + /** * @group 6761 * @group 6730 From c27b2a00207187c422bc390cd43868ad10a4d775 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:25:48 +0100 Subject: [PATCH 06/11] zendframework/zf2#6730 zendframework/zf2#6761 - using reflection to access the internal config reader --- test/Reader/XmlTest.php | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index e93e613..ba0e954 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -9,6 +9,7 @@ namespace ZendTest\Config\Reader; +use ReflectionProperty; use XMLReader; use Zend\Config\Reader\Xml; @@ -154,9 +155,6 @@ public function testReadRemovedFilesWillFailWithException() public function testCloseWhenCallFromFileReaderGetInvalid() { $configReader = new Xml(); - $configReader->getReader()->open($this->getTestAssetPath('attributes')); - $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); - $this->assertTrue($configReader->getReader()->isValid()); $configReader->fromFile($this->getTestAssetPath('attributes')); $this->setExpectedException('PHPUnit_Framework_Error_Warning'); @@ -180,12 +178,33 @@ public function testCloseWhenCallFromStringReaderGetInvalid() ECS; $configReader = new Xml(); - $configReader->getReader()->xml($xml, null, LIBXML_XINCLUDE); - $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); - $this->assertTrue($configReader->getReader()->isValid()); $configReader->fromString($xml); + + $xmlReader = $this->getInternalXmlReader($configReader); + $this->setExpectedException('PHPUnit_Framework_Error_Warning'); - $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + + $xmlReader->setParserProperty(XMLReader::VALIDATE, true); + } + + /** + * Reads the internal XML reader from a given Xml config reader + * + * @param Xml $xml + * + * @return XMLReader + */ + private function getInternalXmlReader(Xml $xml) + { + $reflectionReader = new ReflectionProperty('Zend\Config\Reader\Xml', 'reader'); + + $reflectionReader->setAccessible(true); + + $xmlReader = $reflectionReader->getValue($xml); + + $this->assertInstanceOf('XMLReader', $xmlReader); + + return $xmlReader; } } From 9d716f33bd69b8a40e0797120ae63d4c899eaf0e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:26:37 +0100 Subject: [PATCH 07/11] zendframework/zf2#6730 zendframework/zf2#6761 - using reflection to access the internal config reader --- test/Reader/XmlTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index ba0e954..5d1de3e 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -157,8 +157,11 @@ public function testCloseWhenCallFromFileReaderGetInvalid() $configReader = new Xml(); $configReader->fromFile($this->getTestAssetPath('attributes')); + + $xmlReader = $this->getInternalXmlReader($configReader); + $this->setExpectedException('PHPUnit_Framework_Error_Warning'); - $configReader->getReader()->setParserProperty(XMLReader::VALIDATE, true); + $xmlReader->setParserProperty(XMLReader::VALIDATE, true); } /** @@ -184,7 +187,6 @@ public function testCloseWhenCallFromStringReaderGetInvalid() $xmlReader = $this->getInternalXmlReader($configReader); $this->setExpectedException('PHPUnit_Framework_Error_Warning'); - $xmlReader->setParserProperty(XMLReader::VALIDATE, true); } From 2f5b7f6145a837680492befd4db803c22f172158 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:26:59 +0100 Subject: [PATCH 08/11] zendframework/zf2#6730 zendframework/zf2#6761 - removing newly introduced getter (not needed) --- src/Reader/Xml.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Reader/Xml.php b/src/Reader/Xml.php index fd32a4c..4961c76 100644 --- a/src/Reader/Xml.php +++ b/src/Reader/Xml.php @@ -43,19 +43,6 @@ class Xml implements ReaderInterface XMLReader::SIGNIFICANT_WHITESPACE ); - /** - * Get reader - * - * @return XMLReader - */ - public function getReader() - { - if ($this->reader === null) { - $this->reader = new XMLReader(); - } - return $this->reader; - } - /** * fromFile(): defined by Reader interface. * From a3ee4bb5bb8f33938d13b1cebd7acc04bb430837 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:28:22 +0100 Subject: [PATCH 09/11] zendframework/zf2#6730 zendframework/zf2#6761 - simplifying test case for non-existing paths --- test/Reader/XmlTest.php | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index 5d1de3e..8b9b2ed 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -130,22 +130,13 @@ public function testElementWithBothAttributesAndAStringValueIsProcessedCorrectly * @group 6761 * @group 6730 */ - public function testReadRemovedFilesWillFailWithException() + public function testReadNonExistingFilesWillFailWithException() { $configReader = new Xml(); - $asset = $this->getTestAssetPath('attributes'); - $tmpAssetPath = tempnam(sys_get_temp_dir(), 'attributesTestAsset'); - - copy($asset, $tmpAssetPath); - - $configReader->fromFile($tmpAssetPath); - - unlink($tmpAssetPath); - $this->setExpectedException('Zend\Config\Exception\RuntimeException'); - $configReader->fromFile($tmpAssetPath); + $configReader->fromFile(sys_get_temp_dir() . '/path/that/does/not/exist'); } /** From def47087381bb381bae5aff11ffdba6b894c5fc6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:28:47 +0100 Subject: [PATCH 10/11] zendframework/zf2#6730 zendframework/zf2#6761 - removing unused variable --- test/Reader/XmlTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index 8b9b2ed..599dab6 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -76,7 +76,7 @@ public function testInvalidString() ECS; $this->setExpectedException('Zend\Config\Exception\RuntimeException'); - $arrayXml = $this->reader->fromString($xml); + $this->reader->fromString($xml); } public function testZF300_MultipleKeysOfTheSameName() From 138488a86354ffdaf832cd4462c5ed36fbd31590 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 31 Dec 2014 10:30:45 +0100 Subject: [PATCH 11/11] zendframework/zf2#6730 zendframework/zf2#6761 - clarifying on weird warning assertions --- test/Reader/XmlTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/Reader/XmlTest.php b/test/Reader/XmlTest.php index 599dab6..6a945e7 100644 --- a/test/Reader/XmlTest.php +++ b/test/Reader/XmlTest.php @@ -152,6 +152,8 @@ public function testCloseWhenCallFromFileReaderGetInvalid() $xmlReader = $this->getInternalXmlReader($configReader); $this->setExpectedException('PHPUnit_Framework_Error_Warning'); + + // following operation should fail because the internal reader is closed (and expected to be closed) $xmlReader->setParserProperty(XMLReader::VALIDATE, true); } @@ -178,6 +180,8 @@ public function testCloseWhenCallFromStringReaderGetInvalid() $xmlReader = $this->getInternalXmlReader($configReader); $this->setExpectedException('PHPUnit_Framework_Error_Warning'); + + // following operation should fail because the internal reader is closed (and expected to be closed) $xmlReader->setParserProperty(XMLReader::VALIDATE, true); }