Fix bug #81521

The current error message is incorrect -- the problem here is not that the property is invalid, but that these methods are unusable prior to loading data, same as read().
php · Oct 12, 2021 · 53f8921 · 53f8921
1 parent 9ebe849
commit 53f8921
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 6 deletions.
diff --git a/NEWS b/NEWS
@@ -8,6 +8,10 @@ PHP                                                                        NEWS
 - PCRE:
   . Fixed bug #81424 (PCRE2 10.35 JIT performance regression). (cmb)
 
+- XMLReader:
+  . Fixed bug #81521 (XMLReader::getParserProperty may throw with a valid
+    property). (Nikita)
+
 21 Oct 2021, PHP 8.0.12
 
 - CLI:

diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c
@@ -614,7 +614,6 @@ PHP_METHOD(XMLReader, getParserProperty)
 {
 	zval *id;
 	zend_long property;
-	int retval = -1;
 	xmlreader_object *intern;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &property) == FAILURE) {
@@ -624,9 +623,12 @@ PHP_METHOD(XMLReader, getParserProperty)
 	id = ZEND_THIS;
 
 	intern = Z_XMLREADER_P(id);
-	if (intern && intern->ptr) {
-		retval = xmlTextReaderGetParserProp(intern->ptr,property);
+	if (!intern || !intern->ptr) {
+		zend_throw_error(NULL, "Cannot access parser properties before loading data");
+		RETURN_THROWS();
 	}
+
+	int retval = xmlTextReaderGetParserProp(intern->ptr,property);
 	if (retval == -1) {
 		zend_argument_value_error(1, "must be a valid parser property");
 		RETURN_THROWS();
@@ -968,7 +970,6 @@ PHP_METHOD(XMLReader, setParserProperty)
 {
 	zval *id;
 	zend_long property;
-	int retval = -1;
 	zend_bool value;
 	xmlreader_object *intern;
 
@@ -979,9 +980,12 @@ PHP_METHOD(XMLReader, setParserProperty)
 	id = ZEND_THIS;
 
 	intern = Z_XMLREADER_P(id);
-	if (intern && intern->ptr) {
-		retval = xmlTextReaderSetParserProp(intern->ptr,property, value);
+	if (!intern || !intern->ptr) {
+		zend_throw_error(NULL, "Cannot access parser properties before loading data");
+		RETURN_THROWS();
 	}
+
+	int retval = xmlTextReaderSetParserProp(intern->ptr,property, value);
 	if (retval == -1) {
 		zend_argument_value_error(1, "must be a valid parser property");
 		RETURN_THROWS();

diff --git a/ext/xmlreader/tests/bug81521.phpt b/ext/xmlreader/tests/bug81521.phpt
@@ -0,0 +1,19 @@
+--TEST--
+Bug #81521: XMLReader::getParserProperty may throw with a valid property
+--FILE--
+<?php
+$reader = new XMLReader();
+try {
+    var_dump($reader->setParserProperty(XMLReader::LOADDTD, 1));
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($reader->getParserProperty(XMLReader::LOADDTD));
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--EXPECT--
+Cannot access parser properties before loading data
+Cannot access parser properties before loading data