Fix processing instructions gaining an extra "?" when an HTML-parsed document is serialized

CHANGELOG.md

@@ -12,6 +12,7 @@ QueryPath Changelog
 - Add `QueryPathTests\ExamplesTest`, which runs every offline example on each supported PHP version and fails if one stops working. The examples that call third-party services are run by the new `Examples` workflow, weekly and whenever an example changes
 - Rewrite the cURL example against the PubMed E-utilities API. MusicBrainz throttles by IP address, which made the example unusable from any shared address
 - Add `composer run test:examples` (and `test:examples:network`) to run the examples locally
+- Fix processing instructions gaining an extra `?` each time an HTML-parsed document was serialized, so `<?php echo $title; ?>` came back out of `html()`, `innerHTML()`, `innerXML()`, `innerXHTML()`, `xml()`, `html5()`, `innerHTML5()`, and `writeXML()` as `<?php echo $title; ??>`. libxml's HTML parser keeps the closing `?` as part of the node's data, unlike its XML parser and the Masterminds HTML5 parser, so QueryPath now strips it on load. `DOMProcessingInstruction::$data` consequently no longer has a stray `?` on the end for documents read with `htmlqp()` or `qp()` on an `.html` file
 
 # 4.1.0
 

src/DOM.php

@@ -5,6 +5,7 @@
 use Countable;
 use DOMDocument;
 use DOMNode;
+use DOMXPath;
 use IteratorAggregate;
 use Masterminds\HTML5;
 use QueryPath\CSS\DOMTraverser;
@@ -204,8 +205,12 @@ private function parseXMLString($string, $flags = 0)
 				$useParser = strtolower($this->options['use_parser']);
 			}
 
+			// Only the libxml HTML parser leaves the closing "?" in processing-instruction data.
+			$usedHTMLParser = false;
+
 			// If HTML parser is requested, we use it.
 			if ($useParser === 'html') {
+				$usedHTMLParser = true;
 				$document->loadHTML($string);
 			} // Parse as XML if it looks like XML, or if XML parser is requested.
 			elseif ($lead === '<?xml' || $useParser === 'xml') {
@@ -215,6 +220,7 @@ private function parseXMLString($string, $flags = 0)
 				$document->loadXML($string, $flags);
 			} // In all other cases, we try the HTML parser.
 			else {
+				$usedHTMLParser = true;
 				$document->loadHTML($string);
 			}
 		} // Emulate 'finally' behavior.
@@ -228,9 +234,63 @@ private function parseXMLString($string, $flags = 0)
 			throw new ParseException('Unknown parser exception.');
 		}
 
+		if ($usedHTMLParser) {
+			self::normalizeProcessingInstructions($document);
+		}
+
 		return $document;
 	}
 
