ext/xsl: Remove dead code in importStylesheet and use direct property slot access for doXInclude and cloneDocument. (#21533)

The node and its document are validated before cloning, making
the subsequent NULL checks on nodep and newdoc redundant.

Use OBJ_PROP_NUM via XSL_DEFINE_PROP_ACCESSOR for doXInclude and
cloneDocument, avoiding zend_string allocation and property hash
lookup per call.

Author: devnexen

ext/xsl/php_xsl.c

@@ -144,6 +144,8 @@ zend_object *xsl_objects_new(zend_class_entry *class_type)
 	}
 #endif
 
+XSL_DEFINE_PROP_ACCESSOR(do_xinclude, "doXInclude", 0)
+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)
 

ext/xsl/php_xsl.h

@@ -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);

ext/xsl/tests/special_operations_with_properties.phpt

@@ -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";
 
@@ -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"?>

ext/xsl/xsltprocessor.c

@@ -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) {
@@ -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);
@@ -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));
 	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
@@ -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;
@@ -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);