XML options handling refactoring

Zend/Optimizer/zend_func_infos.h

@@ -688,7 +688,7 @@ static const func_info_t func_infos[] = {
 	F1("var_export", MAY_BE_STRING|MAY_BE_NULL),
 	F1("serialize", MAY_BE_STRING),
 	F1("xml_error_string", MAY_BE_STRING|MAY_BE_NULL),
-	F1("xml_parser_get_option", MAY_BE_STRING|MAY_BE_LONG),
+	F1("xml_parser_get_option", MAY_BE_STRING|MAY_BE_LONG|MAY_BE_BOOL),
 	FN("zip_open", MAY_BE_RESOURCE|MAY_BE_LONG|MAY_BE_FALSE),
 	FN("zip_read", MAY_BE_RESOURCE|MAY_BE_FALSE),
 	F1("ob_gzhandler", MAY_BE_STRING|MAY_BE_FALSE),

ext/xml/tests/bug72714.phpt

@@ -2,6 +2,10 @@
 Bug #72714 (_xml_startElementHandler() segmentation fault)
 --EXTENSIONS--
 xml
+--SKIPIF--
+<?php
+if (PHP_INT_SIZE < 8) die('skip this test is for 64bit builds only');
+?>
 --FILE--
 <?php
 function startElement($parser, $name, $attribs) {
@@ -26,6 +30,6 @@ parse(3015809298423721);
 parse(20);
 ?>
 --EXPECTF--
-Warning: xml_parser_set_option(): tagstart ignored, because it is out of range in %s on line %d
+Warning: xml_parser_set_option(): Argument #3 ($value) must be between 0 and 2147483647 for option XML_OPTION_SKIP_TAGSTART in %s on line %d
 string(9) "NS1:TOTAL"
 string(0) ""

ext/xml/tests/xml_parser_get_option_variation3.phpt

@@ -8,20 +8,26 @@ $parser = xml_parser_create();
 echo "defaults:\n";
 var_dump(xml_parser_get_option($parser, XML_OPTION_SKIP_TAGSTART));
 var_dump(xml_parser_get_option($parser, XML_OPTION_SKIP_WHITE));
+var_dump(xml_parser_get_option($parser, XML_OPTION_CASE_FOLDING));
 echo "setting:\n";
 var_dump(xml_parser_set_option($parser, XML_OPTION_SKIP_TAGSTART, 7));
 var_dump(xml_parser_set_option($parser, XML_OPTION_SKIP_WHITE, 1));
+var_dump(xml_parser_set_option($parser, XML_OPTION_CASE_FOLDING, false));
 echo "getting:\n";
 var_dump(xml_parser_get_option($parser, XML_OPTION_SKIP_TAGSTART));
 var_dump(xml_parser_get_option($parser, XML_OPTION_SKIP_WHITE));
+var_dump(xml_parser_get_option($parser, XML_OPTION_CASE_FOLDING));
 ?>
 --EXPECT--
 defaults:
 int(0)
-int(0)
+bool(false)
+bool(true)
 setting:
 bool(true)
 bool(true)
+bool(true)
 getting:
 int(7)
-int(1)
+bool(true)
+bool(false)

ext/xml/tests/xml_parser_get_option_variation4.phpt

@@ -15,4 +15,4 @@ try {
 
 ?>
 --EXPECT--
-xml_parser_get_option(): Argument #2 ($option) must be a PHP_XML_OPTION_* constant
+xml_parser_get_option(): Argument #2 ($option) must be a XML_OPTION_* constant

ext/xml/tests/xml_parser_set_option_basic.phpt

@@ -32,15 +32,15 @@ echo "Done\n";
 ?>
 --EXPECT--
 Simple testcase for xml_parser_get_option() function
-int(1)
+bool(true)
 string(5) "UTF-8"
 bool(true)
 bool(true)
-int(1)
+bool(true)
 string(10) "ISO-8859-1"
 bool(true)
 bool(true)
-int(0)
+bool(false)
 string(5) "UTF-8"
 bool(true)
 string(8) "US-ASCII"

ext/xml/tests/xml_parser_set_option_errors.phpt

@@ -0,0 +1,88 @@
+--TEST--
+xml_parser_set_option(): Setting options to invalid values
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+
+$xmlParser = xml_parser_create();
+
+echo "Case folding\n";
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_CASE_FOLDING, []);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_CASE_FOLDING, new stdClass());
+} catch (TypeError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+echo "Skip Whitespace\n";
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_SKIP_WHITE, []);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_SKIP_WHITE, new stdClass());
+} catch (TypeError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+echo "Tag Start\n";
+xml_parser_set_option($xmlParser, XML_OPTION_SKIP_TAGSTART, -5);
+
+xml_parser_set_option($xmlParser, XML_OPTION_SKIP_TAGSTART, []);
+
+xml_parser_set_option($xmlParser, XML_OPTION_SKIP_TAGSTART, new stdClass());
+
+echo "Encodings\n";
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_TARGET_ENCODING, 'Invalid Encoding');
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_TARGET_ENCODING, []);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+try {
+    xml_parser_set_option($xmlParser, XML_OPTION_TARGET_ENCODING, new stdClass());
+} catch (Error $exception) {
+    echo $exception::class, ': ', $exception->getMessage() . "\n";
+}
+
+?>
+--EXPECTF--
+Case folding
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, array given in %s on line %d
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, stdClass given in %s on line %d
+Skip Whitespace
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, array given in %s on line %d
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, stdClass given in %s on line %d
+Tag Start
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be between 0 and 2147483647 for option XML_OPTION_SKIP_TAGSTART in %s on line %d
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, array given in %s on line %d
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, stdClass given in %s on line %d
+
+Warning: Object of class stdClass could not be converted to int in %s on line %d
+Encodings
+xml_parser_set_option(): Argument #3 ($value) is not a supported target encoding
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, array given in %s on line %d
+
+Warning: Array to string conversion in %s on line %d
+xml_parser_set_option(): Argument #3 ($value) is not a supported target encoding
+
+Warning: xml_parser_set_option(): Argument #3 ($value) must be of type string|int|bool, stdClass given in %s on line %d
+Error: Object of class stdClass could not be converted to string