+	/**
+	 * Strip the closing "?" that libxml's HTML parser leaves in processing instruction data.
+	 *
+	 * libxml's HTML parser stores the terminating "?" of a `<?php ... ?>` block as part of the
+	 * node's data, while its XML parser -- and the Masterminds HTML5 parser -- do not. Leaving it
+	 * in place corrupts every serializer that appends its own "?>": `saveXML()` turns
+	 * `<?php echo $a; ?>` into `<?php echo $a; ??>`, and each further round trip adds another "?".
+	 *
+	 * Normalising on load gives every parser QueryPath supports the same invariant -- processing
+	 * instruction data never contains the closing "?" -- which fixes the serializers and also means
+	 * a caller reading `$pi->data` gets usable source rather than source with a stray "?" glued to
+	 * the end.
+	 *
+	 * Exactly one "?" is removed, so a processing instruction whose content legitimately ends in
+	 * "?" (`<?php $a = 1; ??>`) still round trips correctly.
+	 *
+	 * @param DOMDocument $document
+	 *
+	 * @return void
+	 */
+	protected static function normalizeProcessingInstructions(DOMDocument $document)
+	{
+		foreach (self::processingInstructions($document) as $instruction) {
+			if (substr($instruction->data, -1) === '?') {
+				$instruction->data = substr($instruction->data, 0, -1);
+			}
+		}
+	}
+
+	/**
+	 * Every processing instruction in a document.
+	 *
+	 * The invariant that PI data never carries its closing "?" is established here on load and
+	 * paid back by the HTML serializers, so both sides select the nodes the same way.
+	 *
+	 * @param DOMDocument $document
+	 *
+	 * @return array
+	 */
+	protected static function processingInstructions(DOMDocument $document): array
+	{
+		$found = [];
+		$xpath = new DOMXPath($document);
+		foreach ($xpath->query('//processing-instruction()') as $instruction) {
+			$found[] = $instruction;
+		}
+
+		return $found;
+	}
+
 	/**
 	 * EXPERT: Be very, very careful using this.
 	 * A utility function for setting the current set of matches.
@@ -471,6 +531,9 @@ private function parseXMLFile($filename, $flags = 0, $context = null)
 
 		$ext = $lastDot !== false ? strtolower(substr($filename, $lastDot)) : '';
 
+		// Only the libxml HTML parser leaves the closing "?" in processing-instruction data.
+		$usedHTMLParser = false;
+
 		try {
 			set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes);
 
@@ -480,6 +543,7 @@ private function parseXMLFile($filename, $flags = 0, $context = null)
 			} // Otherwise, see if it looks like HTML.
 			elseif ($useParser === 'html' || isset($htmlExtensions[$ext])) {
 				// Try parsing it as HTML.
+				$usedHTMLParser = true;
 				$document->loadHTMLFile($filename);
 			} // Default to XML.
 			else {
@@ -493,6 +557,10 @@ private function parseXMLFile($filename, $flags = 0, $context = null)
 		}
 		restore_error_handler();
 
+		if ($usedHTMLParser) {
+			self::normalizeProcessingInstructions($document);
+		}
+
 		return $document;
 	}
 

src/DOMQuery.php

@@ -700,13 +700,51 @@ public function html($markup = null)
 		}
 
 		if ($first instanceof DOMDocument || $first->isSameNode($first->ownerDocument->documentElement)) {
-			return $this->document->saveHTML();
+			return $this->saveDocumentHTML();
 		}
 
 		// saveHTML cannot take a node and serialize it.
 		return $this->document->saveXML($first);
 	}
 
+	/**
+	 * Serialize the whole document with libxml's HTML serializer.
+	 *
+	 * libxml writes a processing instruction verbatim as `<?target data>` -- unlike its XML
+	 * serializer, and unlike the Masterminds HTML5 serializer, it never appends the closing "?"
+	 * itself. QueryPath normalises processing instruction data on load so that the "?" is never
+	 * part of the data (see DOM::normalizeProcessingInstructions()), so it has to be put back for
+	 * the duration of the write and taken off again afterwards.
+	 *
+	 * @param string|null $path
+	 *  When given, the document is written to this file rather than returned.
+	 *
+	 * @return string|null
+	 */
+	private function saveDocumentHTML($path = null)
+	{
+		// libxml's HTML serializer writes a PI verbatim and never adds the closing "?", so the
+		// terminator stripped on load is put back for the duration of the write.
+		$instructions = self::processingInstructions($this->document);
+		foreach ($instructions as $instruction) {
+			$instruction->data = $instruction->data . '?';
+		}
+
+		try {
+			if ($path === null) {
+				return $this->document->saveHTML();
+			}
+
+			$this->document->saveHTMLFile($path);
+
+			return null;
+		} finally {
+			foreach ($instructions as $instruction) {
+				$instruction->data = substr($instruction->data, 0, -1);
+			}
+		}
+	}
+
 	/**
 	 * Set or get the markup for an element using the HTML5 parser
 	 *
@@ -1335,11 +1373,11 @@ public function writeXML($path = null, $options = 0)
 	public function writeHTML($path = null)
 	{
 		if ($path === null) {
-			print $this->document->saveHTML();
+			print $this->saveDocumentHTML();
 		} else {
 			try {
 				set_error_handler(['\QueryPath\ParseException', 'initializeFromError']);
-				$this->document->saveHTMLFile($path);
+				$this->saveDocumentHTML($path);
 			} catch (Exception $e) {
 				restore_error_handler();
 				throw $e;