Restore the error handler in finally, through one shared helper

Seven blocks installed initializeFromError() as the error handler and
restored it only from catch (QueryPath\Exception), so any other
throwable left the handler installed. Route them all through
DOM::withErrorHandler(), which restores it in finally.

Fixes #95

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: gravitypdf-bot

CHANGELOG.md

@@ -27,6 +27,7 @@ QueryPath Changelog
 - Fix the `:text` pseudo-class so it matches jQuery: it selects `input` elements whose `type` attribute is absent or is `text` (case-insensitively). It never indicated, and still does not indicate, whether a node is a text node
 - `find('*')` now matches the nodes in the match set as well as their descendants, so that a selector can be tested against an element already in hand
 - `writeHTML5()` now returns the `DOMQuery` like the other `write*()` methods, and throws `QueryPath\IOException` instead of a `TypeError` when the file cannot be written (issue #80)
+- Fix QueryPath's error handler staying installed for the rest of the request when an exception other than a `QueryPath\Exception` (a `TypeError`, or one thrown by a stream wrapper, for example) escaped from parsing a document, inserting markup, or `writeXML()`/`writeHTML()` (issue #95)
 
 # 4.1.0
 

src/DOM.php

@@ -177,9 +177,7 @@ private function parseXMLString($string, $flags = 0)
 	{
 		$document = self::createDocument('1.0');
 		$lead     = strtolower(substr($string, 0, 5)); // <?xml
-		try {
-			set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes);
-
+		self::withErrorHandler(ParseException::class, function () use ($document, $string, $flags, $lead) {
 			if (isset($this->options['convert_to_encoding'])) {
 				// Is there another way to do this?
 
@@ -217,12 +215,7 @@ private function parseXMLString($string, $flags = 0)
 			else {
 				self::loadHTMLString($document, $string);
 			}
-		} // Emulate 'finally' behavior.
-		catch (Exception $e) {
-			restore_error_handler();
-			throw $e;
-		}
-		restore_error_handler();
+		}, $this->errTypes);
 
 		if (empty($document)) {
 			throw new ParseException('Unknown parser exception.');
@@ -453,15 +446,9 @@ protected function prepareInsert($item)
 			}
 
 			$frag = $this->document->createDocumentFragment();
-			try {
-				set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes);
+			self::withErrorHandler(ParseException::class, function () use ($frag, $item) {
 				$frag->appendXML($item);
-			} // Simulate a finally block.
-			catch (Exception $e) {
-				restore_error_handler();
-				throw $e;
-			}
-			restore_error_handler();
+			}, $this->errTypes);
 
 			return $frag;
 		}
@@ -498,6 +485,28 @@ protected function prepareInsert($item)
 		throw new Exception('Cannot prepare item of unsupported type: ' . gettype($item));
 	}
 
+	/**
+	 * Run a callback with PHP errors converted into exceptions.
+	 *
+	 * @param string   $exceptionClass
+	 *  ParseException or a subclass, whose initializeFromError() handles the errors.
+	 * @param callable $callback
+	 * @param int      $types
+	 *  The error levels to convert.
+	 *
+	 * @return mixed
+	 *  Whatever the callback returns.
+	 */
+	protected static function withErrorHandler($exceptionClass, callable $callback, $types = E_ALL)
+	{
+		set_error_handler([$exceptionClass, 'initializeFromError'], $types);
+		try {
+			return $callback();
+		} finally {
+			restore_error_handler();
+		}
+	}
+
 	/**
 	 * Convenience function for getNthMatch(0).
 	 */
