Skip to content

Commit f7a6fd4

Browse files
GirgiasGirgias
committed
[skip ci] Check old FCCs for new set object
Has heap after free and still need more tests, especially around using proper callables for methods on the same class
1 parent e260bd6 commit f7a6fd4

File tree

2 files changed

+71
-36
lines changed

2 files changed

+71
-36
lines changed

‎ext/xml/tests/xml_set_object_multiple_times.phpt

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ xml
55
--FILE--
66
<?php
77

8+
function end_handler(XMLParser $parser, string $tag) {
9+
echo "end_handler($tag)\n";
10+
}
11+
812
class A {
913
public function start_element($parser, $name, $attributes) {
1014
global $b;
@@ -17,22 +21,27 @@ class B {
1721
public function start_element($parser, $name) {
1822
echo "B::start_element($name)\n";
1923
}
24+
public function end_element($parser, $name) {
25+
echo "B::end_element($name)\n";
26+
}
2027
}
2128

2229
$a = new A;
2330
$b = new B;
2431

2532
$parser = xml_parser_create();
2633
xml_set_object($parser, $a);
27-
xml_set_element_handler($parser, "start_element", null);
34+
xml_set_element_handler($parser, "start_element", "end_handler");
2835
xml_parse($parser, <<<XML
2936
<?xml version="1.0"?>
3037
<container>
31-
<child>
38+
<child/>
3239
</container>
3340
XML);
3441

3542
?>
3643
--EXPECT--
3744
A::start_element(CONTAINER)
3845
B::start_element(CHILD)
46+
end_handler(CHILD)
47+
end_handler(CONTAINER)

‎ext/xml/xml.c

Lines changed: 60 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,9 @@ static HashTable *xml_parser_get_gc(zend_object *object, zval **table, int *n)
371371
xml_parser *parser = xml_parser_from_obj(object);
372372

373373
zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
374-
zend_get_gc_buffer_add_obj(gc_buffer, parser->object);
374+
if (parser->object) {
375+
zend_get_gc_buffer_add_obj(gc_buffer, parser->object);
376+
}
375377
if (ZEND_FCC_INITIALIZED(parser->startElementHandler)) {
376378
zend_get_gc_buffer_add_fcc(gc_buffer, &parser->startElementHandler);
377379
}
@@ -1050,33 +1052,9 @@ PHP_FUNCTION(xml_parser_create_ns)
10501052
}
10511053
/* }}} */
10521054

1053-
/* {{{ Set up object which should be used for callbacks */
1054-
PHP_FUNCTION(xml_set_object)
1055-
{
1056-
xml_parser *parser;
1057-
zval *pind, *mythis;
1058-
1059-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oo", &pind, xml_parser_ce, &mythis) == FAILURE) {
1060-
RETURN_THROWS();
1061-
}
1062-
1063-
parser = Z_XMLPARSER_P(pind);
1064-
1065-
if (parser->object) {
1066-
// TODO Check methods exist in set FCCs
1067-
OBJ_RELEASE(parser->object);
1068-
}
1069-
1070-
parser->object = Z_OBJ_P(mythis);
1071-
GC_ADDREF(parser->object);
1072-
1073-
RETURN_TRUE;
1074-
}
1075-
/* }}} */
1076-
10771055
static bool php_xml_check_string_method_arg(
10781056
unsigned int arg_num,
1079-
const xml_parser *parser,
1057+
zend_object *object,
10801058
zend_string *method_name,
10811059
zend_fcall_info_cache *const parser_handler_fcc
10821060
) {
@@ -1085,12 +1063,12 @@ static bool php_xml_check_string_method_arg(
10851063
return true;
10861064
}
10871065

1088-
if (!parser->object) {
1066+
if (!object) {
10891067
zend_argument_value_error(arg_num, "an object must be set via xml_set_object() to be able to lookup method");
10901068
return false;
10911069
}
10921070

1093-
zend_class_entry *ce = parser->object->ce;
1071+
zend_class_entry *ce = object->ce;
10941072
zend_string *lc_name = zend_string_tolower(method_name);
10951073
zend_function *method_ptr = zend_hash_find_ptr(&ce->function_table, lc_name);
10961074
zend_string_release_ex(lc_name, 0);
@@ -1102,11 +1080,59 @@ static bool php_xml_check_string_method_arg(
11021080
parser_handler_fcc->function_handler = method_ptr;
11031081
parser_handler_fcc->calling_scope = ce;
11041082
parser_handler_fcc->called_scope = ce;
1105-
parser_handler_fcc->object = parser->object;
1083+
parser_handler_fcc->object = object;
11061084

11071085
return true;
11081086
}
11091087

1088+
#define PHP_XML_CHECK_NEW_THIS_METHODS(parser_to_check, new_this_obj, fcc_field) \
1089+
if (ZEND_FCC_INITIALIZED(parser_to_check->fcc_field) && parser_to_check->fcc_field.object == parser_to_check->object) { \
1090+
zend_string *method_name = zend_string_copy(parser_to_check->fcc_field.function_handler->common.function_name); \
1091+
zend_fcc_dtor(&parser_to_check->fcc_field); \
1092+
bool status = php_xml_check_string_method_arg(2, new_this_obj, method_name, &parser_to_check->fcc_field); \
1093+
if (status == false) { \
1094+
/* TODO Better error message */ RETURN_THROWS(); \
1095+
} \
1096+
}
1097+
1098+
1099+
/* {{{ Set up object which should be used for callbacks */
1100+
PHP_FUNCTION(xml_set_object)
1101+
{
1102+
xml_parser *parser;
1103+
zval *pind, *mythis;
1104+
zend_object *new_this;
1105+
1106+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oo", &pind, xml_parser_ce, &mythis) == FAILURE) {
1107+
RETURN_THROWS();
1108+
}
1109+
1110+
parser = Z_XMLPARSER_P(pind);
1111+
new_this = Z_OBJ_P(mythis);
1112+
1113+
if (parser->object) {
1114+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, startElementHandler);
1115+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, endElementHandler);
1116+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, characterDataHandler);
1117+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, processingInstructionHandler);
1118+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, defaultHandler);
1119+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, unparsedEntityDeclHandler);
1120+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, notationDeclHandler);
1121+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, externalEntityRefHandler);
1122+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, unknownEncodingHandler);
1123+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, startNamespaceDeclHandler);
1124+
PHP_XML_CHECK_NEW_THIS_METHODS(parser, new_this, endNamespaceDeclHandler);
1125+
1126+
OBJ_RELEASE(parser->object);
1127+
}
1128+
1129+
parser->object = new_this;
1130+
GC_ADDREF(parser->object);
1131+
1132+
RETURN_TRUE;
1133+
}
1134+
/* }}} */
1135+
11101136
/* {{{ Set up start and end element handlers */
11111137
PHP_FUNCTION(xml_set_element_handler)
11121138
{
@@ -1136,7 +1162,7 @@ PHP_FUNCTION(xml_set_element_handler)
11361162
} else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "Of!S", &pind, xml_parser_ce, &start_fci, &start_fcc, &end_method_name) == SUCCESS) {
11371163
parser = Z_XMLPARSER_P(pind);
11381164

1139-
bool status = php_xml_check_string_method_arg(3, parser, end_method_name, &end_fcc);
1165+
bool status = php_xml_check_string_method_arg(3, parser->object, end_method_name, &end_fcc);
11401166
if (status == false) {
11411167
RETURN_THROWS();
11421168
}
@@ -1150,7 +1176,7 @@ PHP_FUNCTION(xml_set_element_handler)
11501176
} else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OSf!", &pind, xml_parser_ce, &start_method_name, &end_fci, &end_fcc) == SUCCESS) {
11511177
parser = Z_XMLPARSER_P(pind);
11521178