ext/xml/tests/xml_parser_set_option_variation5.phpt → ext/xml/tests/xml_parser_set_option_nonexistent_option.phpt

@@ -1,5 +1,5 @@
 --TEST--
-xml_parser_set_option() - Test invalid parameter
+xml_parser_set_option(): Non-existent option
 --EXTENSIONS--
 xml
 --FILE--
@@ -15,4 +15,4 @@ try {
 
 ?>
 --EXPECT--
-xml_parser_set_option(): Argument #2 ($option) must be a PHP_XML_OPTION_* constant
+xml_parser_set_option(): Argument #2 ($option) must be a XML_OPTION_* constant

ext/xml/xml.c

@@ -132,7 +132,7 @@ ZEND_GET_MODULE(xml)
 
 #define XML_MAXLEVEL 255 /* XXX this should be dynamic */
 
-#define SKIP_TAGSTART(str) ((str) + (parser->toffset > (int)strlen(str) ? strlen(str) : parser->toffset))
+#define SKIP_TAGSTART(str) ((str) + (parser->toffset > strlen(str) ? strlen(str) : parser->toffset))
 
 static zend_class_entry *xml_parser_ce;
 static zend_object_handlers xml_parser_object_handlers;
@@ -1392,37 +1392,51 @@ PHP_FUNCTION(xml_parser_free)
 PHP_FUNCTION(xml_parser_set_option)
 {
 	xml_parser *parser;
-	zval *pind, *val;
+	zval *pind;
 	zend_long opt;
+	zval *value;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "Olz", &pind, xml_parser_ce, &opt, &val) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "Olz", &pind, xml_parser_ce, &opt, &value) == FAILURE) {
 		RETURN_THROWS();
 	}
 