@@ -537,17 +546,9 @@ private function parseXMLFile($filename, $flags = 0, $context = null)
 		// If a context is specified, we basically have to do the reading in
 		// two steps:
 		if (! empty($context)) {
-			try {
-				set_error_handler(['\QueryPath\ParseException', 'initializeFromError'], $this->errTypes);
-				$contents = file_get_contents($filename, false, $context);
-			}
-				// Apparently there is no 'finally' in PHP, so we have to restore the error
-				// handler this way:
-			catch (Exception $e) {
-				restore_error_handler();
-				throw $e;
-			}
-			restore_error_handler();
+			$contents = self::withErrorHandler(ParseException::class, function () use ($filename, $context) {
+				return file_get_contents($filename, false, $context);
+			}, $this->errTypes);
 
 			if ($contents == false) {
 				throw new ParseException(sprintf(
@@ -576,27 +577,16 @@ private function parseXMLFile($filename, $flags = 0, $context = null)
 
 		$ext = $lastDot !== false ? strtolower(substr($filename, $lastDot)) : '';
 
-		try {
-			set_error_handler([ParseException::class, 'initializeFromError'], $this->errTypes);
+		// An explicit parser setting wins; otherwise go by the file extension, defaulting to XML.
+		$asHtml = $useParser === 'html' || ($useParser !== 'xml' && isset($htmlExtensions[$ext]));
 
-			// If the parser is explicitly set to XML, use that parser.
-			if ($useParser === 'xml') {
-				$document->load($filename, $flags);
-			} // Otherwise, see if it looks like HTML.
-			elseif ($useParser === 'html' || isset($htmlExtensions[$ext])) {
-				// Try parsing it as HTML.
+		self::withErrorHandler(ParseException::class, function () use ($document, $filename, $flags, $asHtml) {
+			if ($asHtml) {
 				self::loadHTMLFile($document, $filename);
-			} // Default to XML.
-			else {
+			} else {
 				$document->load($filename, $flags);
 			}
-
-		} // Emulate 'finally' behavior.
-		catch (Exception $e) {
-			restore_error_handler();
-			throw $e;
-		}
-		restore_error_handler();
+		}, $this->errTypes);
 
 		return $document;
 	}

src/DOMQuery.php

@@ -1338,14 +1338,9 @@ public function writeXML($path = null, $options = 0)
 		if ($path === null) {
 			print $this->document->saveXML(null, $options);
 		} else {
-			try {
-				set_error_handler([IOException::class, 'initializeFromError']);
+			self::withErrorHandler(IOException::class, function () use ($path, $options) {
 				$this->document->save($path, $options);
-			} catch (Exception $e) {
-				restore_error_handler();
-				throw $e;
-			}
-			restore_error_handler();
+			});
 		}
 
 		return $this;
@@ -1376,14 +1371,9 @@ public function writeHTML($path = null)
 		if ($path === null) {
 			print $this->saveDocumentHTML();
 		} else {
-			try {
-				set_error_handler(['\QueryPath\ParseException', 'initializeFromError']);
+			self::withErrorHandler(ParseException::class, function () use ($path) {
 				$this->saveDocumentHTML($path);
-			} catch (Exception $e) {
-				restore_error_handler();
-				throw $e;
-			}
-			restore_error_handler();
+			});
 		}
 
 		return $this;
@@ -1413,12 +1403,9 @@ public function writeHTML5($path = null)
 			// Print the document to stdout.
 			print $html5->saveHTML($this->document);
 		} else {
-			set_error_handler([IOException::class, 'initializeFromError']);
-			try {
+			self::withErrorHandler(IOException::class, function () use ($html5, $path) {
 				$html5->save($this->document, $path);
-			} finally {
-				restore_error_handler();
-			}
+			});
 		}
 
 		return $this;

tests/QueryPath/DOMQueryTest.php

@@ -16,6 +16,7 @@
 use QueryPath\ParseException;
 use QueryPath\QueryPath;
 use QueryPath\DOMQuery;
+use RuntimeException;
 use SplDoublyLinkedList;
 use stdClass;
 
@@ -1504,6 +1505,55 @@ public function testFailWriteHTML5()
 		html5qp('<html><body>bar</body></html>')->writeHTML5('./test/no-writing.html');
 	}
 
+	public function throwingCallProvider(): array
+	{
+		$url = ThrowingStreamWrapper::PROTOCOL . '://out';
+
+		return [
+			'writeXML'          => [function () use ($url) {
+				qp('<?xml version="1.0"?><foo/>')->writeXML($url);
+			}],
+			'writeHTML'         => [function () use ($url) {
+				htmlqp('<p>bar</p>')->writeHTML($url);
+			}],
+			'writeHTML5'        => [function () use ($url) {
+				html5qp('<p>bar</p>')->writeHTML5($url);
+			}],
+			'load with context' => [function () use ($url) {
+				qp($url . '.xml', null, ['context' => stream_context_create()]);
+			}],
+		];
+	}
+
+	/**
+	 * A throwable QueryPath does not convert must not leave its error handler installed.
+	 *
+	 * @dataProvider throwingCallProvider
+	 *
+	 * @param callable $callback
+	 */
+	public function testErrorHandlerRestoredAfterForeignThrowable(callable $callback)
+	{
+		$sentinel = function () {
+			return false;
+		};
+
+		stream_wrapper_register(ThrowingStreamWrapper::PROTOCOL, ThrowingStreamWrapper::class);
+		set_error_handler($sentinel);
+		try {
+			$callback();
+			$this->fail('Expected a RuntimeException.');
+		} catch (RuntimeException $e) {
+			$current = set_error_handler(null);
+			restore_error_handler();
+		} finally {
+			restore_error_handler();
+			stream_wrapper_unregister(ThrowingStreamWrapper::PROTOCOL);
+		}
+
+		$this->assertSame($sentinel, $current);
+	}
+
 	public function testWriteHTML5()
 	{
 		$qp = html5qp('<html><body>bar</body></html>');

tests/QueryPath/ThrowingStreamWrapper.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace QueryPathTests;
+
+use RuntimeException;
+
+/**
+ * A stream wrapper that throws a non-QueryPath exception when a stream is opened.
+ */
+class ThrowingStreamWrapper
+{
+	public const PROTOCOL = 'qpthrow';
+
+	public $context;
+
+	public function stream_open($path, $mode, $options, &$opened_path) // phpcs:ignore PSR1.Methods.CamelCapsMethodName
+	{
+		throw new RuntimeException('Cannot open ' . $path);
+	}
+}