Skip to content

Xsl dom objects free storage upd #21533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
devnexen merged 1 commit into from
Mar 26, 2026
Merged

Xsl dom objects free storage upd #21533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ext/xsl/php_xsl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ zend_object *xsl_objects_new(zend_class_entry *class_type)
}
#endif

XSL_DEFINE_PROP_ACCESSOR(do_xinclude, "doXInclude", 0)
Copy link
Copy Markdown
Member

@ndossche ndossche Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd reorder these such that they last argument goes from 0 to 1 to 2 to 3.

XSL_DEFINE_PROP_ACCESSOR(clone_document, "cloneDocument", 1)
XSL_DEFINE_PROP_ACCESSOR(max_template_depth, "maxTemplateDepth", 2)
XSL_DEFINE_PROP_ACCESSOR(max_template_vars, "maxTemplateVars", 3)

Expand Down
2 changes: 2 additions & 0 deletions ext/xsl/php_xsl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ void xsl_ext_function_object_php(xmlXPathParserContextPtr ctxt, int nargs);

zval *xsl_prop_max_template_depth(zend_object *object);
zval *xsl_prop_max_template_vars(zend_object *object);
zval *xsl_prop_do_xinclude(zend_object *object);
zval *xsl_prop_clone_document(zend_object *object);

PHP_MINIT_FUNCTION(xsl);
PHP_MSHUTDOWN_FUNCTION(xsl);
Expand Down
21 changes: 9 additions & 12 deletions ext/xsl/tests/special_operations_with_properties.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,16 @@ echo "--- Unset cloneDocument ---\n";
$xslt = new XSLTProcessor;
$xslt->registerPHPFunctions();
unset($xslt->cloneDocument);
try {
$xslt->importStylesheet($xsl);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
$xslt->importStylesheet($xsl);
echo $xslt->transformToXml($xml);

echo "--- Unset doXInclude ---\n";

$xslt = new XSLTProcessor;
$xslt->registerPHPFunctions();
unset($xslt->doXInclude);
$xslt->importStylesheet($xsl);
try {
echo $xslt->transformToXml($xml);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
echo $xslt->transformToXml($xml);

echo "--- Make properties references ---\n";

Expand All @@ -59,9 +52,13 @@ echo $xslt->transformToXml($xml);
?>
--EXPECT--
--- Unset cloneDocument ---
Typed property XSLTProcessor::$cloneDocument must not be accessed before initialization
Called test
<?xml version="1.0"?>
hello
--- Unset doXInclude ---
Typed property XSLTProcessor::$doXInclude must not be accessed before initialization
Called test
<?xml version="1.0"?>
hello
--- Make properties references ---
Called test
<?xml version="1.0"?>
Expand Down
24 changes: 5 additions & 19 deletions ext/xsl/xsltprocessor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ PHP_METHOD(XSLTProcessor, importStylesheet)
xsltStylesheetPtr sheetp;
bool clone_docu = false;
xmlNode *nodep = NULL;
zval *cloneDocu, rv, clone_zv, owner_zv;
zend_string *member;
zval *cloneDocu, clone_zv, owner_zv;

id = ZEND_THIS;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "o", &docp) == FAILURE) {
Expand Down Expand Up @@ -216,15 +215,7 @@ PHP_METHOD(XSLTProcessor, importStylesheet)
ZVAL_OBJ(&clone_zv, clone);
nodep = php_libxml_import_node(&clone_zv);

if (nodep) {
newdoc = nodep->doc;
}
if (newdoc == NULL) {
OBJ_RELEASE(clone);
zend_argument_type_error(1, "must be a valid XML node");
RETURN_THROWS();
}

newdoc = nodep->doc;
php_libxml_node_object *clone_lxml_obj = Z_LIBXML_NODE_P(&clone_zv);

PHP_LIBXML_SANITIZE_GLOBALS(parse);
Expand Down Expand Up @@ -259,10 +250,8 @@ PHP_METHOD(XSLTProcessor, importStylesheet)
intern->sheet_ref_obj->refcount++;
OBJ_RELEASE(clone);

member = ZSTR_INIT_LITERAL("cloneDocument", 0);
cloneDocu = zend_std_read_property(Z_OBJ_P(id), member, BP_VAR_R, NULL, &rv);
cloneDocu = xsl_prop_clone_document(Z_OBJ_P(id));
Copy link
Copy Markdown
Member

@ndossche ndossche Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change these?

Copy link
Copy Markdown
Member Author

@devnexen devnexen Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid a zend_string allocation + property hash lookup per call. maxTemplateDepth and maxTemplateVars already use direct slot access via XSL_DEFINE_PROP_ACCESSOR —
this brings doXInclude and cloneDocument in line with the same pattern. but I put these in separate commit just in case :)

clone_docu = zend_is_true(cloneDocu);
zend_string_release_ex(member, 0);
if (!clone_docu) {
/* Check if the stylesheet is using xsl:key, if yes, we have to clone the document _always_ before a transformation.
* xsl:key elements may only occur at the top level. Furthermore, all elements at the top level must be in a
Expand Down Expand Up @@ -302,8 +291,7 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl
xmlNodePtr node = NULL;
xsltTransformContextPtr ctxt;
php_libxml_node_object *object;
zval *doXInclude, rv;
zend_string *member;
zval *doXInclude;
FILE *f;
int secPrefsError = 0;
xsltSecurityPrefsPtr secPrefs = NULL;
Expand Down Expand Up @@ -359,10 +347,8 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl
}
}

member = ZSTR_INIT_LITERAL("doXInclude", 0);
doXInclude = zend_std_read_property(Z_OBJ_P(id), member, BP_VAR_R, NULL, &rv);
doXInclude = xsl_prop_do_xinclude(Z_OBJ_P(id));
ctxt->xinclude = zend_is_true(doXInclude);
zend_string_release_ex(member, 0);

zval *max_template_depth = xsl_prop_max_template_depth(Z_OBJ_P(id));
ZEND_ASSERT(Z_TYPE_P(max_template_depth) == IS_LONG);
Expand Down
Loading