From 55af30811d9d6f09b67e5fc864bde6320eb89cde Mon Sep 17 00:00:00 2001 From: Maciej Brencz Date: Thu, 11 Jun 2026 15:08:12 +0100 Subject: [PATCH] XMLParser: fix premature stream termination on empty mid-stream reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stream_get_contents() returns '' (empty string) at EOF, not false, so the previous $isFinal = ($data === false) check never triggered, and an empty read caused the nodesQueue to stay empty and valid() to return false — silently stopping iteration mid-document. Compressed or network streams (e.g. compress.zlib:// over HTTP) can also return '' between internal block boundaries (zlib flush markers, TCP chunk boundaries) without being at EOF. The fix wraps the read loop so parseNextChunk() keeps reading until either at least one node is queued or feof() confirms the stream is truly exhausted, and a streamFinished flag prevents calling xml_parse() on an already-finalised parser. Fixes https://github.com/elecena/xml-iterator/issues/13 Co-Authored-By: Claude Sonnet 4.6 --- src/XMLParser.php | 39 ++++++-- tests/XMLParserNestedElementsTest.php | 139 ++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 11 deletions(-) create mode 100644 tests/XMLParserNestedElementsTest.php diff --git a/src/XMLParser.php b/src/XMLParser.php index 3005b66..8834106 100644 --- a/src/XMLParser.php +++ b/src/XMLParser.php @@ -19,6 +19,8 @@ class XMLParser implements \Iterator private string $currentTagContent = ''; + private bool $streamFinished = false; + /** * The stack of the XML node names as go deeper into the tree. * @@ -51,6 +53,7 @@ public function setUp(): void $this->currentTagName = null; $this->currentTagAttributes = []; $this->nodeNamesStack = []; + $this->streamFinished = false; $this->parser = \xml_parser_create(); @@ -179,25 +182,39 @@ public function rewind(): void * * Callbacks are called, nodes are pushed to the stack and iterator can go over them. * + * Loops until at least one node is queued or the stream is exhausted. + * This handles compressed or network streams that may return empty reads mid-stream + * (e.g. zlib flush markers or TCP packet boundaries) without signalling EOF. + * * @return void * @throws ParsingError */ private function parseNextChunk(): void { - // the nodes stack has been iterated over, consume and parse the next piece of the XML stream - $data = stream_get_contents($this->stream, length: self::BATCH_READ_SIZE); - $isFinal = ($data === false); + if ($this->streamFinished) { + return; + } - $res = xml_parse($this->parser, $data, $isFinal); + while (empty($this->nodesQueue)) { + $data = stream_get_contents($this->stream, length: self::BATCH_READ_SIZE); + $isFinal = ($data === false || feof($this->stream)); - if ($res === 0 /* returns 0 on failure */) { - // take more details from the parser instance and throw an exception - throw ParsingError::fromParserInstance($this->parser, is_string($data) ? $data : null); - } + if ($data === false) { + $data = ''; + } + + $res = xml_parse($this->parser, $data, $isFinal); - // we're done with reading and parsing the stream, close the XML parser instance - if ($isFinal) { - $this->close(); + if ($res === 0 /* returns 0 on failure */) { + // take more details from the parser instance and throw an exception + throw ParsingError::fromParserInstance($this->parser, $data !== '' ? $data : null); + } + + if ($isFinal) { + $this->streamFinished = true; + $this->close(); + return; + } } } diff --git a/tests/XMLParserNestedElementsTest.php b/tests/XMLParserNestedElementsTest.php new file mode 100644 index 0000000..0d53702 --- /dev/null +++ b/tests/XMLParserNestedElementsTest.php @@ -0,0 +1,139 @@ +position = 0; + $this->readCount = 0; + return true; + } + + public function stream_read(int $count): string + { + $this->readCount++; + + // Return empty string every N reads unless already at EOF + if ($this->readCount % self::$emptyEveryN === 0 && !$this->stream_eof()) { + return ''; + } + + $chunk = substr(self::$content, $this->position, $count); + $this->position += strlen($chunk); + return $chunk; + } + + public function stream_eof(): bool + { + return $this->position >= strlen(self::$content); + } + + public function stream_stat(): array + { + return []; + } +} + +/** + * Tests XML parsing of nested elements that share the same tag name, + * as seen in the Discogs data dump (label > sublabels > label). + * + * @see https://github.com/elecena/xml-iterator/issues/13 + */ +class XMLParserNestedElementsTest extends XMLParserTestCase +{ + private static string $xml = ''; + + public static function setUpBeforeClass(): void + { + $xml = ''; + for ($i = 0; $i < 300; $i++) { + $xml .= sprintf( + '', + $i, + $i, + $i + ); + } + $xml .= ''; + self::$xml = $xml; + } + + protected function tearDown(): void + { + IntermittentReadStreamWrapper::unregister(); + } + + protected function getParserStream() + { + return self::streamFromString(self::$xml); + } + + public function testParsesNestedSameNameElements(): void + { + $labelCount = 0; + + foreach ($this->getParser() as $node) { + if ($node instanceof XMLNodeContent && $node->name === 'label') { + $labelCount++; + } + } + + // 300 outer labels + 300 inner labels inside sublabels + $this->assertSame(600, $labelCount); + } + + public function testParsesStreamWithIntermittentEmptyReads(): void + { + $url = IntermittentReadStreamWrapper::open(self::$xml, emptyEveryN: 3); + $parser = new XMLParser(stream: fopen($url, 'r')); + + $labelCount = 0; + + foreach ($parser as $node) { + if ($node instanceof XMLNodeContent && $node->name === 'label') { + $labelCount++; + } + } + + // 300 outer labels + 300 inner labels inside sublabels + $this->assertSame(600, $labelCount); + } +}