Commits

Tim Brody  committed 95fe5c0

Create a new owning document for function callbacks which fixes rt71345. Added the two test cases from that ticket to unit tests.

Discussion: LibXSLT assumed the owner of the XPath node was the document being transformed. If the document was actually created by an intermediate extension e.g. exslt:node-set it won't be owned by us. This change creates an entirely new document for every callback that is passed to perl_dispatch and lasts for the duration of the dispatch or the parent scope if it or any owned elements are referenced.

  • Participants
  • Parent commits fc3d465

Comments (0)

Files changed (4)

 # ido - perl dispatcher
 sub perl_dispatcher {
     my $func = shift;
+	my $owner_doc = shift;
     my @params = @_;
     my @perlParams;
     
 
 static void
 LibXSLT__function (xmlXPathParserContextPtr ctxt, int nargs, SV *perl_function) {
+	SV * owner_doc;
     xmlXPathObjectPtr obj,ret;
     xmlNodeSetPtr nodelist = NULL;
     int count;
     
     XPUSHs(perl_function);
 
+	/* clone all of the arguments into a new owning document */
+	owner_doc = x_PmmNodeToSv(INT2PTR(xmlNodePtr,xmlNewDoc("1.0")),NULL);
+	XPUSHs( sv_2mortal(owner_doc) );
+
     /* set up call to perl dispatcher function */
     for (i = 0; i < nargs; i++) {
         obj = (xmlXPathObjectPtr)valuePop(ctxt);
         case XPATH_NODESET:
         case XPATH_XSLT_TREE:
             nodelist = obj->nodesetval;
-            if ( nodelist ) {
-                XPUSHs(sv_2mortal(newSVpv("XML::LibXML::NodeList", 0)));				
-                XPUSHs(sv_2mortal(newSViv(nodelist->nodeNr)));
-                if ( nodelist->nodeNr > 0 ) {
-                    int i = 0 ;
-                    const char * cls = "XML::LibXML::Node";
-                    xmlNodePtr tnode = NULL;
-                    SV * element = NULL;
-                    len = nodelist->nodeNr;
-                    for( ; i < len; i++ ){
-                        tnode = nodelist->nodeTab[i];
-                        if (tnode->type == XML_NAMESPACE_DECL) {
-                            element = sv_newmortal();
-                            cls = x_PmmNodeTypeName( tnode );
-                            element = sv_setref_pv( element,
-                                                    (const char *)cls,
-                                                    (void *)xmlCopyNamespace((xmlNsPtr)tnode)
-                                                );
-			} else {
-                          /* need to copy the node as libxml2 will free it */
-			  xmlNodePtr tnode_cpy = xmlCopyNode(tnode, 1);
-			    if( tnode_cpy != NULL) {
-			      ProxyNodePtr owner = NULL;
-			      if ( tnode_cpy != NULL && tnode_cpy->doc != NULL) {
-				owner = x_PmmOWNERPO(x_PmmNewNode(INT2PTR(xmlNodePtr,tnode_cpy->doc)));
-			      }
-			      element = x_PmmNodeToSv(tnode_cpy, owner);
-			    }
-                        }
-                        XPUSHs( sv_2mortal(element) );
-                    }
-                }
-            }
+			if ( nodelist == NULL )
+				break;
+			XPUSHs(sv_2mortal(newSVpv("XML::LibXML::NodeList", 0)));
+			XPUSHs(sv_2mortal(newSViv(nodelist->nodeNr)));
+			if ( nodelist->nodeNr == 0 )
+				break;
+			const char * cls = "XML::LibXML::Node";
+			xmlNodePtr tnode = NULL;
+			SV * element = NULL;
+			int i;
+			for(i=0; i < nodelist->nodeNr; i++ ){
+				tnode = nodelist->nodeTab[i];
+				/* need to copy the node as libxml2 will free it */
+				if (tnode->type == XML_NAMESPACE_DECL) {
+					element = sv_newmortal();
+					cls = x_PmmNodeTypeName( tnode );
+					element = sv_setref_pv( element,
+							(const char *)cls,
+							(void *)xmlCopyNamespace((xmlNsPtr)tnode)
+							);
+				}
+				else {
+					xmlNodePtr tnode_cpy = xmlDocCopyNode(tnode,INT2PTR(xmlDocPtr,x_PmmNODE(SvPROXYNODE(owner_doc))),1);
+					if( tnode_cpy == NULL )
+						break;
+					element = x_PmmNodeToSv(tnode_cpy,SvPROXYNODE(owner_doc));
+				}
+				XPUSHs( sv_2mortal(element) );
+			}
             break;
         case XPATH_BOOLEAN:
             XPUSHs(sv_2mortal(newSVpv("XML::LibXML::Boolean", 0)));

File t/rt71345_a.t

+use Test::More tests => 2;
+
+{
+use strict;
+use warnings;
+
+use XML::LibXSLT;
+use XML::LibXML;
+
+my $xslt = XML::LibXSLT->new();
+my $ext_uri = "urn:local";
+my @keep;
+XML::LibXSLT->register_function($ext_uri, "uc", sub { push @keep, @_; return uc shift; } );
+
+my $stylesheet = $xslt->parse_stylesheet(XML::LibXML->load_xml(string => <<'EOF'));
+<xsl:stylesheet version="1.0"
+                extension-element-prefixes="exsl local"
+                xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+                xmlns:exsl="http://exslt.org/common" 
+                xmlns:local="urn:local">
+  <xsl:template match="/">
+    <xsl:variable name="foo"><foo a="foo"/></xsl:variable>
+    <bar><xsl:value-of select="local:uc(exsl:node-set($foo)//@a)"/></bar>
+  </xsl:template>
+</xsl:stylesheet>
+EOF
+
+my $input = XML::LibXML->load_xml(string => "<input/>");
+ok($stylesheet->transform($input)->toString =~ "<bar>FOO</bar>");
+
+# Next line crashes Perl
+@keep = undef;
+
+}
+ok(1);

File t/rt71345_b.t

+use Test::More tests => 2;
+
+{
+use strict;
+use warnings;
+
+use XML::LibXSLT;
+use XML::LibXML;
+
+my $xslt = XML::LibXSLT->new();
+my $ext_uri = "urn:local";
+XML::LibXSLT->register_function($ext_uri, "uc", sub { return uc shift; } );
+
+my $stylesheet = $xslt->parse_stylesheet(XML::LibXML->load_xml(string => <<'EOF'));
+<xsl:stylesheet version="1.0"
+                extension-element-prefixes="exsl local"
+                xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+                xmlns:exsl="http://exslt.org/common" 
+                xmlns:local="urn:local">
+  <xsl:template match="/">
+    <xsl:variable name="foo"><foo a="foo"/></xsl:variable>
+    <bar><xsl:value-of select="local:uc(exsl:node-set($foo)//@a)"/></bar>
+  </xsl:template>
+</xsl:stylesheet>
+EOF
+
+my $input = XML::LibXML->load_xml(string => "<input/>");
+ok($stylesheet->transform($input)->toString =~ "<bar>FOO</bar>");
+
+}
+ok(1);