+	if (Z_TYPE_P(value) != IS_FALSE && Z_TYPE_P(value) != IS_TRUE &&
+		Z_TYPE_P(value) != IS_LONG && Z_TYPE_P(value) != IS_STRING) {
+		php_error_docref(NULL, E_WARNING,
+			"Argument #3 ($value) must be of type string|int|bool, %s given", zend_zval_type_name(value));
+	}
+
 	parser = Z_XMLPARSER_P(pind);
 	switch (opt) {
+		/* Boolean option */
 		case PHP_XML_OPTION_CASE_FOLDING:
-			parser->case_folding = zval_get_long(val);
+			parser->case_folding = zend_is_true(value);
+			break;
+		/* Boolean option */
+		case PHP_XML_OPTION_SKIP_WHITE:
+			parser->skipwhite = zend_is_true(value);
 			break;
+		/* Integer option */
 		case PHP_XML_OPTION_SKIP_TAGSTART:
-			parser->toffset = zval_get_long(val);
+			/* The tag start offset is stored in an int */
+			/* TODO Improve handling of values? */
+			parser->toffset = zval_get_long(value);
 			if (parser->toffset < 0) {
-				php_error_docref(NULL, E_WARNING, "tagstart ignored, because it is out of range");
-				parser->toffset = 0;
 				/* TODO Promote to ValueError in PHP 9.0 */
+				php_error_docref(NULL, E_WARNING, "Argument #3 ($value) must be between 0 and %d"
+					" for option XML_OPTION_SKIP_TAGSTART", INT_MAX);
+				parser->toffset = 0;
 				RETURN_FALSE;
 			}
 			break;
-		case PHP_XML_OPTION_SKIP_WHITE:
-			parser->skipwhite = zval_get_long(val);
-			break;
+		/* String option */
 		case PHP_XML_OPTION_TARGET_ENCODING: {
 			const xml_encoding *enc;
-			if (!try_convert_to_string(val)) {
+			if (!try_convert_to_string(value)) {
 				RETURN_THROWS();
 			}
 
-			enc = xml_get_encoding((XML_Char*)Z_STRVAL_P(val));
+			enc = xml_get_encoding((XML_Char*)Z_STRVAL_P(value));
 			if (enc == NULL) {
 				zend_argument_value_error(3, "is not a supported target encoding");
 				RETURN_THROWS();
@@ -1432,7 +1446,7 @@ PHP_FUNCTION(xml_parser_set_option)
 			break;
 		}
 		default:
-			zend_argument_value_error(2, "must be a PHP_XML_OPTION_* constant");
+			zend_argument_value_error(2, "must be a XML_OPTION_* constant");
 			RETURN_THROWS();
 			break;
 	}
@@ -1455,19 +1469,19 @@ PHP_FUNCTION(xml_parser_get_option)
 	parser = Z_XMLPARSER_P(pind);
 	switch (opt) {
 		case PHP_XML_OPTION_CASE_FOLDING:
-			RETURN_LONG(parser->case_folding);
+			RETURN_BOOL(parser->case_folding);
 			break;
 		case PHP_XML_OPTION_SKIP_TAGSTART:
 			RETURN_LONG(parser->toffset);
 			break;
 		case PHP_XML_OPTION_SKIP_WHITE:
-			RETURN_LONG(parser->skipwhite);
+			RETURN_BOOL(parser->skipwhite);
 			break;
 		case PHP_XML_OPTION_TARGET_ENCODING:
 			RETURN_STRING((char *)parser->target_encoding);
 			break;
 		default:
-			zend_argument_value_error(2, "must be a PHP_XML_OPTION_* constant");
+			zend_argument_value_error(2, "must be a XML_OPTION_* constant");
 			RETURN_THROWS();
 	}
 }

ext/xml/xml.stub.php

@@ -197,11 +197,11 @@ function xml_get_current_byte_index(XMLParser $parser): int {}
 
 function xml_parser_free(XMLParser $parser): bool {}
 
-/** @param string|int $value */
+/** @param string|int|bool $value */
 function xml_parser_set_option(XMLParser $parser, int $option, $value): bool {}
 
 /** @refcount 1 */
-function xml_parser_get_option(XMLParser $parser, int $option): string|int {}
+function xml_parser_get_option(XMLParser $parser, int $option): string|int|bool {}
 
 /**
  * @strict-properties