1153-
bool status = php_xml_check_string_method_arg(2, parser, start_method_name, &start_fcc);
1179+
bool status = php_xml_check_string_method_arg(2, parser->object, start_method_name, &start_fcc);
11541180
if (status == false) {
11551181
RETURN_THROWS();
11561182
}
@@ -1164,11 +1190,11 @@ PHP_FUNCTION(xml_set_element_handler)
11641190
} else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OSS", &pind, xml_parser_ce, &start_method_name, &end_method_name) == SUCCESS) {
11651191
parser = Z_XMLPARSER_P(pind);
11661192

1167-
bool status = php_xml_check_string_method_arg(2, parser, start_method_name, &start_fcc);
1193+
bool status = php_xml_check_string_method_arg(2, parser->object, start_method_name, &start_fcc);
11681194
if (status == false) {
11691195
RETURN_THROWS();
11701196
}
1171-
status = php_xml_check_string_method_arg(3, parser, end_method_name, &end_fcc);
1197+
status = php_xml_check_string_method_arg(3, parser->object, end_method_name, &end_fcc);
11721198
if (status == false) {
11731199
RETURN_THROWS();
11741200
}
@@ -1225,7 +1251,7 @@ static void php_xml_set_handler_parse_callable(
12251251
} else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OS", &pind, xml_parser_ce, &method_name) == SUCCESS) {
12261252
*parser = Z_XMLPARSER_P(pind);
12271253

1228-
bool status = php_xml_check_string_method_arg(2, *parser, method_name, parser_handler_fcc);
1254+
bool status = php_xml_check_string_method_arg(2, (*parser)->object, method_name, parser_handler_fcc);
12291255
if (status == false) {
12301256
RETURN_THROWS();
12311257
}

0 commit comments

Comments
 (0)